From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 41AB7361640 for ; Fri, 21 Aug 2026 21:56:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787349384; cv=none; b=rYMDFCSrvZy3cdcXYVTE8kXXo01DNOTmJIWw3mCbGWWEp9ynPlLxaFZLJ9qrOsYgeDSUslfRNBtspASe4QFZWezCJWtzJPavh4rluQgKAV9JB3S5NApqzwUyv07B+WxlTH4hR6u/48UybBqmZnRy1h0gvKxFnsPDZG0kCKXZenQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787349384; c=relaxed/simple; bh=ZahgwlH6DLRqHzEjMClIUNRGVT/2LmF8sOANfcAyfKM=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=YigkAa2g1B+w9gw+Dclg0NAjl2CVozInbOPV6KRAihean/gN89oPFSDJNb12wBHxARgiYzZ+rwFHnsXF1HJVXt0mvs588dpR4Qgd5oudO7saHJMbyg2PfjkhRHeMCTi9xJmeNjTLg4kZhTHeHnjpVtp76H2RvH9Kt4k49fbjVJI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=KFkPKmhV; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="KFkPKmhV" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A47681F000E9; Fri, 21 Aug 2026 21:56:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1787349381; bh=EPIGmxKIFNmlx+3y0uGr0GYT2tvPAfi15bHjJnBM2D4=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=KFkPKmhVkHfGmqG/2WR/XU0q+6EyscdIG3NYd72BrxEgiJMz7D4bFS9yvRunslbJF 7yONhaYOmYA/T4OjYGINUS6cK3MQTOsMnnDltr7CO2XyOPshMpqOkHQNefaJNz7uZA HF2ivLHkQGNUFa4bauDsaR5kRqDtVR1Js40qoSTQofH/5m7E+gAGw2o/NRHG55CaUc CEpAcwQTYD1VVrNIblysV84dOI43FA6XSbURbnBEzxCOAjk/cF4PtbqtIvcc7T5w1s 4w9Ccv/ILqWVlPViYrl73vwnCJnsEHNONFF1SZDVisqU57Cdh4aYeuU7T6697sX+Ep DyLJdPq43Rasw== From: sashiko-bot@kernel.org Subject: Re: [PATCH bpf-next 01/11] bpf: Add a bpf keyring for program signature validation Reply-To: sashiko-reviews@lists.linux.dev To: "Daniel Borkmann" Cc: bpf@vger.kernel.org In-Reply-To: <20260821214111.1120748-2-daniel@iogearbox.net> References: <20260821214111.1120748-1-daniel@iogearbox.net> <20260821214111.1120748-2-daniel@iogearbox.net> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Fri, 21 Aug 2026 21:56:21 +0000 Message-Id: <20260821215621.A47681F000E9@smtp.kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: - [Low] `VERIFY_USE_BPF_KEYRING` aliases a valid user keyring ID, making us= er keyring ID 3 unusable for BPF signature verification. -- --- Patch [1]: [PATCH bpf-next 01/11] bpf: Add a bpf keyring for program si= gnature validation --- commit b7fa8763dbdc44199a6b443ce9ddca2d84edc3b7 Author: Daniel Borkmann bpf: Add a bpf keyring for program signature validation This commit adds a dedicated bpf keyring for program signature validation, selectable via a new well-known keyring_id VERIFY_USE_BPF_KEYRING. The keyring is sealed empty at init but can be left open for provisioning via a boot parameter. > diff --git a/include/linux/verification.h b/include/linux/verification.h > index dec7f2beabfd4..1cb59ddda2509 100644 > --- a/include/linux/verification.h > +++ b/include/linux/verification.h > @@ -18,6 +18,16 @@ > #define VERIFY_USE_SECONDARY_KEYRING ((struct key *)1UL) > #define VERIFY_USE_PLATFORM_KEYRING ((struct key *)2UL) > =20 > +/* > + * The id of BPF's ".bpf" keyring, reserved from the same space. It is > + * explicitly not a sentinel like the two above as BPF resolves it to > + * the keyring itself and passes that, so verify_pkcs7_signature() never > + * sees this value, and system_keyring_id_check() must keep rejecting it. > + * Left as a plain integer so that handing it over as @trusted_keys does > + * not compile. > + */ > +#define VERIFY_USE_BPF_KEYRING 3 [Severity: Low] Does this constant overlap with the valid user keyring ID space? Since user keyring IDs generated by key_alloc_serial() are positive integers that can start from 3 (avoiding 0, 1, and 2), 3 is technically a valid, assignable user keyring ID. [ ... ] > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 16de43f172382..8075bdbee6254 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -20908,10 +20910,17 @@ static int bpf_prog_verify_signature(struct bpf= _verifier_env *env, > if (!attr->signature_size || > attr->signature_size > KMALLOC_MAX_CACHE_SIZE) > return -EINVAL; > - if (system_keyring_id_check(attr->keyring_id) =3D=3D 0) > + if (attr->keyring_id =3D=3D VERIFY_USE_BPF_KEYRING) { > + key =3D bpf_lookup_keyring(); [Severity: Low] If a user happens to generate a keyring with ID 3 and attempts to use it for BPF signature verification here in bpf_prog_verify_signature(), wouldn't this code incorrectly resolve it to the system .bpf keyring instead of their intended user keyring? While the probability of receiving serial ID 3 is very small, it appears to create a structural API design flaw because a sentinel value intercepts a valid user-space ID. > + if (!key) { > + verbose(env, "the bpf keyring is empty or has not been restricted\n"); > + return -ENOKEY; > + } > + } else if (system_keyring_id_check(attr->keyring_id) =3D=3D 0) { > key =3D bpf_lookup_system_key(attr->keyring_id); --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260821214111.1120= 748-1-daniel@iogearbox.net?part=3D1