From nobody Fri Jun 23 15:56:18 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 4QnhgB5cX3z4h9q7; Fri, 23 Jun 2023 15:56:22 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-qv1-xf32.google.com (mail-qv1-xf32.google.com [IPv6:2607:f8b0:4864:20::f32]) (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 4QnhgB15Z6z3s6Y; Fri, 23 Jun 2023 15:56:22 +0000 (UTC) (envelope-from markjdb@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-qv1-xf32.google.com with SMTP id 6a1803df08f44-630066deb1dso7040936d6.3; Fri, 23 Jun 2023 08:56:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1687535781; x=1690127781; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:sender :from:to:cc:subject:date:message-id:reply-to; bh=Z8CwAC52BNxqoHlvatFK3KTTTVBkt105pThXVgOr7j4=; b=jSmC8x6Xgb2lvq4uGNQcCjjhtnBRaLlcIpyKB2qCrHlezClSZJi+efxEQcQkF375e0 iZ8beCWXzjiYtRf7ks/ExN9LqLY0WDymHsSRGmTmwvAZVPeSPkjzbij9TjrOVXwmy+KN g3+rOwh5G1p/BcQGCiYL+T5t4itlfVwrCz/auEliR9AhUlEJbDkXF0Y+7h4vCCBF9MEd xAwImkO/wVBmUNkZfU7rcrwUnYKzvXK/gczRrGP5dqM39Z9TwDAMlN0uWhiaqnC/9lTN ln0WkdifeneaDl9Zs9CJjXUTEvOZBxIn4+penh6ECjRgB7D/896izkjbXZmi5AJ++6+S /u2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687535781; x=1690127781; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:sender :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Z8CwAC52BNxqoHlvatFK3KTTTVBkt105pThXVgOr7j4=; b=lvEeUXeyy63EqWdpWTlq3uvZNV3OH0ulz55q7FM0DGkDlHVCuoZM5L2cIrBEDL5s1P i36jdD3uMMzzXKCJmgvbayZXEjUgXBYRLAmy4OddZ63fx3DXloup919MHJJYL08854uV Jlxe3PWzXzf2dnJUFm6rHSiWVEptdAEN9RN4ownhQ3xYE48y2eHThCgeqzR0OmEBqEjo nbXnB6EOVlYj7eEi0ADcIRDs3mX7bwPEwXxFq7EtA4f+whkRC3dBuizXWPMld4bIHAh2 ObBsOz/O8StdMN36nmkQOB3shDQXEbsMhQhICK58+5ygMOruI7UmIXiZqRcnK+c8VnXN hKcw== X-Gm-Message-State: AC+VfDy4tLxwN+5kPbGyfFawTRfoD2OKukPcMKHWy1MP2/h7wDGq3NCg ItbVRFevZqR34md9v21gqOvBpeWCb7k= X-Google-Smtp-Source: ACHHUZ4zbzA1sBYtI60ACQDXYmcbnYRTJbl4N43da1HrN2Jr3CNWIAjR58bi5lwB9DWMfS8A8meNBg== X-Received: by 2002:a05:6214:1303:b0:626:2bf5:d532 with SMTP id pn3-20020a056214130300b006262bf5d532mr31159418qvb.14.1687535781232; Fri, 23 Jun 2023 08:56:21 -0700 (PDT) Received: from nuc (192-0-220-237.cpe.teksavvy.com. [192.0.220.237]) by smtp.gmail.com with ESMTPSA id a15-20020a0ce38f000000b00626195bdbbdsm5152861qvl.132.2023.06.23.08.56.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 23 Jun 2023 08:56:20 -0700 (PDT) Date: Fri, 23 Jun 2023 11:56:18 -0400 From: Mark Johnston To: Enji Cooper Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: fc915f1be145 - main - pseudofs: Fix a potential out-of-bounds access in pfs_lookup() Message-ID: References: <202306231509.35NF9sAk037726@gitrepo.freebsd.org> <0BAC85B7-6A67-4F6E-87B8-97ABD2FF7075@gmail.com> 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=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <0BAC85B7-6A67-4F6E-87B8-97ABD2FF7075@gmail.com> X-Rspamd-Queue-Id: 4QnhgB15Z6z3s6Y X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N On Fri, Jun 23, 2023 at 08:38:44AM -0700, Enji Cooper wrote: > > > On Jun 23, 2023, at 08:09, Mark Johnston wrote: > > > > The branch main has been updated by markj: > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=fc915f1be145a52c53f6f1c37525043216e32bb8 > > > > commit fc915f1be145a52c53f6f1c37525043216e32bb8 > > Author: Mark Johnston > > AuthorDate: 2023-06-23 13:54:39 +0000 > > Commit: Mark Johnston > > CommitDate: 2023-06-23 13:54:39 +0000 > > > > pseudofs: Fix a potential out-of-bounds access in pfs_lookup() > > > > pseudofs nodes store their name in a flexible array member, so the node > > allocation is sized using the length of the name, including a nul > > terminator. pfs_lookup() scans a directory of nodes, comparing names to > > find a match. The comparison was incorrect and assumed that all node > > names were at least as long as the name being looked up, which of course > > isn't true. > > > > I believe the bug is mostly harmless since it cannot result in false > > positive or negative matches from the lookup, but it triggers a KASAN > > check. > > > > Reported by: pho > > Reviewed by: kib, Olivier Certner > > MFC after: 2 weeks > > Sponsored by: The FreeBSD Foundation > > Differential Revision: https://reviews.freebsd.org/D40692 > > --- > > sys/fs/pseudofs/pseudofs_vnops.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/sys/fs/pseudofs/pseudofs_vnops.c b/sys/fs/pseudofs/pseudofs_vnops.c > > index 53e4c2b6b85c..bf423f0ad4db 100644 > > --- a/sys/fs/pseudofs/pseudofs_vnops.c > > +++ b/sys/fs/pseudofs/pseudofs_vnops.c > > @@ -537,8 +537,8 @@ pfs_lookup(struct vop_cachedlookup_args *va) > > for (pn = pd->pn_nodes; pn != NULL; pn = pn->pn_next) > > if (pn->pn_type == pfstype_procdir) > > pdn = pn; > > - else if (pn->pn_name[namelen] == '\0' && > > - bcmp(pname, pn->pn_name, namelen) == 0) { > > + else if (strncmp(pname, pn->pn_name, namelen) == 0 && > > + pn->pn_name[namelen] == '\0') { > > pfs_unlock(pd); > > goto got_pnode; > > } > > Naive question: should this be an && conditional or an || conditional? It should be &&. Using || here would reintroduce the original bug. If strncmp(pname, pn->pn_name, namelen) == 0, then strlen(pn->pn_name) >= namelen, and pn->pn_name is nul-terminated, so it is safe to check pn->pn_name[namelen] == '\0'. > If the former, could this be simplified by using a direct NUL char equality check instead of using strncmp? I'm not sure what you mean by this. This code is simply checking whether pname and pn->pn_name are the same string, without assuming that pname is nul-terminated.