From nobody Tue Jul 18 13:49:27 2023 X-Original-To: dev-commits-src-all@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 4R50gN3JkSz4mxTN; Tue, 18 Jul 2023 13:49:36 +0000 (UTC) (envelope-from jhibbits@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (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-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4R50gN2mQ9z3ldK; Tue, 18 Jul 2023 13:49:36 +0000 (UTC) (envelope-from jhibbits@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1689688176; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3ejVLOQIXh+dPXa1Sy31TIDFFs8XY4bLwd02rgTzEjQ=; b=vGzUxPEiaq2/W0U4+ER++bovQZSsQPs4rf0T5N5D63eRwduo46hrfd6EK9AHgY0mNg+ofX kpPgKunnoFhPd2gQEOswwVz2JSPPE1JZYfKTwIVTKYyi9BqvDvSOjbnrE3Xo+82iiHbVwb tdmgcNb/CO/UXkNLxkZ2RamMZ4fsioC8F+ke5lUaYQ+hdod6+KYbts/esOl0EnGMmgcbJA A+MLdLVOMzR2NLDSClb/H0yIrLoYm4AvKI91j1KS6k2+P3UKOVLoVGFsofaFZa0STTg7Pk eK3vGum4sxuOdM4e3eRNBOu9IItXGvfadB8wFggNkUMoZeuk++D/E5KFCpZd6w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1689688176; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3ejVLOQIXh+dPXa1Sy31TIDFFs8XY4bLwd02rgTzEjQ=; b=x96J3LyXxBLhFihvMkEVZFcO7Y4ZZ6Cx6jT8c7KeKfb6bHIk0US4ggZzY/g9qDxaiApvb5 UpEoKHb40/iSIPga2jUAEdOVjZhG4egkO1ib9pneQETQLizytNYQWrSkY388xJhFz7pPfj 9CyCRthB58TNQ99xwbjOF0oVFH+ExszE73GUjnlvVvWTK9kmuxoJUsAAaKfnOCpSmXSEii +mMMk6owD1s2Js6tmr012i67WvCEa/Nq9qv58xU7dtUPgj5zZRJv6mMvzHZA9iU8UtPVXH b4zl/mNgVjAB0IEYnwBuAATt4p60EWzcoMeZzC+VVxsKBPK31b6RC1zbbdQULw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1689688176; a=rsa-sha256; cv=none; b=BuCO4wS0LTwko/IfpShgon4gHDoaPCQdS7R/sjiaKGC2epi1mbpS3Lg7p2XZDnQIGMHccS MS1C6k4jZmSNzu3tADpLTgLxsv6hnGutDnwa0uYdccohoUz5uET7IBn1mdfWnPtoChnOOy hZnfK13g9xYXZK3tIf5UH5BP71xIHWhUlY3QoBpzL0OPPKj8n7sSjE2DRYA/bpMAdg4UOw BP1onBJ5xBJP93P62eyIl4dXnRBcSZpKFk7Ke1n9tME9ctxhDhXmc9cmV92vborE4N9Lbv meJVenznnx/WGxfS++UMyZA7bYltjS5vdjpL+7+ODjJicUXrqWFWQ03WJefWyA== Received: from ralga.knownspace (ip-163-182-7-56.dynamic.fuse.net [163.182.7.56]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: jhibbits) by smtp.freebsd.org (Postfix) with ESMTPSA id 4R50gM6xJwzWTl; Tue, 18 Jul 2023 13:49:35 +0000 (UTC) (envelope-from jhibbits@FreeBSD.org) Date: Tue, 18 Jul 2023 09:49:27 -0400 From: Justin Hibbits To: Jessica Clarke Cc: Ka Ho Ng , "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Subject: Re: git: 034c0856018c - main - modules: fix freebsd32_modstat on big endian platforms Message-ID: <20230718094927.1474f972@ralga.knownspace> In-Reply-To: References: <202307070424.3674OBaa074389@gitrepo.freebsd.org> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.37; powerpc64le-unknown-linux-gnu) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Fri, 7 Jul 2023 18:59:41 +0100 Jessica Clarke wrote: > On 7 Jul 2023, at 05:24, Ka Ho Ng wrote: > >=20 > > The branch main has been updated by khng: > >=20 > > URL: > > https://cgit.FreeBSD.org/src/commit/?id=3D034c0856018ccf73c0f2d6140825f= 3edf43f47b2 > >=20 > > commit 034c0856018ccf73c0f2d6140825f3edf43f47b2 > > Author: Ka Ho Ng > > AuthorDate: 2023-07-07 04:21:01 +0000 > > Commit: Ka Ho Ng > > CommitDate: 2023-07-07 04:22:59 +0000 > >=20 > > modules: fix freebsd32_modstat on big endian platforms > >=20 > > The layout of modspecific_t on both little endian and big endian > > are as follows: > > |0|1|2|3|4|5|6|7| > > +-------+-------+ > > |uintval| | > > +-------+-------+ > > |ulongval | > > +-------+-------+ > >=20 > > For the following code snippet: > > CP(mod->data, data32, longval); > > CP(mod->data, data32, ulongval); > > It only takes care of little endian platforms that it truncates > > the highest 32bit automatically. However on big endian platforms it > > takes the highest 32bit instead. This eventually returns a garbage > > syscall number to the 32bit userland. =20 >=20 > This fixes the case where uintval is the active member, but breaks the > case where ulongval is the active member, since it=E2=80=99ll take bytes = 0, 1, > 2 and 3 which are the high bytes for BE, not the low bytes. This is an > intrinsic issue with BE; LE is very permissive when you access the > wrong union member for different-width integer types, but BE is not > and you have to access the right one. How does userspace know which > member to access, anyway? The kernel needs to repeat that and copy > the right member based on that information, otherwise it is > inherently impossible to have both cases work on BE. >=20 > > Since modspecific_t's usage currently is for the use of syscall > > modules, we only initialize modspecific32_t with uintval. =20 >=20 > Is this saying that uintval is always the active member, never > ulongval? If so, there need to be comments, both in freebsd32_modstat > to justify this and on the type to say =E2=80=9CDO NOT USE (u)longval=E2= =80=9D, > otherwise there is a significant risk someone will start using > (u)longval and we=E2=80=99ll be stuck. Perhaps (u)longval should even be > renamed to be ABI-compatibility padding (and aligning). >=20 > Jess A better solution is to make these fields fixed size, so we don't run into this problem again. Is this feasible, factoring in any other consumers? - Justin >=20 > > Now on both BE and LE > > 64-bit platforms it always pick up the first 4 bytes. > >=20 > > Sponsored by: Juniper Networks, Inc. > > Reviewed by: markj > > Differential Revision: https://reviews.freebsd.org/D40814 > > MFC after: 1 week > > --- > > sys/kern/kern_module.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > >=20 > > diff --git a/sys/kern/kern_module.c b/sys/kern/kern_module.c > > index 40e47868481c..d4edc64a5ce4 100644 > > --- a/sys/kern/kern_module.c > > +++ b/sys/kern/kern_module.c > > @@ -520,10 +520,9 @@ freebsd32_modstat(struct thread *td, struct > > freebsd32_modstat_args *uap) id =3D mod->id; > > refs =3D mod->refs; > > name =3D mod->name; > > - CP(mod->data, data32, intval); > > + _Static_assert(sizeof(data32) =3D=3D sizeof(data32.uintval), > > + "bad modspecific32_t size"); > > CP(mod->data, data32, uintval); > > - CP(mod->data, data32, longval); > > - CP(mod->data, data32, ulongval); > > MOD_SUNLOCK; > > stat32 =3D uap->stat; > > =20