From nobody Thu May 19 16:49:39 2022 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 50B4A1ADDA41; Thu, 19 May 2022 16:49:49 +0000 (UTC) (envelope-from dchagin@heemeyer.club) Received: from heemeyer.club (heemeyer.club [195.93.173.158]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4L3wnS1jWbz3Hb4; Thu, 19 May 2022 16:49:48 +0000 (UTC) (envelope-from dchagin@heemeyer.club) Received: from heemeyer.club (localhost [127.0.0.1]) by heemeyer.club (8.17.1/8.16.1) with ESMTP id 24JGndwA040098; Thu, 19 May 2022 19:49:39 +0300 (MSK) (envelope-from dchagin@heemeyer.club) Received: (from dchagin@localhost) by heemeyer.club (8.17.1/8.16.1/Submit) id 24JGndEi040097; Thu, 19 May 2022 19:49:39 +0300 (MSK) (envelope-from dchagin) Date: Thu, 19 May 2022 19:49:39 +0300 From: Dmitry Chagin To: Justin Hibbits Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 47a57144af25 - main - cpuset: Byte swap cpuset for compat32 on big endian architectures Message-ID: References: <202205191548.24JFmg0w065602@gitrepo.freebsd.org> List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <202205191548.24JFmg0w065602@gitrepo.freebsd.org> X-Rspamd-Queue-Id: 4L3wnS1jWbz3Hb4 X-Spamd-Bar: - Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=none (mx1.freebsd.org: domain of dchagin@heemeyer.club has no SPF policy when checking 195.93.173.158) smtp.mailfrom=dchagin@heemeyer.club X-Spamd-Result: default: False [-1.61 / 15.00]; RCVD_TLS_LAST(0.00)[]; ARC_NA(0.00)[]; FREEFALL_USER(0.00)[dchagin]; RCPT_COUNT_THREE(0.00)[4]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; FROM_HAS_DN(0.00)[]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[freebsd.org]; AUTH_NA(1.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_MEDIUM(-1.00)[-0.999]; NEURAL_HAM_SHORT(-0.81)[-0.808]; MLMMJ_DEST(0.00)[dev-commits-src-all,dev-commits-src-main]; FORGED_SENDER(0.30)[dchagin@freebsd.org,dchagin@heemeyer.club]; R_SPF_NA(0.00)[no SPF record]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:61400, ipnet:195.93.173.0/24, country:RU]; FROM_NEQ_ENVFROM(0.00)[dchagin@freebsd.org,dchagin@heemeyer.club]; RCVD_COUNT_TWO(0.00)[2] X-ThisMailContainsUnwantedMimeParts: N On Thu, May 19, 2022 at 03:48:42PM +0000, Justin Hibbits wrote: > The branch main has been updated by jhibbits: > > URL: https://cgit.FreeBSD.org/src/commit/?id=47a57144af25a7bd768b29272d50a36fdf2874ba > > commit 47a57144af25a7bd768b29272d50a36fdf2874ba > Author: Justin Hibbits > AuthorDate: 2022-05-12 20:38:22 +0000 > Commit: Justin Hibbits > CommitDate: 2022-05-19 15:49:55 +0000 > > cpuset: Byte swap cpuset for compat32 on big endian architectures > > Summary: > BITSET uses long as its basic underlying type, which is dependent on the > compile type, meaning on 32-bit builds the basic type is 32 bits, but on > 64-bit builds it's 64 bits. On little endian architectures this doesn't > matter, because the LSB is always at the low bit, so the words get > effectively concatenated moving between 32-bit and 64-bit, but on > big-endian architectures it throws a wrench in, as setting bit 0 in > 32-bit mode is equivalent to setting bit 32 in 64-bit mode. To > demonstrate: > > 32-bit mode: > > BIT_SET(foo, 0): 0x00000001 > > 64-bit sees: 0x0000000100000000 > > cpuset is the only system interface that uses bitsets, so solve this > by swapping the integer sub-components at the copyin/copyout points. > > Reviewed by: kib > MFC after: 3 days > Sponsored by: Juniper Networks, Inc. > Differential Revision: https://reviews.freebsd.org/D35225 > --- > sys/compat/freebsd32/freebsd32_misc.c | 70 +++++++++++++++++++++++++++++++++-- > sys/kern/kern_cpuset.c | 32 ++++++++++------ > sys/sys/cpuset.h | 9 +++++ > sys/sys/syscallsubr.h | 10 +++-- > 4 files changed, 101 insertions(+), 20 deletions(-) > > diff --git a/sys/compat/freebsd32/freebsd32_misc.c b/sys/compat/freebsd32/freebsd32_misc.c > index 25c7a4e17dc7..d4206fdd24aa 100644 > --- a/sys/compat/freebsd32/freebsd32_misc.c > +++ b/sys/compat/freebsd32/freebsd32_misc.c > @@ -3310,13 +3310,72 @@ freebsd32_cpuset_getid(struct thread *td, > PAIR32TO64(id_t, uap->id), uap->setid)); > } > > +static int > +copyin32_set(const void *u, void *k, size_t size) > +{ > +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ > + int rv; > + struct bitset *kb = k; > + int *p; > + > + rv = copyin(u, k, size); > + if (rv != 0) > + return (rv); > + > + p = (int *)kb->__bits; > + /* Loop through swapping words. > + * `size' is in bytes, we need bits. */ > + for (int i = 0; i < __bitset_words(size * 8); i++) { > + int tmp = p[0]; > + p[0] = p[1]; > + p[1] = tmp; > + p += 2; > + } > + return (0); > +#else > + return (copyin(u, k, size)); > +#endif > +} > + > +static int > +copyout32_set(const void *k, void *u, size_t size) > +{ > +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ > + const struct bitset *kb = k; > + struct bitset *ub = u; > + const int *kp = (const int *)kb->__bits; > + int *up = (int *)ub->__bits; > + int rv; > + > + for (int i = 0; i < __bitset_words(CPU_SETSIZE); i++) { > + /* `size' is in bytes, we need bits. */ > + for (int i = 0; i < __bitset_words(size * 8); i++) { > + rv = suword32(up, kp[1]); > + if (rv == 0) > + rv = suword32(up + 1, kp[0]); > + if (rv != 0) > + return (EFAULT); > + } > + } > + return (0); > +#else > + return (copyout(k, u, size)); > +#endif > +} > + > +static const struct cpuset_copy_cb cpuset_copy32_cb = { > + .copyin = copyin32_set, > + .copyout = copyout32_set > +}; > + > int > freebsd32_cpuset_getaffinity(struct thread *td, > struct freebsd32_cpuset_getaffinity_args *uap) > { > > return (kern_cpuset_getaffinity(td, uap->level, uap->which, > - PAIR32TO64(id_t,uap->id), uap->cpusetsize, uap->mask)); > + PAIR32TO64(id_t,uap->id), uap->cpusetsize, uap->mask, > + &cpuset_copy32_cb)); > } > > int > @@ -3325,7 +3384,8 @@ freebsd32_cpuset_setaffinity(struct thread *td, > { > > return (user_cpuset_setaffinity(td, uap->level, uap->which, > - PAIR32TO64(id_t,uap->id), uap->cpusetsize, uap->mask)); > + PAIR32TO64(id_t,uap->id), uap->cpusetsize, uap->mask, > + &cpuset_copy32_cb)); > } > > int > @@ -3334,7 +3394,8 @@ freebsd32_cpuset_getdomain(struct thread *td, > { > > return (kern_cpuset_getdomain(td, uap->level, uap->which, > - PAIR32TO64(id_t,uap->id), uap->domainsetsize, uap->mask, uap->policy)); > + PAIR32TO64(id_t,uap->id), uap->domainsetsize, uap->mask, uap->policy, > + &cpuset_copy32_cb)); > } > > int > @@ -3343,7 +3404,8 @@ freebsd32_cpuset_setdomain(struct thread *td, > { > > return (kern_cpuset_setdomain(td, uap->level, uap->which, > - PAIR32TO64(id_t,uap->id), uap->domainsetsize, uap->mask, uap->policy)); > + PAIR32TO64(id_t,uap->id), uap->domainsetsize, uap->mask, uap->policy, > + &cpuset_copy32_cb)); > } > > int > diff --git a/sys/kern/kern_cpuset.c b/sys/kern/kern_cpuset.c > index 56d92687a693..e21ec146bb00 100644 > --- a/sys/kern/kern_cpuset.c > +++ b/sys/kern/kern_cpuset.c > @@ -52,6 +52,7 @@ __FBSDID("$FreeBSD$"); > #include > #include > #include > +#include > #include > #include > #include > @@ -1742,6 +1743,11 @@ cpuset_check_capabilities(struct thread *td, cpulevel_t level, cpuwhich_t which, > return (0); > } > > +static const struct cpuset_copy_cb copy_set = { > + .copyin = copyin, > + .copyout = copyout > +}; > + > #ifndef _SYS_SYSPROTO_H_ > struct cpuset_args { > cpusetid_t *setid; > @@ -1883,12 +1889,12 @@ sys_cpuset_getaffinity(struct thread *td, struct cpuset_getaffinity_args *uap) > { > > return (kern_cpuset_getaffinity(td, uap->level, uap->which, > - uap->id, uap->cpusetsize, uap->mask)); > + uap->id, uap->cpusetsize, uap->mask, ©_set)); > } > > int > kern_cpuset_getaffinity(struct thread *td, cpulevel_t level, cpuwhich_t which, > - id_t id, size_t cpusetsize, cpuset_t *maskp) > + id_t id, size_t cpusetsize, cpuset_t *maskp, const struct cpuset_copy_cb *cb) > { this broke the build, as Linuxulator also uses kern_cpuset_getaffinity, may be we need to move out the userspace related code from kern_cpuset_getaffinity to user_cpuset_getaffinity? > struct thread *ttd; > struct cpuset *nset; > @@ -1977,7 +1983,7 @@ kern_cpuset_getaffinity(struct thread *td, cpulevel_t level, cpuwhich_t which, > goto out; > } > size = min(cpusetsize, sizeof(cpuset_t)); > - error = copyout(mask, maskp, size); > + error = cb->copyout(mask, maskp, size); > if (error != 0) > goto out; > if (cpusetsize > size) { > @@ -2021,7 +2027,7 @@ sys_cpuset_setaffinity(struct thread *td, struct cpuset_setaffinity_args *uap) > { > > return (user_cpuset_setaffinity(td, uap->level, uap->which, > - uap->id, uap->cpusetsize, uap->mask)); > + uap->id, uap->cpusetsize, uap->mask, ©_set)); > } > > int > @@ -2109,7 +2115,7 @@ kern_cpuset_setaffinity(struct thread *td, cpulevel_t level, cpuwhich_t which, > > int > user_cpuset_setaffinity(struct thread *td, cpulevel_t level, cpuwhich_t which, > - id_t id, size_t cpusetsize, const cpuset_t *maskp) > + id_t id, size_t cpusetsize, const cpuset_t *maskp, const struct cpuset_copy_cb *cb) > { > cpuset_t *mask; > int error; > @@ -2117,7 +2123,7 @@ user_cpuset_setaffinity(struct thread *td, cpulevel_t level, cpuwhich_t which, > > size = min(cpusetsize, sizeof(cpuset_t)); > mask = malloc(sizeof(cpuset_t), M_TEMP, M_WAITOK | M_ZERO); > - error = copyin(maskp, mask, size); > + error = cb->copyin(maskp, mask, size); > if (error) > goto out; > /* > @@ -2165,12 +2171,13 @@ sys_cpuset_getdomain(struct thread *td, struct cpuset_getdomain_args *uap) > { > > return (kern_cpuset_getdomain(td, uap->level, uap->which, > - uap->id, uap->domainsetsize, uap->mask, uap->policy)); > + uap->id, uap->domainsetsize, uap->mask, uap->policy, ©_set)); > } > > int > kern_cpuset_getdomain(struct thread *td, cpulevel_t level, cpuwhich_t which, > - id_t id, size_t domainsetsize, domainset_t *maskp, int *policyp) > + id_t id, size_t domainsetsize, domainset_t *maskp, int *policyp, > + const struct cpuset_copy_cb *cb) > { > struct domainset outset; > struct thread *ttd; > @@ -2268,7 +2275,7 @@ kern_cpuset_getdomain(struct thread *td, cpulevel_t level, cpuwhich_t which, > } > DOMAINSET_COPY(&outset.ds_mask, mask); > if (error == 0) > - error = copyout(mask, maskp, domainsetsize); > + error = cb->copyout(mask, maskp, domainsetsize); > if (error == 0) > if (suword32(policyp, outset.ds_policy) != 0) > error = EFAULT; > @@ -2292,12 +2299,13 @@ sys_cpuset_setdomain(struct thread *td, struct cpuset_setdomain_args *uap) > { > > return (kern_cpuset_setdomain(td, uap->level, uap->which, > - uap->id, uap->domainsetsize, uap->mask, uap->policy)); > + uap->id, uap->domainsetsize, uap->mask, uap->policy, ©_set)); > } > > int > kern_cpuset_setdomain(struct thread *td, cpulevel_t level, cpuwhich_t which, > - id_t id, size_t domainsetsize, const domainset_t *maskp, int policy) > + id_t id, size_t domainsetsize, const domainset_t *maskp, int policy, > + const struct cpuset_copy_cb *cb) > { > struct cpuset *nset; > struct cpuset *set; > @@ -2318,7 +2326,7 @@ kern_cpuset_setdomain(struct thread *td, cpulevel_t level, cpuwhich_t which, > return (error); > memset(&domain, 0, sizeof(domain)); > mask = malloc(domainsetsize, M_TEMP, M_WAITOK | M_ZERO); > - error = copyin(maskp, mask, domainsetsize); > + error = cb->copyin(maskp, mask, domainsetsize); > if (error) > goto out; > /* > diff --git a/sys/sys/cpuset.h b/sys/sys/cpuset.h > index 0ce470d5f569..4f55cdc27103 100644 > --- a/sys/sys/cpuset.h > +++ b/sys/sys/cpuset.h > @@ -154,6 +154,15 @@ struct prison; > struct proc; > struct thread; > > +/* > + * Callbacks for copying in/out a cpuset or domainset. Used for alternate > + * ABIs, like compat32. > + */ > +struct cpuset_copy_cb { > + int (*copyin)(const void *, void *, size_t); > + int (*copyout)(const void *, void *, size_t); > +}; > + > struct cpuset *cpuset_thread0(void); > struct cpuset *cpuset_ref(struct cpuset *); > void cpuset_rel(struct cpuset *); > diff --git a/sys/sys/syscallsubr.h b/sys/sys/syscallsubr.h > index 294539c6593c..70e33e9244c4 100644 > --- a/sys/sys/syscallsubr.h > +++ b/sys/sys/syscallsubr.h > @@ -39,6 +39,7 @@ > #include > > struct __wrusage; > +struct cpuset_copy_cb; > struct file; > struct filecaps; > enum idtype; > @@ -119,18 +120,19 @@ int kern_connectat(struct thread *td, int dirfd, int fd, > int kern_copy_file_range(struct thread *td, int infd, off_t *inoffp, > int outfd, off_t *outoffp, size_t len, unsigned int flags); > int kern_cpuset_getaffinity(struct thread *td, cpulevel_t level, > - cpuwhich_t which, id_t id, size_t cpusetsize, cpuset_t *maskp); > + cpuwhich_t which, id_t id, size_t cpusetsize, cpuset_t *maskp, > + const struct cpuset_copy_cb *cb); > int kern_cpuset_setaffinity(struct thread *td, cpulevel_t level, > cpuwhich_t which, id_t id, cpuset_t *maskp); > int user_cpuset_setaffinity(struct thread *td, cpulevel_t level, > cpuwhich_t which, id_t id, size_t cpusetsize, > - const cpuset_t *maskp); > + const cpuset_t *maskp, const struct cpuset_copy_cb *cb); > int kern_cpuset_getdomain(struct thread *td, cpulevel_t level, > cpuwhich_t which, id_t id, size_t domainsetsize, > - domainset_t *maskp, int *policyp); > + domainset_t *maskp, int *policyp, const struct cpuset_copy_cb *cb); > int kern_cpuset_setdomain(struct thread *td, cpulevel_t level, > cpuwhich_t which, id_t id, size_t domainsetsize, > - const domainset_t *maskp, int policy); > + const domainset_t *maskp, int policy, const struct cpuset_copy_cb *cb); > int kern_cpuset_getid(struct thread *td, cpulevel_t level, > cpuwhich_t which, id_t id, cpusetid_t *setid); > int kern_cpuset_setid(struct thread *td, cpuwhich_t which,