* Re: [PATCH RFC 12/15] LSM: Allow reservation of netlabel
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-13-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Allow LSMs to request exclusive access to the netlabel facility.
> Provide mechanism for LSMs to determine if they have access to
> netlabel. Update the current users of netlabel, SELinux and Smack,
> to use and respect the exclusive use of netlabel.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/lsm_hooks.h | 1 +
> security/lsm_init.c | 6 +++++
> security/selinux/hooks.c | 7 +++---
> security/selinux/include/netlabel.h | 5 ++++
> security/selinux/netlabel.c | 4 ++--
> security/smack/smack.h | 5 ++++
> security/smack/smack_lsm.c | 36 +++++++++++++++++++++--------
> security/smack/smackfs.c | 20 +++++++++++++++-
> 8 files changed, 69 insertions(+), 15 deletions(-)
We discussed this patch in a separate patchset, lore link below.
https://lore.kernel.org/linux-security-module/20251001215643.31465-1-casey@schaufler-ca.com/
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 11/15] LSM: Infrastructure management of the mnt_opts security blob
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-12-casey@schaufler-ca.com>
On Jun 21, 2025 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. Blobs are still allocated within the modules
> as they are only required when mount options are present.
> The modules tell the infrastructure how much space is required,
> and the space is allocated if needed. Modules can no longer
> count on the presence of a blob implying that mount options
> specific to that module are present, so flags are added
> to the module specific blobs to indicate that this module
> has options.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> security/security.c | 14 ++++-----
> security/selinux/hooks.c | 58 +++++++++++++++++++++++-------------
> security/smack/smack_lsm.c | 61 ++++++++++++++++++++++++++------------
> 3 files changed, 85 insertions(+), 48 deletions(-)
We discussed this patch in a separate patchset, lore link below.
https://lore.kernel.org/linux-security-module/20250925171208.5997-1-casey@schaufler-ca.com/
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 10/15] LSM: allocate mnt_opts blobs instead of module specific data
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-11-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Replace allocations of LSM specific mount data with the
> shared mnt_opts blob.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/lsm_hooks.h | 1 +
> security/security.c | 12 ++++++++++++
> security/selinux/hooks.c | 10 +++++++---
> security/smack/smack_lsm.c | 4 ++--
> 4 files changed, 22 insertions(+), 5 deletions(-)
We discussed the concepts in this patch in a separate patchset, lore link below.
https://lore.kernel.org/linux-security-module/20250925171208.5997-1-casey@schaufler-ca.com/
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 9/15] LSM: Add mount opts blob size tracking
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-10-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Add mount option data to the blob size accounting in anticipation
> of using a shared mnt_opts blob.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/lsm_hooks.h | 1 +
> security/lsm_init.c | 2 ++
> security/selinux/hooks.c | 1 +
> security/smack/smack_lsm.c | 1 +
> 4 files changed, 5 insertions(+)
We discussed this patch in a separate patchset, lore link below.
https://lore.kernel.org/linux-security-module/20250925171208.5997-1-casey@schaufler-ca.com/
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 7/15] Audit: Call only the first of the audit rule 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-8-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> The audit system is not (yet) capable for distinguishing
> between audit rules specified for multiple security modules.
> Call only the first registered of the audit rule hooks.
> The order of registration, which can be specified with the
> lsm= boot parameter, is hence an important consideration.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> security/security.c | 30 ++++++++++++++++++++++++++----
> 1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/security/security.c b/security/security.c
> index 2286285f8aea..93d4ac39fe9f 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -5056,7 +5056,13 @@ void security_key_post_create_or_update(struct key *keyring, struct key *key,
> int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule,
> gfp_t gfp)
> {
> - return call_int_hook(audit_rule_init, field, op, rulestr, lsmrule, gfp);
> + struct lsm_static_call *scall;
> +
> + lsm_for_each_hook(scall, audit_rule_init) {
> + return scall->hl->hook.audit_rule_init(field, op, rulestr,
> + lsmrule, gfp);
> + }
> + return LSM_RET_DEFAULT(audit_rule_init);
> }
Similar to the comments in patch 5/15, what would it look like to do the
enforcement of this callback restriction at LSM registration time?
> /**
> @@ -5070,7 +5076,12 @@ int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule,
> */
> int security_audit_rule_known(struct audit_krule *krule)
> {
> - return call_int_hook(audit_rule_known, krule);
> + struct lsm_static_call *scall;
> +
> + lsm_for_each_hook(scall, audit_rule_known) {
> + return scall->hl->hook.audit_rule_known(krule);
> + }
> + return LSM_RET_DEFAULT(audit_rule_known);
> }
>
> /**
> @@ -5082,7 +5093,12 @@ int security_audit_rule_known(struct audit_krule *krule)
> */
> void security_audit_rule_free(void *lsmrule)
> {
> - call_void_hook(audit_rule_free, lsmrule);
> + struct lsm_static_call *scall;
> +
> + lsm_for_each_hook(scall, audit_rule_free) {
> + scall->hl->hook.audit_rule_free(lsmrule);
> + return;
> + }
> }
>
> /**
> @@ -5101,7 +5117,13 @@ void security_audit_rule_free(void *lsmrule)
> int security_audit_rule_match(struct lsm_prop *prop, u32 field, u32 op,
> void *lsmrule)
> {
> - return call_int_hook(audit_rule_match, prop, field, op, lsmrule);
> + struct lsm_static_call *scall;
> +
> + lsm_for_each_hook(scall, audit_rule_match) {
> + return scall->hl->hook.audit_rule_match(prop, field, op,
> + lsmrule);
> + }
> + return LSM_RET_DEFAULT(audit_rule_match);
> }
> #endif /* CONFIG_AUDIT */
>
> --
> 2.47.0
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 6/15] LSM: Exclusive secmark usage
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-7-casey@schaufler-ca.com>
On Jun 21, 2025 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/lsm_init.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(-)
We discussed this patch in a separate patchset, lore link below.
https://lore.kernel.org/linux-security-module/20251001215643.31465-1-casey@schaufler-ca.com/
--
paul-moore.com
^ permalink raw reply
* 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
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