From nobody Fri Jul 07 17:59:41 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 4QyLlG0YHHz4l5hJ for ; Fri, 7 Jul 2023 17:59:54 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: from mail-wm1-f47.google.com (mail-wm1-f47.google.com [209.85.128.47]) (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 4QyLlF5x0Yz3F4b for ; Fri, 7 Jul 2023 17:59:53 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-wm1-f47.google.com with SMTP id 5b1f17b1804b1-3fbc656873eso27459975e9.1 for ; Fri, 07 Jul 2023 10:59:53 -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=jqOLgZz4C47OeAzYAsWUYZtasOcHZh7LRr/p0Y9d4MyzGV2yCN5YgVly/ZC5ctfE/v PtiN1X7Y7l2Kss6edrmsAFJ8lmWViFrqBptuZAVE203cE+rGEIFKCvxHlQKyI2QllO1u EXSbs5mZLa519HzZK+szY4VH0/Y06X8Sk5qU8R+0j1P4UzSHcD11O4iZouZTNrRP3gQJ S9uls8RS9TAZeLrySLNzO1CO9z3eyi2RDU7ZjnIlDwzrLMmw/1MVxjKvqmEygqIyco9y pHiTbAeKMVQfsY4gOIdmWls9YvIlRwtq1Y6x9XB2lTlr/ZbPPl1CLOexQKbVko7BMD2Y LfsQ== X-Gm-Message-State: ABy/qLY6e3NfpbTN4w1rvt2MwEHFj3XH3zy8Zd1V7+mnrdEVoXrUD7dS L2azcEEXxCNsSUonwUTHGfkCRQ== 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 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 (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: 4QyLlF5x0Yz3F4b 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