From nobody Fri Jul 07 17:59:41 2023 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 4QyLlH4BWgz4l5hP for ; Fri, 7 Jul 2023 17:59:55 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: from mail-wr1-f41.google.com (mail-wr1-f41.google.com [209.85.221.41]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4QyLlG0SHxz3Dlp for ; Fri, 7 Jul 2023 17:59:54 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-wr1-f41.google.com with SMTP id ffacd0b85a97d-3090d3e9c92so2451564f8f.2 for ; Fri, 07 Jul 2023 10:59:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688752792; x=1691344792; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=7ndXPBcia3gI1RNYT3jPzR6yIteT0dopvDBiYQ6dv3A=; b=J+U9a4AsuFxQR38dv9Z0RgsPZemCIt3Q9Qohpt5G09V1bzFoQs+UkwcIvLSTzPvtqm MFfQQHn+RcmsyG+62qSAicMlCGkglyXaR2mUeZGeh+FvvnF1etvsbPtHqafSG8QSn4Cd CNPYO/8cdMIegNl+VrGX/xpfudOdryvuTwTMvr84Z4VThrB6kGNzYXLibArsAdV8nOpk zpCEq+8UB6cpfVdeX7n8d/6sD4PrqHJ7lhYc8yObmdUXxllY0erRhpkp77NTJekv7QW5 EQAo2ZFIWUqmxq3tMFhKyIUYrh0B075rGfZTOFL7XQj5LM4iA62Ybkv8zKUh3bVCE6Hj eu5A== X-Gm-Message-State: ABy/qLbFv4m/Oz79IoadD2MPvNtploebxI5hJoQfxO0C/ciclQoCuvvy iFJ8bVEOcfQg58t0WjO76ZBFJg== X-Google-Smtp-Source: APBJJlFzmu5R1T9NBK74lzuPugNAhb0c0iV21DQKbdmdcupneG8dJJVYFR2ZSTnCu862Hb+bqsMO1A== X-Received: by 2002:adf:ea10:0:b0:314:77a:c2b9 with SMTP id q16-20020adfea10000000b00314077ac2b9mr6244348wrm.39.1688752792107; Fri, 07 Jul 2023 10:59:52 -0700 (PDT) Received: from smtpclient.apple ([131.111.5.246]) by smtp.gmail.com with ESMTPSA id p5-20020adff205000000b003062b2c5255sm5005982wro.40.2023.07.07.10.59.51 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 07 Jul 2023 10:59:51 -0700 (PDT) Content-Type: text/plain; charset=utf-8 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 (Mac OS X Mail 16.0 \(3731.600.7\)) Subject: Re: git: 034c0856018c - main - modules: fix freebsd32_modstat on big endian platforms From: Jessica Clarke In-Reply-To: <202307070424.3674OBaa074389@gitrepo.freebsd.org> Date: Fri, 7 Jul 2023 18:59:41 +0100 Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: References: <202307070424.3674OBaa074389@gitrepo.freebsd.org> To: Ka Ho Ng X-Mailer: Apple Mail (2.3731.600.7) X-Rspamd-Queue-Id: 4QyLlG0SHxz3Dlp X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N 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=3D034c0856018ccf73c0f2d6140825f3ed= f43f47b2 >=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. 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. > Since modspecific_t's usage currently is for the use of syscall = modules, > we only initialize modspecific32_t with uintval. 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). Jess > 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