* Re: [PATCH RFC 5/15] LSM: Single calls in secid hooks
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-6-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> security_socket_getpeersec_stream(), security_socket_getpeersec_dgram()
> and security_secctx_to_secid() can only provide a single security context
> or secid to their callers. Open code these hooks to return the first
> hook provided. Because only one "major" LSM is allowed there will only
> be one hook in the list, with the excepton being BPF. BPF is not expected
> to be using these interfaces.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> security/security.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/security/security.c b/security/security.c
> index db85006d2fd5..2286285f8aea 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -3806,8 +3806,13 @@ EXPORT_SYMBOL(security_lsmprop_to_secctx);
> */
> int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
> {
> + struct lsm_static_call *scall;
> +
> *secid = 0;
> - return call_int_hook(secctx_to_secid, secdata, seclen, secid);
> + lsm_for_each_hook(scall, secctx_to_secid) {
> + return scall->hl->hook.secctx_to_secid(secdata, seclen, secid);
> + }
> + return LSM_RET_DEFAULT(secctx_to_secid);
> }
> EXPORT_SYMBOL(security_secctx_to_secid);
Two thoughts come to mind:
If we are relying on BPF not using these hooks we should remove the BPF
callback. It looks like the secctx_to_secid and socket_getpeersec_stream
callbacks are already absent from the BPF LSM, so it's just a matter of
working with the BPF folks to see if socket_getpeersec_dgram can be
removed. If it can't be removed, you'll need to find another solution.
Instead of opening up the call_int_hook() wrapper here, what would it
look like if we enforced the single callback rule at LSM registration
time?
> @@ -4268,8 +4273,13 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
> int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval,
> sockptr_t optlen, unsigned int len)
> {
> - return call_int_hook(socket_getpeersec_stream, sock, optval, optlen,
> - len);
> + struct lsm_static_call *scall;
> +
> + lsm_for_each_hook(scall, socket_getpeersec_stream) {
> + return scall->hl->hook.socket_getpeersec_stream(sock, optval,
> + optlen, len);
> + }
> + return LSM_RET_DEFAULT(socket_getpeersec_stream);
> }
>
> /**
> @@ -4289,7 +4299,13 @@ int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval,
> int security_socket_getpeersec_dgram(struct socket *sock,
> struct sk_buff *skb, u32 *secid)
> {
> - return call_int_hook(socket_getpeersec_dgram, sock, skb, secid);
> + struct lsm_static_call *scall;
> +
> + lsm_for_each_hook(scall, socket_getpeersec_dgram) {
> + return scall->hl->hook.socket_getpeersec_dgram(sock, skb,
> + secid);
> + }
> + return LSM_RET_DEFAULT(socket_getpeersec_dgram);
> }
> EXPORT_SYMBOL(security_socket_getpeersec_dgram);
>
> --
> 2.47.0
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 4/15] Audit: Add record for multiple object contexts
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-5-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Create a new audit record AUDIT_MAC_OBJ_CONTEXTS.
> An example of the MAC_OBJ_CONTEXTS record is:
>
> type=MAC_OBJ_CONTEXTS
> msg=audit(1601152467.009:1050):
> obj_selinux=unconfined_u:object_r:user_home_t:s0
>
> When an audit event includes a AUDIT_MAC_OBJ_CONTEXTS record
> the "obj=" field in other records in the event will be "obj=?".
> An AUDIT_MAC_OBJ_CONTEXTS record is supplied when the system has
> multiple security modules that may make access decisions based
> on an object security context.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/audit.h | 7 +++++
> include/uapi/linux/audit.h | 1 +
> kernel/audit.c | 58 +++++++++++++++++++++++++++++++++++++-
> kernel/auditsc.c | 45 ++++++++---------------------
> security/selinux/hooks.c | 3 +-
> security/smack/smack_lsm.c | 3 +-
> 6 files changed, 80 insertions(+), 37 deletions(-)
Similar to patch 1/15, dropped due to this already being in Linus'
tree.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 3/15] Audit: Add record for multiple task security contexts
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-4-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Replace the single skb pointer in an audit_buffer with a list of
> skb pointers. Add the audit_stamp information to the audit_buffer as
> there's no guarantee that there will be an audit_context containing
> the stamp associated with the event. At audit_log_end() time create
> auxiliary records as have been added to the list. Functions are
> created to manage the skb list in the audit_buffer.
>
> Create a new audit record AUDIT_MAC_TASK_CONTEXTS.
> An example of the MAC_TASK_CONTEXTS record is:
>
> type=MAC_TASK_CONTEXTS
> msg=audit(1600880931.832:113)
> subj_apparmor=unconfined
> subj_smack=_
>
> When an audit event includes a AUDIT_MAC_TASK_CONTEXTS record the
> "subj=" field in other records in the event will be "subj=?".
> An AUDIT_MAC_TASK_CONTEXTS record is supplied when the system has
> multiple security modules that may make access decisions based on a
> subject security context.
>
> Refactor audit_log_task_context(), creating a new audit_log_subj_ctx().
> This is used in netlabel auditing to provide multiple subject security
> contexts as necessary.
>
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/audit.h | 16 +++
> include/uapi/linux/audit.h | 1 +
> kernel/audit.c | 207 +++++++++++++++++++++++++++++------
> net/netlabel/netlabel_user.c | 9 +-
> security/apparmor/lsm.c | 3 +
> security/lsm.h | 4 -
> security/lsm_init.c | 5 -
> security/security.c | 3 -
> security/selinux/hooks.c | 3 +
> security/smack/smack_lsm.c | 3 +
> 10 files changed, 202 insertions(+), 52 deletions(-)
Similar to patch 1/15, dropped due to this already being in Linus'
tree.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 2/15] LSM: security_lsmblob_to_secctx module selection
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-3-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Add a parameter lsmid to security_lsmblob_to_secctx() to identify which
> of the security modules that may be active should provide the security
> context. If the value of lsmid is LSM_ID_UNDEF the first LSM providing
> a hook is used. security_secid_to_secctx() is unchanged, and will
> always report the first LSM providing a hook.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/security.h | 6 ++++--
> kernel/audit.c | 4 ++--
> kernel/auditsc.c | 8 +++++---
> net/netlabel/netlabel_user.c | 3 ++-
> security/security.c | 13 +++++++++++--
> 5 files changed, 24 insertions(+), 10 deletions(-)
Similar to patch 1/15, dropped due to this already being in Linus'
tree.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 1/15] Audit: Create audit_stamp structure
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-2-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Replace the timestamp and serial number pair used in audit records
> with a structure containing the two elements.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> kernel/audit.c | 17 +++++++++--------
> kernel/audit.h | 13 +++++++++----
> kernel/auditsc.c | 22 +++++++++-------------
> 3 files changed, 27 insertions(+), 25 deletions(-)
Dropped as this patch was merged into Linus' tree during the v6.18
merge window via another patchset. To be clear, I generally don't have
a problem with multiple patchsets including a few common patches, it
helps prevent cross-dependencies between patchsets which is a good
thing.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH 2/2] LSM: Allow reservation of netlabel
From: Paul Moore @ 2025-10-13 22:21 UTC (permalink / raw)
To: Casey Schaufler
Cc: Stephen Smalley, linux-security-module, jmorris, serge, keescook,
john.johansen, penguin-kernel, linux-kernel, selinux
In-Reply-To: <01879779-d529-40f2-8693-257cc598dcd7@schaufler-ca.com>
On Fri, Oct 10, 2025 at 5:11 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 10/10/2025 12:53 PM, Stephen Smalley wrote:
> > On Fri, Oct 10, 2025 at 11:09 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 10/9/2025 11:53 AM, Stephen Smalley wrote:
> >>> On Wed, Oct 1, 2025 at 5:56 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
...
> > But some security modules may not function correctly (or at all) if
> > secmark and/or netlabel are silently disabled on them, and the end
> > user needs a better way to express intent.
This is the point I was trying to make in patch 1/2 with secmarks, but
Stephen has captured the idea much better in the sentence above. To
be clear, the argument applies to both secmarks and NetLabel.
> I'm open to suggestions. Would boot options lsm.secmark and lsm.netlabel
> be sufficient to address your concern?
No. Please no. We already have two LSM initialization related
command line parameters, and one of them is pretty broken and very
confusing in the new world of multiple LSMs (as an aside, does someone
want to kick off the work to deprecate "security=?"). Maybe we have
to go this route eventually, but let's keep it simple for right now; I
don't want to add a lot of user-visible APIs for something that is
pretty niche.
If you absolutely can't live with the "first one gets it" approach,
look at the no/wants/must idea in my patch 1/2 comments. It would
require work in the individual LSMs to support it, but I'd rather try
that route first.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3] memfd,selinux: call security_inode_init_security_anon
From: Thiébaud Weksteen @ 2025-10-13 22:19 UTC (permalink / raw)
To: Paul Moore
Cc: Stephen Smalley, Hugh Dickins, James Morris, Jeff Vander Stoep,
Nick Kralevich, Jeff Xu, Baolin Wang, Isaac Manjarres,
linux-kernel, linux-security-module, selinux, linux-mm
In-Reply-To: <CAHC9VhTSRkP=jNw8bLRx5em6MnX9HTywBqXGsJCBPQ9MKJym=g@mail.gmail.com>
>
> ... and the patch is now in selinux/dev, thanks everyone!
Great! Thanks again Paul, Stephen and Hugh for the reviews and help to
land this work.
^ permalink raw reply
* Re: [PATCH 1/2] LSM: Exclusive secmark usage
From: Paul Moore @ 2025-10-13 22:11 UTC (permalink / raw)
To: Casey Schaufler
Cc: Stephen Smalley, linux-security-module, jmorris, serge, keescook,
john.johansen, penguin-kernel, linux-kernel, selinux
In-Reply-To: <d091c8f1-798e-4b58-bed4-5f71478373c4@schaufler-ca.com>
On Fri, Oct 10, 2025 at 11:03 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 10/9/2025 11:49 AM, Stephen Smalley wrote:
> > On Wed, Oct 1, 2025 at 5:56 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> The network secmark can only be used by one security module
> >> at a time. Establish mechanism to identify to security modules
> > a mechanism to inform security modules?
> >
> >> whether they have access to the secmark. SELinux already
> >> incorparates mechanism, but it has to be added to Smack and
> > incorporates
> >
> >> AppArmor.
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >> include/linux/lsm_hooks.h | 1 +
> >> security/apparmor/include/net.h | 5 +++++
> >> security/apparmor/lsm.c | 7 ++++---
> >> security/security.c | 6 ++++++
> >> security/selinux/hooks.c | 4 +++-
> >> security/smack/smack.h | 5 +++++
> >> security/smack/smack_lsm.c | 3 ++-
> >> security/smack/smack_netfilter.c | 10 ++++++++--
> >> 8 files changed, 34 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/security/security.c b/security/security.c
> >> index ad163f06bf7a..e59e3d403de6 100644
> >> --- a/security/security.c
> >> +++ b/security/security.c
> >> @@ -283,6 +283,12 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
> >> lsm_set_blob_size(&needed->lbs_xattr_count,
> >> &blob_sizes.lbs_xattr_count);
> >> lsm_set_blob_size(&needed->lbs_bdev, &blob_sizes.lbs_bdev);
> >> + if (needed->lbs_secmark) {
> >> + if (blob_sizes.lbs_secmark)
> >> + needed->lbs_secmark = false;
> >> + else
> >> + blob_sizes.lbs_secmark = true;
> >> + }
> >
> > So if I understand correctly, the first LSM to register with
> > lbs_secmark set wins.
> > Not sure that's a great idea - seemingly some LSMs may want to insist
> > that they get to use secmark regardless of registration order?
>
> But what if two LSMs insist on getting the secmark? The whole point
> is to make it possible to use multiple LSMs that what the feature at
> the same time.
My current thinking is that if two LSMs *insist* on access to the
secmark, one of them has to fail to load/initialize, even if that
means a panic on boot (we should flag that as an invalid config in
Kconfig).
Perhaps the solution is to have lbs_secmark as a tristate value: don't
use secmarks, would like access to secmarks, must have access to
secmarks. Upon registration a LSM that requested "would like" could
check to see if they have been granted access and could adjust
accordingly. A LSM that requested "must have" would fail to register
if the secmarks were already taken by a prior LSM.
> The limitation on a secmark being a u32 is a huge problem,
> and Paul has battled with the netdev people over it for years.
I suspect the only way forward at this point is to convert the secmark
field into an IDR* that we could use to point to a LSM security blob
that could store LSM specific structs for both secmarks and general
LSM state associated with a skb. This would also allow us to do some
cool things in the forward path that we can't properly do now and
would make it much easier to handle a wider variety of packet level
access control scenarios.
It's on my todo list for <hand_waving>someday</hand_waving>, but if
somebody wanted to do it that would be awesome. Just a word of
warning, this is not a quick task and it is probably only suited for
someone who already has a few netdev inflicted scars.
*I see that IDR is now deprecated in favor of XArray, I haven't looked
that closely at XArray but it looks workable too.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH 1/2] LSM: Exclusive secmark usage
From: Paul Moore @ 2025-10-13 21:57 UTC (permalink / raw)
To: Casey Schaufler
Cc: linux-security-module, jmorris, serge, keescook, john.johansen,
penguin-kernel, stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20251001215643.31465-2-casey@schaufler-ca.com>
On Wed, Oct 1, 2025 at 5:56 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> The network secmark can only be used by one security module
> at a time. Establish mechanism to identify to security modules
> whether they have access to the secmark. SELinux already
> incorparates mechanism, but it has to be added to Smack and
> AppArmor.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/lsm_hooks.h | 1 +
> security/apparmor/include/net.h | 5 +++++
> security/apparmor/lsm.c | 7 ++++---
> security/security.c | 6 ++++++
> security/selinux/hooks.c | 4 +++-
> security/smack/smack.h | 5 +++++
> security/smack/smack_lsm.c | 3 ++-
> security/smack/smack_netfilter.c | 10 ++++++++--
> 8 files changed, 34 insertions(+), 7 deletions(-)
...
> /* Prepare LSM for initialization. */
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index c95a5874bf7d..5b6db7d8effb 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -164,7 +164,8 @@ __setup("checkreqprot=", checkreqprot_setup);
> */
> static int selinux_secmark_enabled(void)
> {
> - return (selinux_policycap_alwaysnetwork() ||
> + return selinux_blob_sizes.lbs_secmark &&
> + (selinux_policycap_alwaysnetwork() ||
> atomic_read(&selinux_secmark_refcount));
> }
This is an odd way to approach secmark enablement in SELinux, and not
something I think I want to see. Ignoring the
selinux_policycap_alwaysnetwork "abomination" (a joke I think only
about four people in the world might understand), the
selinux_secmark_enabled() function is really there simply as a
performance optimization since the majority of SELinux users don't
utilize the per-packet access controls. Using it as a mechanism to
effectively turn off SELinux's secmark functionality could result in a
confusing situation for users who are setting SELinux secmarks on
packets and not seeing the system's policy properly enforced.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v2 2/2] LSM: Infrastructure management of the mnt_opts security blob
From: Paul Moore @ 2025-10-13 20:55 UTC (permalink / raw)
To: Stephen Smalley
Cc: Casey Schaufler, Ondrej Mosnacek, eparis, linux-security-module,
jmorris, serge, keescook, john.johansen, penguin-kernel,
linux-kernel, selinux
In-Reply-To: <CAEjxPJ4D7A4KDF9BfmRa9VvzcAHBkkrdKCvmGazuZUto5=qDuw@mail.gmail.com>
On Thu, Oct 9, 2025 at 2:38 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Thu, Sep 25, 2025 at 1:12 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >
> > Move management of the mnt_opts->security blob out of the individual
> > security modules and into the security infrastructure. The modules
> > tell the infrastructure how much space is required, and the space is
> > allocated as required in the interfaces that use the blob.
> >
> > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> > ---
>
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 4bba9d119713..1ccf880e4894 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -656,19 +651,13 @@ static int selinux_set_mnt_opts(struct super_block *sb,
> > mutex_lock(&sbsec->lock);
> >
> > if (!selinux_initialized()) {
> > - if (!opts) {
> > - /* Defer initialization until selinux_complete_init,
> > - after the initial policy is loaded and the security
> > - server is ready to handle calls. */
> > - if (kern_flags & SECURITY_LSM_NATIVE_LABELS) {
> > - sbsec->flags |= SE_SBNATIVE;
> > - *set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
> > - }
> > - goto out;
> > + /* Defer initialization until selinux_complete_init,
> > + after the initial policy is loaded and the security
> > + server is ready to handle calls. */
> > + if (kern_flags & SECURITY_LSM_NATIVE_LABELS) {
> > + sbsec->flags |= SE_SBNATIVE;
> > + *set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
>
> This seemingly would produce a change in behavior for SELinux.
> Previously we would only do this if there were no SELinux mount
> options specified.
What Stephen said. I think this is good work that needs to be done
(thank you for doing it!), but we have to preserve existing behaviors.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] keys: Remove redundant less-than-zero checks
From: Mimi Zohar @ 2025-10-13 17:17 UTC (permalink / raw)
To: Thorsten Blum, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn, James Bottomley
Cc: keyrings, linux-security-module, linux-kernel, linux-integrity
In-Reply-To: <20251011144824.1257-2-thorsten.blum@linux.dev>
On Sat, 2025-10-11 at 16:48 +0200, Thorsten Blum wrote:
> The local variables 'size_t datalen' are unsigned and cannot be less
> than zero. Remove the redundant conditions.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v3] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Paul Menzel @ 2025-10-13 16:11 UTC (permalink / raw)
To: Thorsten Blum
Cc: Mimi Zohar, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn, linux-hardening, Kees Cook,
linux-integrity, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20251013152627.98231-2-thorsten.blum@linux.dev>
Dear Thorsten,
Thank you for the patch.
Am 13.10.25 um 17:26 schrieb Thorsten Blum:
> strncpy() is deprecated for NUL-terminated destination buffers; use
> strscpy_pad() instead to retain the NUL-padding behavior of strncpy().
>
> The destination buffer is initialized using kzalloc() with a 'signature'
> size of ECRYPTFS_PASSWORD_SIG_SIZE + 1. strncpy() then copies up to
> ECRYPTFS_PASSWORD_SIG_SIZE bytes from 'key_desc', NUL-padding any
> remaining bytes if needed, but expects the last byte to be zero.
>
> strscpy_pad() also copies the source string to 'signature', and NUL-pads
> the destination buffer if needed, but ensures it's always NUL-terminated
> without relying on it being zero-initialized.
>
> strscpy_pad() automatically determines the size of the fixed-length
> destination buffer via sizeof() when the optional size argument is
> omitted, making an explicit size unnecessary.
>
> In encrypted_init(), the source string 'key_desc' is validated by
> valid_ecryptfs_desc() before calling ecryptfs_fill_auth_tok(), and is
> therefore NUL-terminated and satisfies the __must_be_cstr() requirement
> of strscpy_pad().
>
> Link: https://github.com/KSPP/linux/issues/90
> Reviewed-by: Kees Cook <kees@kernel.org>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Changes in v3:
> - Improve commit message
> - Link to v2: https://lore.kernel.org/lkml/20251010161340.458707-2-thorsten.blum@linux.dev/
>
> Changes in v2:
> - Improve commit message as suggested by Jarkko and Kees
> - Link to v1: https://lore.kernel.org/lkml/20251009180316.394708-3-thorsten.blum@linux.dev/
> ---
> security/keys/encrypted-keys/ecryptfs_format.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/security/keys/encrypted-keys/ecryptfs_format.c b/security/keys/encrypted-keys/ecryptfs_format.c
> index 8fdd76105ce3..2fc6f3a66135 100644
> --- a/security/keys/encrypted-keys/ecryptfs_format.c
> +++ b/security/keys/encrypted-keys/ecryptfs_format.c
> @@ -54,8 +54,7 @@ int ecryptfs_fill_auth_tok(struct ecryptfs_auth_tok *auth_tok,
> auth_tok->version = (((uint16_t)(major << 8) & 0xFF00)
> | ((uint16_t)minor & 0x00FF));
> auth_tok->token_type = ECRYPTFS_PASSWORD;
> - strncpy((char *)auth_tok->token.password.signature, key_desc,
> - ECRYPTFS_PASSWORD_SIG_SIZE);
> + strscpy_pad(auth_tok->token.password.signature, key_desc);
> auth_tok->token.password.session_key_encryption_key_bytes =
> ECRYPTFS_MAX_KEY_BYTES;
> /*
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply
* Re: [PATCH v3] memfd,selinux: call security_inode_init_security_anon
From: Paul Moore @ 2025-10-13 16:11 UTC (permalink / raw)
To: Stephen Smalley
Cc: Thiébaud Weksteen, Hugh Dickins, James Morris,
Jeff Vander Stoep, Nick Kralevich, Jeff Xu, Baolin Wang,
Isaac Manjarres, linux-kernel, linux-security-module, selinux,
linux-mm
In-Reply-To: <CAHC9VhS2TU2GWgfUOHerbfjyxb5QZMSMqLdQirjSdkUohR7opg@mail.gmail.com>
On Mon, Sep 22, 2025 at 3:30 PM Paul Moore <paul@paul-moore.com> wrote:
> On Mon, Sep 22, 2025 at 9:12 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > When would you recommend that I re-apply the corresponding userspace
> > patch to reserve this policy capability number for memfd_class?
> > After it is moved to selinux/dev? Understand that it isn't truly
> > reserved until it lands in a kernel.org kernel but would prefer to
> > reapply it sooner than that since there may be other policy capability
> > requests queueing up (e.g. bpf token) that should be done relative to
> > it. Can always revert it again if necessary, at least until another
> > userspace release is made (not sure on timeline for that).
>
> When it comes to API issues like this, my standard answer is "tagged
> release from Linus" as it is the safest option, but you know that
> already.
>
> The fuzzier answer is that unless something crazy happens, I'm likely
> going to move the patches, in order, from selinux/dev-staging into
> selinux/dev when the merge window closes. This means that any
> policycap API additions for the next cycle are going to start with the
> memfd_class policycap, so it *should* be fairly safe to merge the
> userspace bits now, I just wouldn't do a userspace release with that
> API change until we see a tagged release from Linus.
... and the patch is now in selinux/dev, thanks everyone!
--
paul-moore.com
^ permalink raw reply
* [PATCH v3] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Thorsten Blum @ 2025-10-13 15:26 UTC (permalink / raw)
To: Mimi Zohar, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn
Cc: linux-hardening, Thorsten Blum, Kees Cook, linux-integrity,
keyrings, linux-security-module, linux-kernel
strncpy() is deprecated for NUL-terminated destination buffers; use
strscpy_pad() instead to retain the NUL-padding behavior of strncpy().
The destination buffer is initialized using kzalloc() with a 'signature'
size of ECRYPTFS_PASSWORD_SIG_SIZE + 1. strncpy() then copies up to
ECRYPTFS_PASSWORD_SIG_SIZE bytes from 'key_desc', NUL-padding any
remaining bytes if needed, but expects the last byte to be zero.
strscpy_pad() also copies the source string to 'signature', and NUL-pads
the destination buffer if needed, but ensures it's always NUL-terminated
without relying on it being zero-initialized.
strscpy_pad() automatically determines the size of the fixed-length
destination buffer via sizeof() when the optional size argument is
omitted, making an explicit size unnecessary.
In encrypted_init(), the source string 'key_desc' is validated by
valid_ecryptfs_desc() before calling ecryptfs_fill_auth_tok(), and is
therefore NUL-terminated and satisfies the __must_be_cstr() requirement
of strscpy_pad().
Link: https://github.com/KSPP/linux/issues/90
Reviewed-by: Kees Cook <kees@kernel.org>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
Changes in v3:
- Improve commit message
- Link to v2: https://lore.kernel.org/lkml/20251010161340.458707-2-thorsten.blum@linux.dev/
Changes in v2:
- Improve commit message as suggested by Jarkko and Kees
- Link to v1: https://lore.kernel.org/lkml/20251009180316.394708-3-thorsten.blum@linux.dev/
---
security/keys/encrypted-keys/ecryptfs_format.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/security/keys/encrypted-keys/ecryptfs_format.c b/security/keys/encrypted-keys/ecryptfs_format.c
index 8fdd76105ce3..2fc6f3a66135 100644
--- a/security/keys/encrypted-keys/ecryptfs_format.c
+++ b/security/keys/encrypted-keys/ecryptfs_format.c
@@ -54,8 +54,7 @@ int ecryptfs_fill_auth_tok(struct ecryptfs_auth_tok *auth_tok,
auth_tok->version = (((uint16_t)(major << 8) & 0xFF00)
| ((uint16_t)minor & 0x00FF));
auth_tok->token_type = ECRYPTFS_PASSWORD;
- strncpy((char *)auth_tok->token.password.signature, key_desc,
- ECRYPTFS_PASSWORD_SIG_SIZE);
+ strscpy_pad(auth_tok->token.password.signature, key_desc);
auth_tok->token.password.session_key_encryption_key_bytes =
ECRYPTFS_MAX_KEY_BYTES;
/*
--
2.51.0
^ permalink raw reply related
* [PATCH 6.17 004/563] pid: use ns_capable_noaudit() when determining net sysctl permissions
From: Greg Kroah-Hartman @ 2025-10-13 14:37 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, Christian Brauner,
linux-security-module, selinux, Christian Göttsche,
Serge Hallyn, Paul Moore, Sasha Levin
In-Reply-To: <20251013144411.274874080@linuxfoundation.org>
6.17-stable review patch. If anyone has any objections, please let me know.
------------------
From: Christian Göttsche <cgzones@googlemail.com>
[ Upstream commit b9cb7e59ac4ae68940347ebfc41e0436d32d3c6e ]
The capability check should not be audited since it is only being used
to determine the inode permissions. A failed check does not indicate a
violation of security policy but, when an LSM is enabled, a denial audit
message was being generated.
The denial audit message can either lead to the capability being
unnecessarily allowed in a security policy, or being silenced potentially
masking a legitimate capability check at a later point in time.
Similar to commit d6169b0206db ("net: Use ns_capable_noaudit() when
determining net sysctl permissions")
Fixes: 7863dcc72d0f ("pid: allow pid_max to be set per pid namespace")
CC: Christian Brauner <brauner@kernel.org>
CC: linux-security-module@vger.kernel.org
CC: selinux@vger.kernel.org
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
Reviewed-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
kernel/pid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/pid.c b/kernel/pid.c
index c45a28c16cd25..d94ce02505012 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -680,7 +680,7 @@ static int pid_table_root_permissions(struct ctl_table_header *head,
container_of(head->set, struct pid_namespace, set);
int mode = table->mode;
- if (ns_capable(pidns->user_ns, CAP_SYS_ADMIN) ||
+ if (ns_capable_noaudit(pidns->user_ns, CAP_SYS_ADMIN) ||
uid_eq(current_euid(), make_kuid(pidns->user_ns, 0)))
mode = (mode & S_IRWXU) >> 6;
else if (in_egroup_p(make_kgid(pidns->user_ns, 0)))
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v2 0/7] fs/9p: Reuse inode based on path (in addition to qid)
From: Greg Kurz @ 2025-10-13 9:24 UTC (permalink / raw)
To: Christian Schoenebeck
Cc: Mickaël Salaün, Dominique Martinet, qemu-devel,
Eric Van Hensbergen, Latchesar Ionkov, v9fs, Günther Noack,
linux-security-module, Jan Kara, Amir Goldstein,
Matthew Bobrowski, Al Viro, Christian Brauner, linux-fsdevel,
Tingmao Wang
In-Reply-To: <3061192.c3ltI2prpg@silver>
On Mon, 29 Sep 2025 15:06:59 +0200
Christian Schoenebeck <linux_oss@crudebyte.com> wrote:
> On Sunday, September 21, 2025 6:24:49 PM CEST Tingmao Wang wrote:
> > On 9/17/25 16:00, Mickaël Salaün wrote:
> [...]
>
> Hi Greg,
>
Hi Christian,
> I'd appreciate comments from your side as well, as you are much on longer on
> the QEMU 9p front than me.
>
> I know you won't have the time to read up on the entire thread so I try to
> summarize: basically this is yet another user-after-unlink issue, this time on
> directories instead of files.
>
Thread that never landed in my mailbox actually and it is quite
hard to understand the root problem with the content of this
e-mail actually ;-)
> > So I did some quick debugging and realized that I had a wrong
> > understanding of how fids relates to opened files on the host, under QEMU.
> > It turns out that in QEMU's 9p server implementation, a fid does not
> > actually correspond to any opened file descriptors - it merely represents
> > a (string-based) path that QEMU stores internally. It only opens the
> > actual file if the client actually does an T(l)open, which is in fact
> > separate from acquiring the fid with T(l)walk. The reason why renaming
> > file/dirs from the client doesn't break those fids is because QEMU will
> > actually fix those paths when a rename request is processed - c.f.
> > v9fs_fix_fid_paths [1].
>
> Correct, that's based on what the 9p protocols define: a FID does not exactly
> translate to what a file handle is on a local system. Even after acquiring a
> new FID by sending a Twalk request, subsequently client would still need to
> send a Topen for server to actually open that file/directory.
>
> And yes, QEMU's 9p server "fixes" the path string of a FID if it was moved
> upon client request. If the move happened on host side, outside of server's
> knowledge, then this won't happen ATM and hence it would break your use
> case.
>
> > It turns out that even if a guest process opens the file with O_PATH, that
> > file descriptor does not cause an actual Topen, and therefore QEMU does
> > not open the file on the host, and later on reopening that fd with another
> > mode (via e.g. open("/proc/self/fd/...", O_RDONLY)) will fail if the file
> > has moved on the host without QEMU's knowledge. Also, openat will fail if
> > provided with a dir fd that "points" to a moved directory, regardless of
> > whether the fd is opened with O_PATH or not, since path walk in QEMU is
> > completely string-based and does not actually issue openat on the host fs
> > [2].
>
> I don't think the problem here is the string based walk per se, but rather
> that the string based walk always starts from the export root:
>
> https://github.com/qemu/qemu/blob/4975b64efb5aa4248cbc3760312bbe08d6e71638/hw/9pfs/9p-local.c#L64
>
> I guess that's something that could be changed in QEMU such that the walk
> starts from FID's fs point, as the code already uses openat() to walk relative
> to a file descriptor (for security reasons actually), Greg?
>
Yes this was introduced for security reasons. In a nutshell, the idea is
to *not* follow symlinks in any element of the path being opened. It thus
naturally starts at the export root for which we have an fd.
> That alone would still not fix your use case though: things being moved on
> host side. For this to work, it would require to already have a fd open on
> host for the FID. This could be done by server for each FID as you suggested,
> or it could be done by client by opening the FID.
>
Can you elaborate on the "things being move on host side" ? With
an example of code that breaks on the client side ?
> Also keep in mind: once the open file descriptor limit on host is exhausted,
> QEMU is forced to close older open file desciptors to keep the QEMU process
> alive. So this might still break what you are trying to achieve there.
>
Correct.
> Having said that, I wonder whether it'd be simpler for server to track for
> file tree changes (inotify API) and fix the pathes accordingly for host
> side changes as well?
>
The problem is how to have the guest know about such changes, e.g. in
order to invalidate a stale cache entry. 9P doesn't provide any way for
host->client notification.
> /Christian
>
>
Cheers,
--
Greg
^ permalink raw reply
* Re: [PATCH v2] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Jarkko Sakkinen @ 2025-10-13 7:22 UTC (permalink / raw)
To: Kees Cook
Cc: Thorsten Blum, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, linux-hardening, linux-integrity,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <202510101543.80A1D4E3@keescook>
On Fri, Oct 10, 2025 at 03:48:48PM -0700, Kees Cook wrote:
> On Fri, Oct 10, 2025 at 06:13:41PM +0200, Thorsten Blum wrote:
> > strncpy() is deprecated for NUL-terminated destination buffers; use
> > strscpy_pad() instead to retain the zero-padding behavior of strncpy().
> >
> > strscpy_pad() automatically determines the size of the fixed-length
> > destination buffer via sizeof() when the optional size argument is
> > omitted, making an explicit size unnecessary.
>
> I would explicitly say that the old code was NUL terminating the buffer
> due to it being "ECRYPTFS_PASSWORD_SIG_SIZE + 1" sized with strncpy
> left to fill ECRYPTFS_PASSWORD_SIG_SIZE. And then you have to answer the
> question, "how was this initialized?" and trace it back to:
>
> epayload = kzalloc(sizeof(*epayload) + payload_datalen +
> datablob_len + HASH_SIZE + 1, GFP_KERNEL);
>
> so the final byte was always being zeroed there, but now we're
> explicitly zeroing it (good). So there _is_ a functional change (we're
> writing 1 more byte here now), but it's more robust that way. There is
> no expected _logical_ change, though, yes.
Thanks for the remarks.
Thorsten, would you mind posting +1 with the commit message changes,
and reviewed-by tags (from me and Kees).
>
> >
> > In encrypted_init(), the source string 'key_desc' is validated by
> > valid_ecryptfs_desc() before calling ecryptfs_fill_auth_tok(), and is
> > therefore NUL-terminated and satisfies the __must_be_cstr() requirement
> > of strscpy_pad().
> >
> > No functional changes.
[just as reminder: removing this sentence was my earlier remark]
> >
> > Link: https://github.com/KSPP/linux/issues/90
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>
> With "ECRYPTFS_PASSWORD_SIG_SIZE + 1" and tracing of the destination
> buffer initialization added to the commit log:
>
> Reviewed-by: Kees Cook <kees@kernel.org>
>
> -Kees
>
> --
> Kees Cook
BR, Jarkko
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: Paul Moore @ 2025-10-13 2:12 UTC (permalink / raw)
To: Alexei Starovoitov, KP Singh, James Bottomley, Linus Torvalds
Cc: Alexei Starovoitov, Blaise Boscaccy, bpf, LSM List,
K. Y. Srinivasan, Daniel Borkmann, Andrii Nakryiko, wufan,
Quentin Monnet
In-Reply-To: <fe538d3d723b161ee5354bb2de8e3a2ac7cf8255.camel@HansenPartnership.com>
On Sat, Oct 11, 2025 at 1:09 PM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Sat, 2025-10-11 at 09:31 -0700, Alexei Starovoitov wrote:
> > On Sat, Oct 11, 2025 at 7:52 AM James Bottomley
> > <James.Bottomley@hansenpartnership.com> wrote:
> > >
> > > It doesn't need to, once we check both the loader and the map, the
> > > integrity is verified and the loader can be trusted to run and
> > > relocate the map into the bpf program
> >
> > You should read KP's cover letter again and then research trusted
> > hash chains. Here is a quote from the first googled link:
> >
> > "A trusted hash chain is a cryptographic process used to verify the
> > integrity and authenticity of data by creating a sequence of hash
> > values, where each hash is linked to the next".
> >
> > In addition KP's algorithm was vetted by various security teams.
> > There is nothing novel here. It's a classic algorithm used
> > to verify integrity and that's what was implemented.
>
> Both KP and Blaise's patch sets are implementations of trusted hash
> chains. The security argument isn't about whether the hash chain
> algorithm works, it's about where, in relation to the LSM hook, the
> hash chain verification completes.
Alexei, considering the discussion from the past few days, and the
responses to all of your objections, I'm not seeing a clear reason why
you are opposed to sending Blaise's patchset up to Linus. What is
preventing you from sending Blaise's patch up to Linus?
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] apparmor: Replace sprintf/strcpy with scnprintf/strscpy in aa_policy_init
From: Serge E. Hallyn @ 2025-10-12 15:07 UTC (permalink / raw)
To: Thorsten Blum
Cc: John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
linux-hardening, apparmor, linux-security-module, linux-kernel
In-Reply-To: <20251011164645.46822-2-thorsten.blum@linux.dev>
On Sat, Oct 11, 2025 at 06:46:46PM +0200, Thorsten Blum wrote:
> strcpy() is deprecated and sprintf() does not perform bounds checking
> either. Although an overflow is unlikely, it's better to proactively
> avoid it by using the safer strscpy() and scnprintf(), respectively.
>
> Additionally, unify memory allocation for 'hname' to simplify and
> improve aa_policy_init().
>
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
> ---
> security/apparmor/lib.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
> index 82dbb97ad406..acf7f5189bec 100644
> --- a/security/apparmor/lib.c
> +++ b/security/apparmor/lib.c
> @@ -478,19 +478,17 @@ bool aa_policy_init(struct aa_policy *policy, const char *prefix,
> const char *name, gfp_t gfp)
> {
> char *hname;
> + size_t hname_sz;
>
> + hname_sz = (prefix ? strlen(prefix) + 2 : 0) + strlen(name) + 1;
> /* freed by policy_free */
> - if (prefix) {
> - hname = aa_str_alloc(strlen(prefix) + strlen(name) + 3, gfp);
> - if (hname)
> - sprintf(hname, "%s//%s", prefix, name);
> - } else {
> - hname = aa_str_alloc(strlen(name) + 1, gfp);
> - if (hname)
> - strcpy(hname, name);
> - }
> + hname = aa_str_alloc(hname_sz, gfp);
> if (!hname)
> return false;
> + if (prefix)
> + scnprintf(hname, hname_sz, "%s//%s", prefix, name);
> + else
> + strscpy(hname, name, hname_sz);
> policy->hname = hname;
> /* base.name is a substring of fqname */
> policy->name = basename(policy->hname);
> --
> 2.51.0
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: James Bottomley @ 2025-10-11 17:09 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Paul Moore, Alexei Starovoitov, KP Singh, Linus Torvalds,
Blaise Boscaccy, bpf, LSM List, K. Y. Srinivasan, Daniel Borkmann,
Andrii Nakryiko, wufan, Quentin Monnet
In-Reply-To: <CAADnVQ+M+_zLaqmd6As0z95A5BwGR8n8oFto-X-i4BgMvuhrXQ@mail.gmail.com>
On Sat, 2025-10-11 at 09:31 -0700, Alexei Starovoitov wrote:
> On Sat, Oct 11, 2025 at 7:52 AM James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> >
> > It doesn't need to, once we check both the loader and the map, the
> > integrity is verified and the loader can be trusted to run and
> > relocate the map into the bpf program
>
> You should read KP's cover letter again and then research trusted
> hash chains. Here is a quote from the first googled link:
>
> "A trusted hash chain is a cryptographic process used to verify the
> integrity and authenticity of data by creating a sequence of hash
> values, where each hash is linked to the next".
>
> In addition KP's algorithm was vetted by various security teams.
> There is nothing novel here. It's a classic algorithm used
> to verify integrity and that's what was implemented.
Both KP and Blaise's patch sets are implementations of trusted hash
chains. The security argument isn't about whether the hash chain
algorithm works, it's about where, in relation to the LSM hook, the
hash chain verification completes.
Regards,
James
^ permalink raw reply
* [PATCH] apparmor: Replace sprintf/strcpy with scnprintf/strscpy in aa_policy_init
From: Thorsten Blum @ 2025-10-11 16:46 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
Cc: linux-hardening, Thorsten Blum, apparmor, linux-security-module,
linux-kernel
strcpy() is deprecated and sprintf() does not perform bounds checking
either. Although an overflow is unlikely, it's better to proactively
avoid it by using the safer strscpy() and scnprintf(), respectively.
Additionally, unify memory allocation for 'hname' to simplify and
improve aa_policy_init().
Link: https://github.com/KSPP/linux/issues/88
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/apparmor/lib.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
index 82dbb97ad406..acf7f5189bec 100644
--- a/security/apparmor/lib.c
+++ b/security/apparmor/lib.c
@@ -478,19 +478,17 @@ bool aa_policy_init(struct aa_policy *policy, const char *prefix,
const char *name, gfp_t gfp)
{
char *hname;
+ size_t hname_sz;
+ hname_sz = (prefix ? strlen(prefix) + 2 : 0) + strlen(name) + 1;
/* freed by policy_free */
- if (prefix) {
- hname = aa_str_alloc(strlen(prefix) + strlen(name) + 3, gfp);
- if (hname)
- sprintf(hname, "%s//%s", prefix, name);
- } else {
- hname = aa_str_alloc(strlen(name) + 1, gfp);
- if (hname)
- strcpy(hname, name);
- }
+ hname = aa_str_alloc(hname_sz, gfp);
if (!hname)
return false;
+ if (prefix)
+ scnprintf(hname, hname_sz, "%s//%s", prefix, name);
+ else
+ strscpy(hname, name, hname_sz);
policy->hname = hname;
/* base.name is a substring of fqname */
policy->name = basename(policy->hname);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: Alexei Starovoitov @ 2025-10-11 16:31 UTC (permalink / raw)
To: James Bottomley
Cc: Paul Moore, Alexei Starovoitov, KP Singh, Linus Torvalds,
Blaise Boscaccy, bpf, LSM List, K. Y. Srinivasan, Daniel Borkmann,
Andrii Nakryiko, wufan, Quentin Monnet
In-Reply-To: <42bc677e031ed3df4f379cd3d6c9b3e1e8fadd87.camel@HansenPartnership.com>
On Sat, Oct 11, 2025 at 7:52 AM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> It doesn't need to, once we check both the loader and the map, the
> integrity is verified and the loader can be trusted to run and relocate
> the map into the bpf program
You should read KP's cover letter again and then research trusted
hash chains. Here is a quote from the first googled link:
"A trusted hash chain is a cryptographic process used to verify the
integrity and authenticity of data by creating a sequence of hash
values, where each hash is linked to the next".
In addition KP's algorithm was vetted by various security teams.
There is nothing novel here. It's a classic algorithm used
to verify integrity and that's what was implemented.
> > You need to realize that single loader plus single map is
> > an implementation choice of tools/lib/bpf/gen_loader.c.
> > It can do the same job with a single prog and no additional map.
>
> Yes, and if the light skeleton scheme embedded the relocation and the
> program itself into prog->insnsi then we wouldn't need the additional
> map verification before the load hook because the pkcs7 signature check
> would fully verify the integrity.
I'm fine if you want to hack gen_loader.c to produce the loader prog
without a map, but we're not going to pollute the kernel with
pointless apis, because you don't understand hash chains.
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: James Bottomley @ 2025-10-11 14:52 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Paul Moore, Alexei Starovoitov, KP Singh, Linus Torvalds,
Blaise Boscaccy, bpf, LSM List, K. Y. Srinivasan, Daniel Borkmann,
Andrii Nakryiko, wufan, Quentin Monnet
In-Reply-To: <CAADnVQKdsF5_9Vb_J+z27y5Of3P6J3gPNZ=hXKFi=APm6AHX3w@mail.gmail.com>
On Fri, 2025-10-10 at 16:06 -0700, Alexei Starovoitov wrote:
> On Fri, Oct 10, 2025 at 8:53 AM James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> >
> > On Thu, 2025-10-09 at 18:00 -0700, Alexei Starovoitov wrote:
> > [...]
> > > James's concern is valid though:
> > >
> > > > However, the rub for LSM is that the verification of the
> > > > program map by the loader happens *after* the
> > > > security_bpf_prog_load() hook has been called.
> > >
> > > I understand the discomfort, but that's what the kernel module
> > > loading process is doing as well, so you should be concerned with
> > > both. Since both are doing pretty much the same work.
> >
> > OK, so let me push on this one point because I don't agree with
> > what you say here. The way kernel modules and eBPF load is not
> > equivalent. The kernel module signatures go over a relocateable elf
> > binary which is subsequently relocated after signature verification
> > in the kernel by the ELF loader. You can regard the ELF loader as
> > being equivalent to the eBPF loader in terms of function,
> > absolutely. However for security purposes the ELF loader is a
> > trusted part of the kernel security envelope and its integrity is
> > part of the kernel integrity and we have a this single trusted
> > loader for every module. In security terms verification of the ELF
> > object signature is sufficient to guarantee integrity of the module
> > because the integrity of the ELF loader is already checked.
>
> "integrity of ELF loader" is _not_ checked. It's part of the kernel
> and you trust that the kernel is valid, because you trust the
> build tools that compiled that kernel.
No, I trust my kernel is valid because the bootloader checks its
signature on every boot (in case some tampering occurs after build).
There are a variety of integrity mechanisms for the kernel itself, but
they all essentially run a check on boot meaning the ELF loader *is*
integrity checked.
[...]
> > Thus for eBPF, the integrity check must be over both the loader and
> > the program; integrity checking is not complete until the integrity
> > of both has been verified.
>
> The signature covers all components: loader, the map that assists
> the loading and all progs and maps that are encoded inside that
> loader/map tuple.
The pkcs7 signature that is verified hashes over the loader only.
Other mechanisms cover the rest.
> > If you sign only the loader and embed the hash of the program into
> > the loader that is a different way of doing things,
>
> That's simply not true. Please read the current code more carefully.
> There is cover letter that describes what's happening. There are no
> hashes of programs.
Is this a semantic quibble over the meaning of the word "sign"? Fine,
I'll be more precise. Quoting from:
https://lore.kernel.org/bpf/20250921160120.9711-2-kpsingh@kernel.org/
> + bpf_dynptr_init(&insns_ptr, prog->insnsi,
> BPF_DYNPTR_TYPE_LOCAL, 0,
> + prog->len * sizeof(struct bpf_insn));
> +
> + err = bpf_verify_pkcs7_signature((struct bpf_dynptr
> *)&insns_ptr,
> + (struct bpf_dynptr
> *)&sig_ptr, key);
for a light skeleton prog->insnsi points to the loader and the second
statement is verifying the pkcs7 signature of the loader only.
> > There are two potential solutions to this: complete the integrity
> > check before running the load hook (Blaise's patch)
>
> That's not what it's doing! Read his patch. It's adding pointless
> signature to the loader/map tuple. It does nothing to progs, maps,
> relocations that will be created at the end when loader completes.
It doesn't need to, once we check both the loader and the map, the
integrity is verified and the loader can be trusted to run and relocate
the map into the bpf program (since you like the module parallel: we
have the integrity verification of the module object and the ELF
loader, so we can just let the relocation run: the output has integrity
because all inputs did).
> You need to realize that single loader plus single map is
> an implementation choice of tools/lib/bpf/gen_loader.c.
> It can do the same job with a single prog and no additional map.
Yes, and if the light skeleton scheme embedded the relocation and the
program itself into prog->insnsi then we wouldn't need the additional
map verification before the load hook because the pkcs7 signature check
would fully verify the integrity.
> Hence any kinda hard coded extra map signature makes no sense.
> We're not going to burden the kernel with one specific implementation
> detail of gen_loader.
> Tomorrow we might change the gen_loader to use a triple:
> prog+map+btf or any other form.
> The existing approach allows all that extensibility and freedom
> to change the gen_loader.
If we're whatiffing about the future: in the event that you need a
separate btf I assume it will come down as a map; bpftool gen -S -M
would compute a signature over the loader and both maps and the in-
kernel piece of Blaise's patch would just work.
Regards,
James
^ permalink raw reply
* [PATCH] keys: Remove redundant less-than-zero checks
From: Thorsten Blum @ 2025-10-11 14:48 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, James Bottomley
Cc: Thorsten Blum, keyrings, linux-security-module, linux-kernel,
linux-integrity
The local variables 'size_t datalen' are unsigned and cannot be less
than zero. Remove the redundant conditions.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/keys/big_key.c | 2 +-
security/keys/encrypted-keys/encrypted.c | 4 ++--
security/keys/trusted-keys/trusted_core.c | 4 ++--
security/keys/user_defined.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index c3367622c683..d46862ab90d6 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -66,7 +66,7 @@ int big_key_preparse(struct key_preparsed_payload *prep)
BUILD_BUG_ON(sizeof(*payload) != sizeof(prep->payload.data));
- if (datalen <= 0 || datalen > 1024 * 1024 || !prep->data)
+ if (datalen == 0 || datalen > 1024 * 1024 || !prep->data)
return -EINVAL;
/* Set an arbitrary quota */
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index d70f71d37f5f..57f88ae000ba 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -786,7 +786,7 @@ static int encrypted_instantiate(struct key *key,
size_t datalen = prep->datalen;
int ret;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
datablob = kmalloc(datalen + 1, GFP_KERNEL);
@@ -847,7 +847,7 @@ static int encrypted_update(struct key *key, struct key_preparsed_payload *prep)
if (key_is_negative(key))
return -ENOKEY;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
buf = kmalloc(datalen + 1, GFP_KERNEL);
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index e2d9644efde1..b1680ee53f86 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -157,7 +157,7 @@ static int trusted_instantiate(struct key *key,
int key_cmd;
size_t key_len;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
orig_datablob = datablob = kmalloc(datalen + 1, GFP_KERNEL);
@@ -240,7 +240,7 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
p = key->payload.data[0];
if (!p->migratable)
return -EPERM;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
orig_datablob = datablob = kmalloc(datalen + 1, GFP_KERNEL);
diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index 749e2a4dcb13..686d56e4cc85 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -61,7 +61,7 @@ int user_preparse(struct key_preparsed_payload *prep)
struct user_key_payload *upayload;
size_t datalen = prep->datalen;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
upayload = kmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v6 1/5] Wire up lsm_config_self_policy and lsm_config_system_policy syscalls
From: kernel test robot @ 2025-10-11 12:07 UTC (permalink / raw)
To: Maxime Bélair, linux-security-module
Cc: oe-kbuild-all, john.johansen, paul, jmorris, serge, mic, kees,
stephen.smalley.work, casey, takedakn, penguin-kernel, song,
rdunlap, linux-api, apparmor, linux-kernel, Maxime Bélair
In-Reply-To: <20251010132610.12001-2-maxime.belair@canonical.com>
Hi Maxime,
kernel test robot noticed the following build errors:
[auto build test ERROR on 9c32cda43eb78f78c73aee4aa344b777714e259b]
url: https://github.com/intel-lab-lkp/linux/commits/Maxime-B-lair/Wire-up-lsm_config_self_policy-and-lsm_config_system_policy-syscalls/20251010-213606
base: 9c32cda43eb78f78c73aee4aa344b777714e259b
patch link: https://lore.kernel.org/r/20251010132610.12001-2-maxime.belair%40canonical.com
patch subject: [PATCH v6 1/5] Wire up lsm_config_self_policy and lsm_config_system_policy syscalls
config: sh-randconfig-001-20251011 (https://download.01.org/0day-ci/archive/20251011/202510111947.0ObJ6YUH-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251011/202510111947.0ObJ6YUH-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510111947.0ObJ6YUH-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from kernel/umh.c:9:0:
>> include/linux/syscalls.h:994:45: error: expected ';', ',' or ')' before 'u32'
u32 __user size, u32 common_flags u32 flags);
^~~
--
In file included from kernel/fork.c:56:0:
>> include/linux/syscalls.h:994:45: error: expected ';', ',' or ')' before 'u32'
u32 __user size, u32 common_flags u32 flags);
^~~
kernel/fork.c: In function '__do_sys_clone3':
kernel/fork.c:3135:2: warning: #warning clone3() entry point is missing, please fix [-Wcpp]
#warning clone3() entry point is missing, please fix
^~~~~~~
vim +994 include/linux/syscalls.h
817
818 /* CONFIG_MMU only */
819 asmlinkage long sys_swapon(const char __user *specialfile, int swap_flags);
820 asmlinkage long sys_swapoff(const char __user *specialfile);
821 asmlinkage long sys_mprotect(unsigned long start, size_t len,
822 unsigned long prot);
823 asmlinkage long sys_msync(unsigned long start, size_t len, int flags);
824 asmlinkage long sys_mlock(unsigned long start, size_t len);
825 asmlinkage long sys_munlock(unsigned long start, size_t len);
826 asmlinkage long sys_mlockall(int flags);
827 asmlinkage long sys_munlockall(void);
828 asmlinkage long sys_mincore(unsigned long start, size_t len,
829 unsigned char __user * vec);
830 asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
831 asmlinkage long sys_process_madvise(int pidfd, const struct iovec __user *vec,
832 size_t vlen, int behavior, unsigned int flags);
833 asmlinkage long sys_process_mrelease(int pidfd, unsigned int flags);
834 asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
835 unsigned long prot, unsigned long pgoff,
836 unsigned long flags);
837 asmlinkage long sys_mseal(unsigned long start, size_t len, unsigned long flags);
838 asmlinkage long sys_mbind(unsigned long start, unsigned long len,
839 unsigned long mode,
840 const unsigned long __user *nmask,
841 unsigned long maxnode,
842 unsigned flags);
843 asmlinkage long sys_get_mempolicy(int __user *policy,
844 unsigned long __user *nmask,
845 unsigned long maxnode,
846 unsigned long addr, unsigned long flags);
847 asmlinkage long sys_set_mempolicy(int mode, const unsigned long __user *nmask,
848 unsigned long maxnode);
849 asmlinkage long sys_migrate_pages(pid_t pid, unsigned long maxnode,
850 const unsigned long __user *from,
851 const unsigned long __user *to);
852 asmlinkage long sys_move_pages(pid_t pid, unsigned long nr_pages,
853 const void __user * __user *pages,
854 const int __user *nodes,
855 int __user *status,
856 int flags);
857 asmlinkage long sys_rt_tgsigqueueinfo(pid_t tgid, pid_t pid, int sig,
858 siginfo_t __user *uinfo);
859 asmlinkage long sys_perf_event_open(
860 struct perf_event_attr __user *attr_uptr,
861 pid_t pid, int cpu, int group_fd, unsigned long flags);
862 asmlinkage long sys_accept4(int, struct sockaddr __user *, int __user *, int);
863 asmlinkage long sys_recvmmsg(int fd, struct mmsghdr __user *msg,
864 unsigned int vlen, unsigned flags,
865 struct __kernel_timespec __user *timeout);
866 asmlinkage long sys_recvmmsg_time32(int fd, struct mmsghdr __user *msg,
867 unsigned int vlen, unsigned flags,
868 struct old_timespec32 __user *timeout);
869 asmlinkage long sys_wait4(pid_t pid, int __user *stat_addr,
870 int options, struct rusage __user *ru);
871 asmlinkage long sys_prlimit64(pid_t pid, unsigned int resource,
872 const struct rlimit64 __user *new_rlim,
873 struct rlimit64 __user *old_rlim);
874 asmlinkage long sys_fanotify_init(unsigned int flags, unsigned int event_f_flags);
875 #if defined(CONFIG_ARCH_SPLIT_ARG64)
876 asmlinkage long sys_fanotify_mark(int fanotify_fd, unsigned int flags,
877 unsigned int mask_1, unsigned int mask_2,
878 int dfd, const char __user * pathname);
879 #else
880 asmlinkage long sys_fanotify_mark(int fanotify_fd, unsigned int flags,
881 u64 mask, int fd,
882 const char __user *pathname);
883 #endif
884 asmlinkage long sys_name_to_handle_at(int dfd, const char __user *name,
885 struct file_handle __user *handle,
886 void __user *mnt_id, int flag);
887 asmlinkage long sys_open_by_handle_at(int mountdirfd,
888 struct file_handle __user *handle,
889 int flags);
890 asmlinkage long sys_clock_adjtime(clockid_t which_clock,
891 struct __kernel_timex __user *tx);
892 asmlinkage long sys_clock_adjtime32(clockid_t which_clock,
893 struct old_timex32 __user *tx);
894 asmlinkage long sys_syncfs(int fd);
895 asmlinkage long sys_setns(int fd, int nstype);
896 asmlinkage long sys_pidfd_open(pid_t pid, unsigned int flags);
897 asmlinkage long sys_sendmmsg(int fd, struct mmsghdr __user *msg,
898 unsigned int vlen, unsigned flags);
899 asmlinkage long sys_process_vm_readv(pid_t pid,
900 const struct iovec __user *lvec,
901 unsigned long liovcnt,
902 const struct iovec __user *rvec,
903 unsigned long riovcnt,
904 unsigned long flags);
905 asmlinkage long sys_process_vm_writev(pid_t pid,
906 const struct iovec __user *lvec,
907 unsigned long liovcnt,
908 const struct iovec __user *rvec,
909 unsigned long riovcnt,
910 unsigned long flags);
911 asmlinkage long sys_kcmp(pid_t pid1, pid_t pid2, int type,
912 unsigned long idx1, unsigned long idx2);
913 asmlinkage long sys_finit_module(int fd, const char __user *uargs, int flags);
914 asmlinkage long sys_sched_setattr(pid_t pid,
915 struct sched_attr __user *attr,
916 unsigned int flags);
917 asmlinkage long sys_sched_getattr(pid_t pid,
918 struct sched_attr __user *attr,
919 unsigned int size,
920 unsigned int flags);
921 asmlinkage long sys_renameat2(int olddfd, const char __user *oldname,
922 int newdfd, const char __user *newname,
923 unsigned int flags);
924 asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
925 void __user *uargs);
926 asmlinkage long sys_getrandom(char __user *buf, size_t count,
927 unsigned int flags);
928 asmlinkage long sys_memfd_create(const char __user *uname_ptr, unsigned int flags);
929 asmlinkage long sys_bpf(int cmd, union bpf_attr __user *attr, unsigned int size);
930 asmlinkage long sys_execveat(int dfd, const char __user *filename,
931 const char __user *const __user *argv,
932 const char __user *const __user *envp, int flags);
933 asmlinkage long sys_userfaultfd(int flags);
934 asmlinkage long sys_membarrier(int cmd, unsigned int flags, int cpu_id);
935 asmlinkage long sys_mlock2(unsigned long start, size_t len, int flags);
936 asmlinkage long sys_copy_file_range(int fd_in, loff_t __user *off_in,
937 int fd_out, loff_t __user *off_out,
938 size_t len, unsigned int flags);
939 asmlinkage long sys_preadv2(unsigned long fd, const struct iovec __user *vec,
940 unsigned long vlen, unsigned long pos_l, unsigned long pos_h,
941 rwf_t flags);
942 asmlinkage long sys_pwritev2(unsigned long fd, const struct iovec __user *vec,
943 unsigned long vlen, unsigned long pos_l, unsigned long pos_h,
944 rwf_t flags);
945 asmlinkage long sys_pkey_mprotect(unsigned long start, size_t len,
946 unsigned long prot, int pkey);
947 asmlinkage long sys_pkey_alloc(unsigned long flags, unsigned long init_val);
948 asmlinkage long sys_pkey_free(int pkey);
949 asmlinkage long sys_statx(int dfd, const char __user *path, unsigned flags,
950 unsigned mask, struct statx __user *buffer);
951 asmlinkage long sys_rseq(struct rseq __user *rseq, uint32_t rseq_len,
952 int flags, uint32_t sig);
953 asmlinkage long sys_open_tree(int dfd, const char __user *path, unsigned flags);
954 asmlinkage long sys_open_tree_attr(int dfd, const char __user *path,
955 unsigned flags,
956 struct mount_attr __user *uattr,
957 size_t usize);
958 asmlinkage long sys_move_mount(int from_dfd, const char __user *from_path,
959 int to_dfd, const char __user *to_path,
960 unsigned int ms_flags);
961 asmlinkage long sys_mount_setattr(int dfd, const char __user *path,
962 unsigned int flags,
963 struct mount_attr __user *uattr, size_t usize);
964 asmlinkage long sys_fsopen(const char __user *fs_name, unsigned int flags);
965 asmlinkage long sys_fsconfig(int fs_fd, unsigned int cmd, const char __user *key,
966 const void __user *value, int aux);
967 asmlinkage long sys_fsmount(int fs_fd, unsigned int flags, unsigned int ms_flags);
968 asmlinkage long sys_fspick(int dfd, const char __user *path, unsigned int flags);
969 asmlinkage long sys_pidfd_send_signal(int pidfd, int sig,
970 siginfo_t __user *info,
971 unsigned int flags);
972 asmlinkage long sys_pidfd_getfd(int pidfd, int fd, unsigned int flags);
973 asmlinkage long sys_landlock_create_ruleset(const struct landlock_ruleset_attr __user *attr,
974 size_t size, __u32 flags);
975 asmlinkage long sys_landlock_add_rule(int ruleset_fd, enum landlock_rule_type rule_type,
976 const void __user *rule_attr, __u32 flags);
977 asmlinkage long sys_landlock_restrict_self(int ruleset_fd, __u32 flags);
978 asmlinkage long sys_memfd_secret(unsigned int flags);
979 asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
980 unsigned long home_node,
981 unsigned long flags);
982 asmlinkage long sys_cachestat(unsigned int fd,
983 struct cachestat_range __user *cstat_range,
984 struct cachestat __user *cstat, unsigned int flags);
985 asmlinkage long sys_map_shadow_stack(unsigned long addr, unsigned long size, unsigned int flags);
986 asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx __user *ctx,
987 u32 __user *size, u32 flags);
988 asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx __user *ctx,
989 u32 size, u32 flags);
990 asmlinkage long sys_lsm_list_modules(u64 __user *ids, u32 __user *size, u32 flags);
991 asmlinkage long sys_lsm_config_self_policy(u32 lsm_id, u32 op, void __user *buf,
992 u32 __user size, u32 common_flags, u32 flags);
993 asmlinkage long sys_lsm_config_system_policy(u32 lsm_id, u32 op, void __user *buf,
> 994 u32 __user size, u32 common_flags u32 flags);
995
996
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox