* Re: [PATCH 2/3] LSM: allocate mnt_opts blobs instead of module specific data
From: Paul Moore @ 2025-08-07 0:40 UTC (permalink / raw)
To: Casey Schaufler
Cc: eparis, linux-security-module, jmorris, serge, keescook,
john.johansen, penguin-kernel, stephen.smalley.work, linux-kernel,
selinux
In-Reply-To: <f3a8cbc4-069f-4ec2-8bb5-708b90360b05@schaufler-ca.com>
On Wed, Aug 6, 2025 at 7:16 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 8/6/2025 3:06 PM, Paul Moore wrote:
> > On Jun 17, 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(-)
> > ..
> >
> >> diff --git a/security/security.c b/security/security.c
> >> index 8a4e0f70e49d..ec61fb7e6492 100644
> >> --- a/security/security.c
> >> +++ b/security/security.c
> >> @@ -904,6 +904,18 @@ void security_sb_free(struct super_block *sb)
> >> sb->s_security = NULL;
> >> }
> >>
> >> +/**
> >> + * lsm_mnt_opts_alloc - allocate a mnt_opts blob
> >> + * @priority: memory allocation priority
> >> + *
> >> + * Returns a newly allocated mnt_opts blob or NULL if
> >> + * memory isn't available.
> >> + */
> >> +void *lsm_mnt_opts_alloc(gfp_t priority)
> >> +{
> >> + return kzalloc(blob_sizes.lbs_mnt_opts, priority);
> >> +}
> > It's probably better to use lsm_blob_alloc() here so we have some
> > allocator consistency.
> >
> > Also, make this private/static as we should just handle the blob
> > allocation in the LSM framework (see below) just like everything else,
> > unless you can explain why the mount options need to be handled
> > differently?
>
> The mount blob is different from the other blobs in that it is
> only used if there are LSM specific mount options. If there aren't
> LSM specific mount options there is no reason to have a blob.
> I know it's not a huge deal, but there is a performance cost in
> allocating a blob that isn't used.
>
> If you'd really rather accept the overhead, I can make the blob
> always allocated. Let me know.
Well, this is happening at mount time, which should already have a
non-trivial amount of overhead (parsing options, doing the filesystem
setup, mount tree addition, etc.) so I'm not sure this will really be
noticeable in practice. I guess one could also make an argument about
additional memory pressure, but the mount options blob should have a
fairly short lifetime so I don't see that as a significant issue
either. If one, or both, of these becomes an issue we can look into
ways of mitigating them, but right now I'd just assume keep with the
existing LSM blob allocation pattern to simplify the code and make
life easier.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH 2/3] LSM: allocate mnt_opts blobs instead of module specific data
From: Casey Schaufler @ 2025-08-06 23:16 UTC (permalink / raw)
To: Paul Moore, eparis, linux-security-module
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux, Casey Schaufler
In-Reply-To: <f7e03785a79a0ac8f034cd38e263b84f@paul-moore.com>
On 8/6/2025 3:06 PM, Paul Moore wrote:
> On Jun 17, 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(-)
> ..
>
>> diff --git a/security/security.c b/security/security.c
>> index 8a4e0f70e49d..ec61fb7e6492 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -904,6 +904,18 @@ void security_sb_free(struct super_block *sb)
>> sb->s_security = NULL;
>> }
>>
>> +/**
>> + * lsm_mnt_opts_alloc - allocate a mnt_opts blob
>> + * @priority: memory allocation priority
>> + *
>> + * Returns a newly allocated mnt_opts blob or NULL if
>> + * memory isn't available.
>> + */
>> +void *lsm_mnt_opts_alloc(gfp_t priority)
>> +{
>> + return kzalloc(blob_sizes.lbs_mnt_opts, priority);
>> +}
> It's probably better to use lsm_blob_alloc() here so we have some
> allocator consistency.
>
> Also, make this private/static as we should just handle the blob
> allocation in the LSM framework (see below) just like everything else,
> unless you can explain why the mount options need to be handled
> differently?
The mount blob is different from the other blobs in that it is
only used if there are LSM specific mount options. If there aren't
LSM specific mount options there is no reason to have a blob.
I know it's not a huge deal, but there is a performance cost in
allocating a blob that isn't used.
If you'd really rather accept the overhead, I can make the blob
always allocated. Let me know.
>
>> /**
>> * security_free_mnt_opts() - Free memory associated with mount options
>> * @mnt_opts: LSM processed mount options
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 88cd1d56081a..f7eda0cce68f 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -2808,7 +2808,7 @@ static int selinux_fs_context_submount(struct fs_context *fc,
>> if (!(sbsec->flags & (FSCONTEXT_MNT|CONTEXT_MNT|DEFCONTEXT_MNT)))
>> return 0;
>>
>> - opts = kzalloc(sizeof(*opts), GFP_KERNEL);
>> + opts = lsm_mnt_opts_alloc(GFP_KERNEL);
> See above.
>
>> if (!opts)
>> return -ENOMEM;
>>
>> @@ -2830,8 +2830,12 @@ static int selinux_fs_context_dup(struct fs_context *fc,
>> if (!src)
>> return 0;
>>
>> - fc->security = kmemdup(src, sizeof(*src), GFP_KERNEL);
>> - return fc->security ? 0 : -ENOMEM;
>> + fc->security = lsm_mnt_opts_alloc(GFP_KERNEL);
>> + if (!fc->security)
>> + return -ENOMEM;
> Another case where we should do the allocation in the LSM framework.
>
>> + memcpy(fc->security, src, sizeof(*src));
>> + return 0;
>> }
>>
>> static const struct fs_parameter_spec selinux_fs_parameters[] = {
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index 44bd92410425..1d456df40096 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -622,7 +622,7 @@ static int smack_fs_context_submount(struct fs_context *fc,
>> struct smack_mnt_opts *ctx;
>> struct inode_smack *isp;
>>
>> - ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>> + ctx = lsm_mnt_opts_alloc(GFP_KERNEL);
>> if (!ctx)
>> return -ENOMEM;
>> fc->security = ctx;
>> @@ -673,7 +673,7 @@ static int smack_fs_context_dup(struct fs_context *fc,
>> if (!src)
>> return 0;
>>
>> - fc->security = kzalloc(sizeof(struct smack_mnt_opts), GFP_KERNEL);
>> + fc->security = lsm_mnt_opts_alloc(GFP_KERNEL);
>> if (!fc->security)
>> return -ENOMEM;
> Same thing in Smack.
>
> --
> paul-moore.com
>
^ permalink raw reply
* Re: [PATCH 2/3] LSM: allocate mnt_opts blobs instead of module specific data
From: Paul Moore @ 2025-08-06 22:06 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250617210105.17479-3-casey@schaufler-ca.com>
On Jun 17, 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(-)
...
> diff --git a/security/security.c b/security/security.c
> index 8a4e0f70e49d..ec61fb7e6492 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -904,6 +904,18 @@ void security_sb_free(struct super_block *sb)
> sb->s_security = NULL;
> }
>
> +/**
> + * lsm_mnt_opts_alloc - allocate a mnt_opts blob
> + * @priority: memory allocation priority
> + *
> + * Returns a newly allocated mnt_opts blob or NULL if
> + * memory isn't available.
> + */
> +void *lsm_mnt_opts_alloc(gfp_t priority)
> +{
> + return kzalloc(blob_sizes.lbs_mnt_opts, priority);
> +}
It's probably better to use lsm_blob_alloc() here so we have some
allocator consistency.
Also, make this private/static as we should just handle the blob
allocation in the LSM framework (see below) just like everything else,
unless you can explain why the mount options need to be handled
differently?
> /**
> * security_free_mnt_opts() - Free memory associated with mount options
> * @mnt_opts: LSM processed mount options
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 88cd1d56081a..f7eda0cce68f 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2808,7 +2808,7 @@ static int selinux_fs_context_submount(struct fs_context *fc,
> if (!(sbsec->flags & (FSCONTEXT_MNT|CONTEXT_MNT|DEFCONTEXT_MNT)))
> return 0;
>
> - opts = kzalloc(sizeof(*opts), GFP_KERNEL);
> + opts = lsm_mnt_opts_alloc(GFP_KERNEL);
See above.
> if (!opts)
> return -ENOMEM;
>
> @@ -2830,8 +2830,12 @@ static int selinux_fs_context_dup(struct fs_context *fc,
> if (!src)
> return 0;
>
> - fc->security = kmemdup(src, sizeof(*src), GFP_KERNEL);
> - return fc->security ? 0 : -ENOMEM;
> + fc->security = lsm_mnt_opts_alloc(GFP_KERNEL);
> + if (!fc->security)
> + return -ENOMEM;
Another case where we should do the allocation in the LSM framework.
> + memcpy(fc->security, src, sizeof(*src));
> + return 0;
> }
>
> static const struct fs_parameter_spec selinux_fs_parameters[] = {
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 44bd92410425..1d456df40096 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -622,7 +622,7 @@ static int smack_fs_context_submount(struct fs_context *fc,
> struct smack_mnt_opts *ctx;
> struct inode_smack *isp;
>
> - ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> + ctx = lsm_mnt_opts_alloc(GFP_KERNEL);
> if (!ctx)
> return -ENOMEM;
> fc->security = ctx;
> @@ -673,7 +673,7 @@ static int smack_fs_context_dup(struct fs_context *fc,
> if (!src)
> return 0;
>
> - fc->security = kzalloc(sizeof(struct smack_mnt_opts), GFP_KERNEL);
> + fc->security = lsm_mnt_opts_alloc(GFP_KERNEL);
> if (!fc->security)
> return -ENOMEM;
Same thing in Smack.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH 1/3] LSM: Add mount opts blob size tracking
From: Paul Moore @ 2025-08-06 22:06 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250617210105.17479-2-casey@schaufler-ca.com>
On Jun 17, 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(+)
Since you're respinning this patchset for other reasons, just base it
on the existing LSM initialization code. If this patchset collides with
the init rework mid-flight I'll handle the merge fixup.
I appreciate the consideration, but in this case I think it's easier to
not have the dependency.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] lsm: use lsm_blob_alloc() in lsm_bdev_alloc()
From: Casey Schaufler @ 2025-08-06 21:35 UTC (permalink / raw)
To: Paul Moore, linux-security-module
In-Reply-To: <20250806212552.240730-2-paul@paul-moore.com>
On 8/6/2025 2:25 PM, Paul Moore wrote:
> Convert the lsm_bdev_alloc() function to use the lsm_blob_alloc() helper
> like all of the other LSM security blob allocators.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> security/security.c | 12 ++----------
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/security/security.c b/security/security.c
> index ad163f06bf7a..a88ebfca3224 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -823,16 +823,8 @@ static int lsm_msg_msg_alloc(struct msg_msg *mp)
> */
> static int lsm_bdev_alloc(struct block_device *bdev)
> {
> - if (blob_sizes.lbs_bdev == 0) {
> - bdev->bd_security = NULL;
> - return 0;
> - }
> -
> - bdev->bd_security = kzalloc(blob_sizes.lbs_bdev, GFP_KERNEL);
> - if (!bdev->bd_security)
> - return -ENOMEM;
> -
> - return 0;
> + return lsm_blob_alloc(&bdev->bd_security, blob_sizes.lbs_bdev,
> + GFP_KERNEL);
> }
>
> /**
^ permalink raw reply
* [PATCH] lsm: use lsm_blob_alloc() in lsm_bdev_alloc()
From: Paul Moore @ 2025-08-06 21:25 UTC (permalink / raw)
To: linux-security-module
Convert the lsm_bdev_alloc() function to use the lsm_blob_alloc() helper
like all of the other LSM security blob allocators.
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
security/security.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/security/security.c b/security/security.c
index ad163f06bf7a..a88ebfca3224 100644
--- a/security/security.c
+++ b/security/security.c
@@ -823,16 +823,8 @@ static int lsm_msg_msg_alloc(struct msg_msg *mp)
*/
static int lsm_bdev_alloc(struct block_device *bdev)
{
- if (blob_sizes.lbs_bdev == 0) {
- bdev->bd_security = NULL;
- return 0;
- }
-
- bdev->bd_security = kzalloc(blob_sizes.lbs_bdev, GFP_KERNEL);
- if (!bdev->bd_security)
- return -ENOMEM;
-
- return 0;
+ return lsm_blob_alloc(&bdev->bd_security, blob_sizes.lbs_bdev,
+ GFP_KERNEL);
}
/**
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v5 3/5] Audit: Add record for multiple task security contexts
From: Casey Schaufler @ 2025-08-06 16:03 UTC (permalink / raw)
To: Paul Moore, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux, Casey Schaufler
In-Reply-To: <aafebe14727836ea747b97982926cc38@paul-moore.com>
On 8/5/2025 12:39 PM, Paul Moore wrote:
> On Jul 16, 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(-)
> If there were no other issues with this patch I would have just fixed
> this up during the merge (I did it in my review branch already), but
> since you're no longer dependent on the LSM init rework changes (and
> I've dropped the subj/obj counting in the latest revision), just go
> ahead and base your next revision on the audit tree or Linus' tree as
> one normally would.
OK. Can I assume that the dependency on the init changes has dropped?
>> diff --git a/kernel/audit.c b/kernel/audit.c
>> index 226c8ae00d04..c7dea6bfacdd 100644
>> --- a/kernel/audit.c
>> +++ b/kernel/audit.c
> ..
>
>> +/**
>> + * audit_log_subj_ctx - Add LSM subject information
>> + * @ab: audit_buffer
>> + * @prop: LSM subject properties.
>> + *
>> + * Add a subj= field and, if necessary, a AUDIT_MAC_TASK_CONTEXTS record.
>> + */
>> +int audit_log_subj_ctx(struct audit_buffer *ab, struct lsm_prop *prop)
>> {
>> - struct lsm_prop prop;
>> struct lsm_context ctx;
>> + char *space = "";
>> int error;
>> + int i;
>>
>> - security_current_getlsmprop_subj(&prop);
>> - if (!lsmprop_is_set(&prop))
>> + security_current_getlsmprop_subj(prop);
>> + if (!lsmprop_is_set(prop))
>> return 0;
>>
>> - error = security_lsmprop_to_secctx(&prop, &ctx, LSM_ID_UNDEF);
>> - if (error < 0) {
>> - if (error != -EINVAL)
>> - goto error_path;
>> + if (audit_subj_secctx_cnt < 2) {
>> + error = security_lsmprop_to_secctx(prop, &ctx, LSM_ID_UNDEF);
>> + if (error < 0) {
>> + if (error != -EINVAL)
>> + goto error_path;
>> + return 0;
>> + }
>> + audit_log_format(ab, " subj=%s", ctx.context);
>> + security_release_secctx(&ctx);
>> return 0;
>> }
>> -
>> - audit_log_format(ab, " subj=%s", ctx.context);
>> - security_release_secctx(&ctx);
>> + /* Multiple LSMs provide contexts. Include an aux record. */
>> + audit_log_format(ab, " subj=?");
>> + error = audit_buffer_aux_new(ab, AUDIT_MAC_TASK_CONTEXTS);
>> + if (error)
>> + goto error_path;
>> +
>> + for (i = 0; i < audit_subj_secctx_cnt; i++) {
>> + error = security_lsmprop_to_secctx(prop, &ctx,
>> + audit_subj_lsms[i]->id);
>> + if (error < 0) {
>> + /*
>> + * Don't print anything. An LSM like BPF could
>> + * claim to support contexts, but only do so under
>> + * certain conditions.
>> + */
>> + if (error == -EOPNOTSUPP)
>> + continue;
>> + if (error != -EINVAL)
>> + audit_panic("error in audit_log_task_context");
> Argh ... please read prior review comments a bit more carefully. As was
> pointed out in the v4 posting you're using the wrong function name here.
>
> https://lore.kernel.org/audit/fc242f4c853fee16e587e9c78e1f282e@paul-moore.com
Yeah, that was sloppy. Sorry. Will fix.
>> + } else {
>> + audit_log_format(ab, "%ssubj_%s=%s", space,
>> + audit_subj_lsms[i]->name, ctx.context);
>> + space = " ";
>> + security_release_secctx(&ctx);
>> + }
>> + }
>> + audit_buffer_aux_end(ab);
>> return 0;
>>
>> error_path:
>> - audit_panic("error in audit_log_task_context");
>> + audit_panic("error in audit_log_subj_ctx");
>> return error;
>> }
>> +EXPORT_SYMBOL(audit_log_subj_ctx);
> ..
>
>> @@ -2423,25 +2575,16 @@ int audit_signal_info(int sig, struct task_struct *t)
>> void audit_log_end(struct audit_buffer *ab)
>> {
>> struct sk_buff *skb;
>> - struct nlmsghdr *nlh;
>>
>> if (!ab)
>> return;
>>
>> - if (audit_rate_check()) {
>> - skb = ab->skb;
>> - ab->skb = NULL;
>> + while ((skb = skb_dequeue(&ab->skb_list)))
>> + __audit_log_end(skb);
>>
>> - /* setup the netlink header, see the comments in
>> - * kauditd_send_multicast_skb() for length quirks */
>> - nlh = nlmsg_hdr(skb);
>> - nlh->nlmsg_len = skb->len - NLMSG_HDRLEN;
>> -
>> - /* queue the netlink packet and poke the kauditd thread */
>> - skb_queue_tail(&audit_queue, skb);
>> + /* poke the kauditd thread */
>> + if (audit_rate_check())
>> wake_up_interruptible(&kauditd_wait);
>> - } else
>> - audit_log_lost("rate limit exceeded");
> .. here is another case where you've missed/ignored previous feedback.
> I believe this is the second revision in the history of this patchset
> where you've missed feedback; *please* try to do better Casey, stuff like
> this wastes time and drags things out longer than needed.
>
> https://lore.kernel.org/audit/fc242f4c853fee16e587e9c78e1f282e@paul-moore.com
My bad. I've been staring at this patch for over five years and sometimes
can't see where it has, hasn't or needs to be changed. I mucked this up
while pulling the indentation fixes into the separate patch.
>> audit_buffer_free(ab);
>> }
> --
> paul-moore.com
>
^ permalink raw reply
* [PATCH] x86/bpf: use bpf_capable() instead of capable(CAP_SYS_ADMIN)
From: Ondrej Mosnacek @ 2025-08-06 14:31 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: bpf, selinux, linux-security-module
Don't check against the overloaded CAP_SYS_ADMINin do_jit(), but instead
use bpf_capable(), which checks against the more granular CAP_BPF first.
Going straight to CAP_SYS_ADMIN may cause unnecessary audit log spam
under SELinux, as privileged domains using BPF would usually only be
allowed CAP_BPF and not CAP_SYS_ADMIN.
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2369326
Fixes: d4e89d212d40 ("x86/bpf: Call branch history clearing sequence on exit")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
arch/x86/net/bpf_jit_comp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 15672cb926fc1..2a825e5745ca1 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -2591,8 +2591,7 @@ emit_jmp:
seen_exit = true;
/* Update cleanup_addr */
ctx->cleanup_addr = proglen;
- if (bpf_prog_was_classic(bpf_prog) &&
- !capable(CAP_SYS_ADMIN)) {
+ if (bpf_prog_was_classic(bpf_prog) && !bpf_capable()) {
u8 *ip = image + addrs[i - 1];
if (emit_spectre_bhb_barrier(&prog, ip, bpf_prog))
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v2 0/3] Allow individual features to be locked down
From: dan.j.williams @ 2025-08-05 23:43 UTC (permalink / raw)
To: Nicolas Bouchinet, Nikolay Borisov
Cc: linux-security-module, linux-kernel, paul, serge, jmorris,
dan.j.williams
In-Reply-To: <kl4rvgnupxnz4zrwlofrawdfy23tj2ylp5s3wovnsjxkr6tbrt@x5s3avqo2e7t>
Nicolas Bouchinet wrote:
> Hi Nikolay,
>
> Thanks for you patch.
>
> Quoting Kees [1], Lockdown is "about creating a bright line between
> uid-0 and ring-0".
>
> Having a bitmap enabled Lockdown would mean that Lockdown reasons could
> be activated independently. I fear this would lead to a false sense of
> security, locking one reason alone often permits Lockdown restrictions
> bypass. i.e enforcing kernel module signature verification but not
> blocking accesses to `/dev/{k,}mem` or authorizing gkdb which can be
> used to disable the module signature enforcement.
>
> If one wants to restrict accesses to `/dev/mem`,
> `security_locked_down(LOCKDOWN_DEV_MEM)` should be sufficient.
>
> My understanding of your problem is that this locks too much for your
> usecase and you want to restrict reasons of Lockdown independently in
> case it has not been enabled in "integrity" mode by default ?
>
> Can you elaborate more on the usecases for COCO ?
Nikolay already shared some of this but the succinct answer is that COCO
breaks the fundamental expectations of /dev/mem that the only
requirement to map memory is to install a page table entry for it.
For COCO an additional step is needed to decide if the memory is private
to the COCO guest VM (cVM) or shared with the host VMM. If it is private
it additionally must be "accepted" by the cVM before it can be mapped.
/dev/mem allows uncoordinated mapping attempts and at present causes
memory protection violations because the /dev/mem backend attetmps to
map it as shared, but another part of the kernel expects it is only
mapped as cVM-private.
The attempt to communicate the mapping type, control for the acceptance
status resulted in a pile of hacks, or even just add another
COCO-specific check near the existing
"security_locked_down(LOCKDOWN_DEV_MEM)" check were met with "please
just use LOCKDOWN_DEV_MEM" directly and be done.
^ permalink raw reply
* Re: [PATCH v5 4/5] Audit: Fix indentation in audit_log_exit
From: Casey Schaufler @ 2025-08-05 23:06 UTC (permalink / raw)
To: Paul Moore, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux, Casey Schaufler
In-Reply-To: <d5f0d7a5edea8511ab4467e0fb225b8b@paul-moore.com>
On 8/5/2025 12:39 PM, Paul Moore wrote:
> On Jul 16, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Fix two indentation errors in audit_log_exit().
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>> kernel/auditsc.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
> As this is indepdendent of all the other changes in this patchset, I'm
> going to merge this into audit/dev-staging now and audit/dev later when
> the merge window is closed.
Spiffy. Thank You.
>
>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>> index 322d4e27f28e..84173d234d4a 100644
>> --- a/kernel/auditsc.c
>> +++ b/kernel/auditsc.c
>> @@ -1780,15 +1780,16 @@ static void audit_log_exit(void)
>> axs->target_sessionid[i],
>> &axs->target_ref[i],
>> axs->target_comm[i]))
>> - call_panic = 1;
>> + call_panic = 1;
>> }
>>
>> if (context->target_pid &&
>> audit_log_pid_context(context, context->target_pid,
>> context->target_auid, context->target_uid,
>> context->target_sessionid,
>> - &context->target_ref, context->target_comm))
>> - call_panic = 1;
>> + &context->target_ref,
>> + context->target_comm))
>> + call_panic = 1;
>>
>> if (context->pwd.dentry && context->pwd.mnt) {
>> ab = audit_log_start(context, GFP_KERNEL, AUDIT_CWD);
>> --
>> 2.50.1
> --
> paul-moore.com
^ permalink raw reply
* Re: [PATCH v2 0/3] Allow individual features to be locked down
From: dan.j.williams @ 2025-08-05 23:28 UTC (permalink / raw)
To: xiujianfeng, Nikolay Borisov, Nicolas Bouchinet
Cc: linux-security-module, linux-kernel, paul, serge, jmorris,
dan.j.williams
In-Reply-To: <42b2cf1b-417e-1594-d525-f4c84f7405b0@huawei.com>
xiujianfeng wrote:
>
>
> On 2025/7/29 20:25, Nikolay Borisov wrote:
> >
> >
> > On 29.07.25 г. 15:16 ч., Nicolas Bouchinet wrote:
> >> Hi Nikolay,
> >>
> >> Thanks for you patch.
> >>
> >> Quoting Kees [1], Lockdown is "about creating a bright line between
> >> uid-0 and ring-0".
> >>
> >> Having a bitmap enabled Lockdown would mean that Lockdown reasons could
> >> be activated independently. I fear this would lead to a false sense of
> >> security, locking one reason alone often permits Lockdown restrictions
> >> bypass. i.e enforcing kernel module signature verification but not
> >> blocking accesses to `/dev/{k,}mem` or authorizing gkdb which can be
> >> used to disable the module signature enforcement.
> >>
> >> If one wants to restrict accesses to `/dev/mem`,
> >> `security_locked_down(LOCKDOWN_DEV_MEM)` should be sufficient.
> >>
> >> My understanding of your problem is that this locks too much for your
> >> usecase and you want to restrict reasons of Lockdown independently in
> >> case it has not been enabled in "integrity" mode by default ?
> >>
> >> Can you elaborate more on the usecases for COCO ?
> >
> > Initially this patchset was supposed to allow us selectively disable
> > /dev/iomem access in a CoCo context [0]. As evident from Dan's initial
> > response that point pretty much became moot as the issue was fixed in a
> > different way. However, later [1] he came back and said that actually
> > this patch could be useful in a similar context. So This v2 is
> > essentially following up on that.
>
> Hi Nikolay,
>
> I share a similar view with Nicolas, namely that using a bitmap
> implementation would compromise the goal of Lockdown.
>
> After reading the threads below, I understand you aim is to block user
> access to /dev/mem, but without having Lockdown integrity mode enabled
> to block other reasons, right? How about using BPF LSM? It seems it
> could address your requirements.
BPF LSM does not seem suitable for the main concern which is that arch
code needs hard guarantess that certain code paths are disabled. For
Confidential Computing it needs to know that userspace access of
/dev/mem is always disabled.
This is a functional concern, not a security concern. Both Arnd [1] and
Greg [2] lamented needing new hacks to achieve the same outcome as just
reusing the existing security_locked_down() checks. The
SECURITY_LOCKDOWN_LSM already has /sys/kernel/security/lockdown ABI for
communicating built-in and now kernel-internal sources of locked
functionality.
[1]: http://lore.kernel.org/0bdb1876-0cb3-4632-910b-2dc191902e3e@app.fastmail.com
[2]: http://lore.kernel.org/2025043025-cathouse-headlamp-7bde@gregkh
^ permalink raw reply
* Re: [PATCH v2 3/3] lockdown: Use snprintf in lockdown_read
From: dan.j.williams @ 2025-08-05 22:30 UTC (permalink / raw)
To: Nikolay Borisov, linux-security-module
Cc: linux-kernel, paul, serge, jmorris, dan.j.williams,
Nikolay Borisov
In-Reply-To: <20250728111517.134116-4-nik.borisov@suse.com>
Nikolay Borisov wrote:
> Since individual features are now locked down separately ensure that if
> the printing code is change to list them a buffer overrun won't be
> introduced. As per Serge's recommendation switch from using sprintf to
> using snprintf and return EINVAL in case longer than 80 char string hasi
> to be printed.
I would have expected this safety to come before patch1, but it also
feels like the maximum buffer size could be calculated at compile time
to make the maximum output always fit.
^ permalink raw reply
* Re: [PATCH v2 1/3] lockdown: Switch implementation to using bitmap
From: dan.j.williams @ 2025-08-05 22:18 UTC (permalink / raw)
To: Nikolay Borisov, linux-security-module
Cc: linux-kernel, paul, serge, jmorris, dan.j.williams,
Nikolay Borisov
In-Reply-To: <20250728111517.134116-2-nik.borisov@suse.com>
Nikolay Borisov wrote:
> Tracking the lockdown at the depth granularity rather than at the
> individual is somewhat inflexible as it provides an "all or nothing"
> approach. Instead there are use cases where it will be useful to be
> able to lockdown individual features - TDX for example wants to disable
> access to just /dev/mem.
>
> To accommodate this use case switch the internal implementation to using
> a bitmap so that individual lockdown features can be turned on. At the
> same time retain the existing semantic where
> INTEGRITY_MAX/CONFIDENTIALITY_MAX are treated as wildcards meaning "lock
> everything below me".
>
> Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
> ---
> security/lockdown/lockdown.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index cf83afa1d879..5014d18c423f 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -10,12 +10,13 @@
> * 2 of the Licence, or (at your option) any later version.
> */
>
> +#include <linux/bitmap.h>
> #include <linux/security.h>
> #include <linux/export.h>
> #include <linux/lsm_hooks.h>
> #include <uapi/linux/lsm.h>
>
> -static enum lockdown_reason kernel_locked_down;
> +static DECLARE_BITMAP(kernel_locked_down, LOCKDOWN_CONFIDENTIALITY_MAX);
>
> static const enum lockdown_reason lockdown_levels[] = {LOCKDOWN_NONE,
> LOCKDOWN_INTEGRITY_MAX,
> @@ -26,10 +27,15 @@ static const enum lockdown_reason lockdown_levels[] = {LOCKDOWN_NONE,
> */
> static int lock_kernel_down(const char *where, enum lockdown_reason level)
> {
> - if (kernel_locked_down >= level)
> - return -EPERM;
So now attempts to reduce security return "success" where previously
they get permission denied?
I think that is an unforunate side effect of trying to have this one
function handle both levels and individual features.
> - kernel_locked_down = level;
> + if (level > LOCKDOWN_CONFIDENTIALITY_MAX)
> + return -EINVAL;
> +
> + if (level == LOCKDOWN_INTEGRITY_MAX || level == LOCKDOWN_CONFIDENTIALITY_MAX)
> + bitmap_set(kernel_locked_down, 1, level);
> + else
> + bitmap_set(kernel_locked_down, level, 1);
> +
The individual case probably deserves its own interface given all
current kernels expect levels and the future callers probably want to
skip the pr_notice() below given only piecemeal features are being
disabled.
You might even special case just LOCKDOWN_DEV_MEM for now as the only
once that can be indepdently set by an internal caller.
^ permalink raw reply
* Re: [PATCH v5 4/5] Audit: Fix indentation in audit_log_exit
From: Paul Moore @ 2025-08-05 19:39 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: <20250716212731.31628-5-casey@schaufler-ca.com>
On Jul 16, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Fix two indentation errors in audit_log_exit().
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> kernel/auditsc.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
As this is indepdendent of all the other changes in this patchset, I'm
going to merge this into audit/dev-staging now and audit/dev later when
the merge window is closed.
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 322d4e27f28e..84173d234d4a 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1780,15 +1780,16 @@ static void audit_log_exit(void)
> axs->target_sessionid[i],
> &axs->target_ref[i],
> axs->target_comm[i]))
> - call_panic = 1;
> + call_panic = 1;
> }
>
> if (context->target_pid &&
> audit_log_pid_context(context, context->target_pid,
> context->target_auid, context->target_uid,
> context->target_sessionid,
> - &context->target_ref, context->target_comm))
> - call_panic = 1;
> + &context->target_ref,
> + context->target_comm))
> + call_panic = 1;
>
> if (context->pwd.dentry && context->pwd.mnt) {
> ab = audit_log_start(context, GFP_KERNEL, AUDIT_CWD);
> --
> 2.50.1
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v5 3/5] Audit: Add record for multiple task security contexts
From: Paul Moore @ 2025-08-05 19:39 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: <20250716212731.31628-4-casey@schaufler-ca.com>
On Jul 16, 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(-)
If there were no other issues with this patch I would have just fixed
this up during the merge (I did it in my review branch already), but
since you're no longer dependent on the LSM init rework changes (and
I've dropped the subj/obj counting in the latest revision), just go
ahead and base your next revision on the audit tree or Linus' tree as
one normally would.
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 226c8ae00d04..c7dea6bfacdd 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
...
> +/**
> + * audit_log_subj_ctx - Add LSM subject information
> + * @ab: audit_buffer
> + * @prop: LSM subject properties.
> + *
> + * Add a subj= field and, if necessary, a AUDIT_MAC_TASK_CONTEXTS record.
> + */
> +int audit_log_subj_ctx(struct audit_buffer *ab, struct lsm_prop *prop)
> {
> - struct lsm_prop prop;
> struct lsm_context ctx;
> + char *space = "";
> int error;
> + int i;
>
> - security_current_getlsmprop_subj(&prop);
> - if (!lsmprop_is_set(&prop))
> + security_current_getlsmprop_subj(prop);
> + if (!lsmprop_is_set(prop))
> return 0;
>
> - error = security_lsmprop_to_secctx(&prop, &ctx, LSM_ID_UNDEF);
> - if (error < 0) {
> - if (error != -EINVAL)
> - goto error_path;
> + if (audit_subj_secctx_cnt < 2) {
> + error = security_lsmprop_to_secctx(prop, &ctx, LSM_ID_UNDEF);
> + if (error < 0) {
> + if (error != -EINVAL)
> + goto error_path;
> + return 0;
> + }
> + audit_log_format(ab, " subj=%s", ctx.context);
> + security_release_secctx(&ctx);
> return 0;
> }
> -
> - audit_log_format(ab, " subj=%s", ctx.context);
> - security_release_secctx(&ctx);
> + /* Multiple LSMs provide contexts. Include an aux record. */
> + audit_log_format(ab, " subj=?");
> + error = audit_buffer_aux_new(ab, AUDIT_MAC_TASK_CONTEXTS);
> + if (error)
> + goto error_path;
> +
> + for (i = 0; i < audit_subj_secctx_cnt; i++) {
> + error = security_lsmprop_to_secctx(prop, &ctx,
> + audit_subj_lsms[i]->id);
> + if (error < 0) {
> + /*
> + * Don't print anything. An LSM like BPF could
> + * claim to support contexts, but only do so under
> + * certain conditions.
> + */
> + if (error == -EOPNOTSUPP)
> + continue;
> + if (error != -EINVAL)
> + audit_panic("error in audit_log_task_context");
Argh ... please read prior review comments a bit more carefully. As was
pointed out in the v4 posting you're using the wrong function name here.
https://lore.kernel.org/audit/fc242f4c853fee16e587e9c78e1f282e@paul-moore.com
> + } else {
> + audit_log_format(ab, "%ssubj_%s=%s", space,
> + audit_subj_lsms[i]->name, ctx.context);
> + space = " ";
> + security_release_secctx(&ctx);
> + }
> + }
> + audit_buffer_aux_end(ab);
> return 0;
>
> error_path:
> - audit_panic("error in audit_log_task_context");
> + audit_panic("error in audit_log_subj_ctx");
> return error;
> }
> +EXPORT_SYMBOL(audit_log_subj_ctx);
...
> @@ -2423,25 +2575,16 @@ int audit_signal_info(int sig, struct task_struct *t)
> void audit_log_end(struct audit_buffer *ab)
> {
> struct sk_buff *skb;
> - struct nlmsghdr *nlh;
>
> if (!ab)
> return;
>
> - if (audit_rate_check()) {
> - skb = ab->skb;
> - ab->skb = NULL;
> + while ((skb = skb_dequeue(&ab->skb_list)))
> + __audit_log_end(skb);
>
> - /* setup the netlink header, see the comments in
> - * kauditd_send_multicast_skb() for length quirks */
> - nlh = nlmsg_hdr(skb);
> - nlh->nlmsg_len = skb->len - NLMSG_HDRLEN;
> -
> - /* queue the netlink packet and poke the kauditd thread */
> - skb_queue_tail(&audit_queue, skb);
> + /* poke the kauditd thread */
> + if (audit_rate_check())
> wake_up_interruptible(&kauditd_wait);
> - } else
> - audit_log_lost("rate limit exceeded");
... here is another case where you've missed/ignored previous feedback.
I believe this is the second revision in the history of this patchset
where you've missed feedback; *please* try to do better Casey, stuff like
this wastes time and drags things out longer than needed.
https://lore.kernel.org/audit/fc242f4c853fee16e587e9c78e1f282e@paul-moore.com
> audit_buffer_free(ab);
> }
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v2 08/13] bpf: Implement signature verification for BPF programs
From: Blaise Boscaccy @ 2025-08-05 18:28 UTC (permalink / raw)
To: KP Singh, bpf, linux-security-module
Cc: paul, kys, ast, daniel, andrii, KP Singh, James.Bottomley, wufan
In-Reply-To: <20250721211958.1881379-9-kpsingh@kernel.org>
KP Singh <kpsingh@kernel.org> writes:
> This patch extends the BPF_PROG_LOAD command by adding three new fields
> to `union bpf_attr` in the user-space API:
>
> - signature: A pointer to the signature blob.
> - signature_size: The size of the signature blob.
> - keyring_id: The serial number of a loaded kernel keyring (e.g.,
> the user or session keyring) containing the trusted public keys.
>
> When a BPF program is loaded with a signature, the kernel:
>
> 1. Retrieves the trusted keyring using the provided `keyring_id`.
> 2. Verifies the supplied signature against the BPF program's
> instruction buffer.
> 3. If the signature is valid and was generated by a key in the trusted
> keyring, the program load proceeds.
> 4. If no signature is provided, the load proceeds as before, allowing
> for backward compatibility. LSMs can chose to restrict unsigned
> programs and implement a security policy.
> 5. If signature verification fails for any reason,
> the program is not loaded.
[...]
The following is what we propose to build on top of this to implement
in-kernel hash chain verification. This allows for signature
verification of arbitrary maps and isn't coupled to light-skeletons or
any specific implementation.
From: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
Date: Mon, 28 Jul 2025 08:14:57 -0700
Subject: bpf: Add hash chain signature support for arbitrary maps
This patch introduces hash chain support for signature verification of
arbitrary bpf map objects which was described here:
https://lore.kernel.org/linux-security-module/20250721211958.1881379-1-kpsingh@kernel.org/
The UAPI is extended to allow for in-kernel checking of maps passed in
via the fd_array. A hash chain is constructed from the maps, in order
specified by the signature_maps field. The hash chain is terminated
with the hash of the program itself.
Signed-off-by: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
---
include/uapi/linux/bpf.h | 6 +++
kernel/bpf/syscall.c | 75 ++++++++++++++++++++++++++++++++--
tools/include/uapi/linux/bpf.h | 6 +++
3 files changed, 83 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index b42c3740e053e..c83f2a34674fd 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1617,6 +1617,12 @@ union bpf_attr {
* verification.
*/
__u32 keyring_id;
+ /* Pointer to a buffer containing the maps used in the signature
+ * hash chain of the BPF program.
+ */
+ __aligned_u64 signature_maps;
+ /* Size of the signature maps buffer. */
+ __u32 signature_maps_size;
};
struct { /* anonymous struct used by BPF_OBJ_* commands */
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 10fd3ea5d91fd..f7e9bcabd9dcc 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2780,15 +2780,36 @@ static bool is_perfmon_prog_type(enum bpf_prog_type prog_type)
}
}
+static inline int bpf_map_get_hash(int map_fd, void *buffer)
+{
+ struct bpf_map *map;
+
+ CLASS(fd, f)(map_fd);
+ map = __bpf_map_get(f);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ if (!map->ops->map_get_hash)
+ return -EINVAL;
+
+ return map->ops->map_get_hash(map, SHA256_DIGEST_SIZE, buffer);
+}
+
static noinline int bpf_prog_verify_signature(struct bpf_prog *prog,
union bpf_attr *attr,
bool is_kernel)
{
bpfptr_t usig = make_bpfptr(attr->signature, is_kernel);
- struct bpf_dynptr_kern sig_ptr, insns_ptr;
+ bpfptr_t umaps;
+ struct bpf_dynptr_kern sig_ptr, insns_ptr, hash_ptr;
struct bpf_key *key = NULL;
void *sig;
+ int *maps;
+ int map_fd;
int err = 0;
+ u64 buffer[8];
+ u64 hash[4];
+ int n;
if (system_keyring_id_check(attr->keyring_id) == 0)
key = bpf_lookup_system_key(attr->keyring_id);
@@ -2808,16 +2829,62 @@ static noinline int bpf_prog_verify_signature(struct bpf_prog *prog,
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);
+ if (!attr->signature_maps_size) {
+ err = bpf_verify_pkcs7_signature((struct bpf_dynptr *)&insns_ptr,
+ (struct bpf_dynptr *)&sig_ptr, key);
+ } else {
+ bpf_dynptr_init(&hash_ptr, hash, BPF_DYNPTR_TYPE_LOCAL, 0,
+ sizeof(hash));
+ umaps = make_bpfptr(attr->signature_maps, is_kernel);
+ maps = kvmemdup_bpfptr(umaps, attr->signature_maps_size * sizeof(*maps));
+ if (!maps) {
+ err = -ENOMEM;
+ goto out;
+ }
+ n = attr->signature_maps_size - 1;
+ err = copy_from_bpfptr_offset(&map_fd, make_bpfptr(attr->fd_array, is_kernel),
+ maps[n] * sizeof(map_fd),
+ sizeof(map_fd));
+ if (err < 0)
+ goto free_maps;
+
+ err = bpf_map_get_hash(map_fd, hash);
+ if (err != 0)
+ goto free_maps;
+
+ n--;
+ while (n >= 0) {
+ memcpy(buffer, hash, sizeof(hash));
+ err = copy_from_bpfptr_offset(&map_fd,
+ make_bpfptr(attr->fd_array, is_kernel),
+ maps[n] * sizeof(map_fd),
+ sizeof(map_fd));
+ if (err < 0)
+ goto free_maps;
+
+ err = bpf_map_get_hash(map_fd, buffer+4);
+ if (err != 0)
+ goto free_maps;
+ sha256((u8 *)buffer, sizeof(buffer), (u8 *)&hash);
+ n--;
+ }
+ sha256((u8 *)prog->insnsi, prog->len * sizeof(struct bpf_insn), (u8 *)&buffer);
+ memcpy(buffer+4, hash, sizeof(hash));
+ sha256((u8 *)buffer, sizeof(buffer), (u8 *)&hash);
+ err = bpf_verify_pkcs7_signature((struct bpf_dynptr *)&hash_ptr,
+ (struct bpf_dynptr *)&sig_ptr, key);
+free_maps:
+ kvfree(maps);
+ }
+out:
bpf_key_put(key);
kvfree(sig);
return err;
}
/* last field in 'union bpf_attr' used by this command */
-#define BPF_PROG_LOAD_LAST_FIELD keyring_id
+#define BPF_PROG_LOAD_LAST_FIELD signature_maps_size
static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
{
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index b42c3740e053e..c83f2a34674fd 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1617,6 +1617,12 @@ union bpf_attr {
* verification.
*/
__u32 keyring_id;
+ /* Pointer to a buffer containing the maps used in the signature
+ * hash chain of the BPF program.
+ */
+ __aligned_u64 signature_maps;
+ /* Size of the signature maps buffer. */
+ __u32 signature_maps_size;
};
struct { /* anonymous struct used by BPF_OBJ_* commands */
--
2.48.1
And here is a reference usage implementation:
From: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
Date: Mon, 28 Jul 2025 21:30:56 -0700
Subject: libbpf: Add hash chain signing support to light skeletons.
This patch introduces a hash chain signing support for light-skeleton
assets. A new flag '-M' is added which constructs a hash chain with
the loader program and the target payload.
Signed-off-by: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
---
tools/bpf/bpftool/gen.c | 25 +++++++++++++++++++++++++
tools/bpf/bpftool/main.c | 8 +++++++-
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/sign.c | 17 ++++++++++++++---
tools/lib/bpf/libbpf.h | 3 ++-
tools/lib/bpf/skel_internal.h | 6 +++++-
6 files changed, 54 insertions(+), 6 deletions(-)
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index ab6fc86598ad3..e660fbc701c5d 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -699,6 +699,9 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
if (sign_progs)
opts.gen_hash = true;
+ if (sign_maps)
+ opts.sign_maps = true;
+
err = bpf_object__gen_loader(obj, &opts);
if (err)
return err;
@@ -793,6 +796,8 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
if (sign_progs) {
sopts.insns = opts.insns;
sopts.insns_sz = opts.insns_sz;
+ sopts.data = opts.data;
+ sopts.data_sz = opts.data_sz;
sopts.excl_prog_hash = prog_sha;
sopts.excl_prog_hash_sz = sizeof(prog_sha);
sopts.signature = sig_buf;
@@ -821,6 +826,13 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
\n\
\";\n");
+ if (sign_maps) {
+ codegen("\
+ \n\
+ static const int opts_signature_maps[1] __attribute__((__aligned__(8))) = {0}; \n\
+ ");
+ }
+
codegen("\
\n\
opts.signature = (void *)opts_sig; \n\
@@ -829,6 +841,19 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
opts.excl_prog_hash_sz = sizeof(opts_excl_hash) - 1; \n\
opts.keyring_id = KEY_SPEC_SESSION_KEYRING; \n\
");
+ if (sign_maps) {
+ codegen("\
+ \n\
+ opts.signature_maps = (void *)opts_signature_maps; \n\
+ opts.signature_maps_sz = 1; \n\
+ ");
+ } else {
+ codegen("\
+ \n\
+ opts.signature_maps = (void *)NULL; \n\
+ opts.signature_maps_sz = 0; \n\
+ ");
+ }
}
codegen("\
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index fc25bb390ec71..287e8205494cb 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -34,6 +34,7 @@ bool use_loader;
struct btf *base_btf;
struct hashmap *refs_table;
bool sign_progs;
+bool sign_maps;
const char *private_key_path;
const char *cert_path;
@@ -477,7 +478,7 @@ int main(int argc, char **argv)
bin_name = "bpftool";
opterr = 0;
- while ((opt = getopt_long(argc, argv, "VhpjfLmndSi:k:B:l",
+ while ((opt = getopt_long(argc, argv, "VhpjfLmndSMi:k:B:l",
options, NULL)) >= 0) {
switch (opt) {
case 'V':
@@ -527,6 +528,11 @@ int main(int argc, char **argv)
sign_progs = true;
use_loader = true;
break;
+ case 'M':
+ sign_maps = true;
+ sign_progs = true;
+ use_loader = true;
+ break;
case 'k':
private_key_path = optarg;
break;
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index f921af3cda87f..805c3d87a1330 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -92,6 +92,7 @@ extern bool use_loader;
extern struct btf *base_btf;
extern struct hashmap *refs_table;
extern bool sign_progs;
+extern bool sign_maps;
extern const char *private_key_path;
extern const char *cert_path;
diff --git a/tools/bpf/bpftool/sign.c b/tools/bpf/bpftool/sign.c
index f0b5dd10a46b2..d5514b7d2b82d 100644
--- a/tools/bpf/bpftool/sign.c
+++ b/tools/bpf/bpftool/sign.c
@@ -22,6 +22,7 @@
#include <errno.h>
#include <bpf/skel_internal.h>
+#include <bpf/libbpf_internal.h>
#include "main.h"
@@ -129,8 +130,18 @@ int bpftool_prog_sign(struct bpf_load_and_run_opts *opts)
long actual_sig_len = 0;
X509 *x509 = NULL;
int err = 0;
-
- bd_in = BIO_new_mem_buf(opts->insns, opts->insns_sz);
+ unsigned char hash[SHA256_DIGEST_LENGTH * 2];
+ unsigned char term[SHA256_DIGEST_LENGTH];
+
+ if (sign_maps) {
+ libbpf_sha256(opts->insns, opts->insns_sz, hash, SHA256_DIGEST_LENGTH);
+ libbpf_sha256(opts->data, opts->data_sz, hash + SHA256_DIGEST_LENGTH,
+ SHA256_DIGEST_LENGTH);
+ libbpf_sha256(hash, sizeof(hash), term, sizeof(term));
+ bd_in = BIO_new_mem_buf(term, sizeof(term));
+ } else {
+ bd_in = BIO_new_mem_buf(opts->insns, opts->insns_sz);
+ }
if (!bd_in) {
err = -ENOMEM;
goto cleanup;
@@ -171,7 +182,7 @@ int bpftool_prog_sign(struct bpf_load_and_run_opts *opts)
EVP_Digest(opts->insns, opts->insns_sz, opts->excl_prog_hash,
&opts->excl_prog_hash_sz, EVP_sha256(), NULL);
- bd_out = BIO_new(BIO_s_mem());
+ bd_out = BIO_new(BIO_s_mem());
if (!bd_out) {
err = -ENOMEM;
goto cleanup;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 7cad8470d9ebe..aad0288cd05e3 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1827,9 +1827,10 @@ struct gen_loader_opts {
__u32 data_sz;
__u32 insns_sz;
bool gen_hash;
+ bool sign_maps;
};
-#define gen_loader_opts__last_field gen_hash
+#define gen_loader_opts__last_field sign_maps
LIBBPF_API int bpf_object__gen_loader(struct bpf_object *obj,
struct gen_loader_opts *opts);
diff --git a/tools/lib/bpf/skel_internal.h b/tools/lib/bpf/skel_internal.h
index 5b6d1b09dc8a6..c25a4f1308e44 100644
--- a/tools/lib/bpf/skel_internal.h
+++ b/tools/lib/bpf/skel_internal.h
@@ -74,6 +74,8 @@ struct bpf_load_and_run_opts {
__u32 keyring_id;
void * excl_prog_hash;
__u32 excl_prog_hash_sz;
+ const int *signature_maps;
+ __u32 signature_maps_sz;
};
long kern_sys_bpf(__u32 cmd, void *attr, __u32 attr_size);
@@ -351,7 +353,7 @@ static inline int skel_map_freeze(int fd)
static inline int bpf_load_and_run(struct bpf_load_and_run_opts *opts)
{
- const size_t prog_load_attr_sz = offsetofend(union bpf_attr, keyring_id);
+ const size_t prog_load_attr_sz = offsetofend(union bpf_attr, signature_maps_size);
const size_t test_run_attr_sz = offsetofend(union bpf_attr, test);
int map_fd = -1, prog_fd = -1, key = 0, err;
union bpf_attr attr;
@@ -394,6 +396,8 @@ static inline int bpf_load_and_run(struct bpf_load_and_run_opts *opts)
#ifndef __KERNEL__
attr.signature = (long) opts->signature;
attr.signature_size = opts->signature_sz;
+ attr.signature_maps = (long) opts->signature_maps;
+ attr.signature_maps_size = opts->signature_maps_sz;
#else
if (opts->signature || opts->signature_sz)
pr_warn("signatures are not supported from bpf_preload\n");
--
2.48.1
-blaise
^ permalink raw reply related
* Re: [PATCH v2 0/3] Allow individual features to be locked down
From: Nikolay Borisov @ 2025-08-05 8:03 UTC (permalink / raw)
To: xiujianfeng, Nicolas Bouchinet
Cc: linux-security-module, linux-kernel, paul, serge, jmorris,
dan.j.williams
In-Reply-To: <42b2cf1b-417e-1594-d525-f4c84f7405b0@huawei.com>
On 8/5/25 09:57, xiujianfeng wrote:
>
>
> On 2025/7/29 20:25, Nikolay Borisov wrote:
>>
>>
>> On 29.07.25 г. 15:16 ч., Nicolas Bouchinet wrote:
>>> Hi Nikolay,
>>>
>>> Thanks for you patch.
>>>
>>> Quoting Kees [1], Lockdown is "about creating a bright line between
>>> uid-0 and ring-0".
>>>
>>> Having a bitmap enabled Lockdown would mean that Lockdown reasons could
>>> be activated independently. I fear this would lead to a false sense of
>>> security, locking one reason alone often permits Lockdown restrictions
>>> bypass. i.e enforcing kernel module signature verification but not
>>> blocking accesses to `/dev/{k,}mem` or authorizing gkdb which can be
>>> used to disable the module signature enforcement.
>>>
>>> If one wants to restrict accesses to `/dev/mem`,
>>> `security_locked_down(LOCKDOWN_DEV_MEM)` should be sufficient.
>>>
>>> My understanding of your problem is that this locks too much for your
>>> usecase and you want to restrict reasons of Lockdown independently in
>>> case it has not been enabled in "integrity" mode by default ?
>>>
>>> Can you elaborate more on the usecases for COCO ?
>>
>> Initially this patchset was supposed to allow us selectively disable
>> /dev/iomem access in a CoCo context [0]. As evident from Dan's initial
>> response that point pretty much became moot as the issue was fixed in a
>> different way. However, later [1] he came back and said that actually
>> this patch could be useful in a similar context. So This v2 is
>> essentially following up on that.
>
> Hi Nikolay,
>
> I share a similar view with Nicolas, namely that using a bitmap
> implementation would compromise the goal of Lockdown.
>
> After reading the threads below, I understand you aim is to block user
> access to /dev/mem, but without having Lockdown integrity mode enabled
> to block other reasons, right? How about using BPF LSM? It seems it
> could address your requirements.
>
Well the use case that my change allows (barring the original issue) is
say if someone wants LOCKDOWN_INTEGRITY_MAX + 1 or 2 things from the
CONFIDENTIALY_MAX level.
>>
>>
>> [0]
>> https://lore.kernel.org/all/67f69600ed221_71fe2946f@dwillia2-xfh.jf.intel.com.notmuch/
>>
>> [1]
>> https://lore.kernel.org/all/68226ad551afd_29032945b@dwillia2-xfh.jf.intel.com.notmuch/
>>
>> <snip>
^ permalink raw reply
* Re: [PATCH v2 3/3] lockdown: Use snprintf in lockdown_read
From: Nikolay Borisov @ 2025-08-05 7:56 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: linux-security-module, linux-kernel, paul, jmorris,
dan.j.williams
In-Reply-To: <aIdvhOEiiPMDY4gW@mail.hallyn.com>
On 7/28/25 15:39, Serge E. Hallyn wrote:
> On Mon, Jul 28, 2025 at 02:15:17PM +0300, Nikolay Borisov wrote:
>> Since individual features are now locked down separately ensure that if
>> the printing code is change to list them a buffer overrun won't be
>> introduced. As per Serge's recommendation switch from using sprintf to
>> using snprintf and return EINVAL in case longer than 80 char string hasi
>> to be printed.
>>
>> Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
>
> Thanks, 2 comments below
>
>> ---
>> security/lockdown/lockdown.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
>> index 412184121279..ed1dde41d7d3 100644
>> --- a/security/lockdown/lockdown.c
>> +++ b/security/lockdown/lockdown.c
>> @@ -112,11 +112,19 @@ static ssize_t lockdown_read(struct file *filp, char __user *buf, size_t count,
>>
>> if (lockdown_reasons[level]) {
>> const char *label = lockdown_reasons[level];
>> + int ret = 0;
>> + int write_len = 80-offset;
>
> 80 should really be a #define (and used to declare the length of temp as
> well).
ack
>
>> +
>>
>> if (test_bit(level, kernel_locked_down))
>> - offset += sprintf(temp+offset, "[%s] ", label);
>> + ret = snprintf(temp+offset, write_len, "[%s] ", label);
>> else
>> - offset += sprintf(temp+offset, "%s ", label);
>> + ret = snprintf(temp+offset, write_len, "%s ", label);
>> +
>> + if (ret < 0 || ret >= write_len)
>> + return -ENOMEM;
>
> is ENOMEM right here, or should it be something like EINVAL or E2BIG?
Indeed, I was wondering the same when writing the code initially. I
guess either einval/e2big are both well fitted in this case. If I had to
choose I'd likely go with E2BIG since it's less prevalent than einval
and if someone changes the implementation and starts getting E2BIG it
should be easier to spot where it's coming from. That'd be my only
consideration between the 2.
However, given that this series seems to be unconvincing for the
maintainer I'll defer those changes until it's decided that it's
eventually getting merged :)
>
>> +
>> + offset += ret;
>> }
>> }
>>
>> --
>> 2.34.1
>>
^ permalink raw reply
* Re: [PATCH v2 0/3] Allow individual features to be locked down
From: xiujianfeng @ 2025-08-05 6:57 UTC (permalink / raw)
To: Nikolay Borisov, Nicolas Bouchinet
Cc: linux-security-module, linux-kernel, paul, serge, jmorris,
dan.j.williams
In-Reply-To: <9b6fd06e-5438-4539-821c-6f3d5fa6b7d1@suse.com>
On 2025/7/29 20:25, Nikolay Borisov wrote:
>
>
> On 29.07.25 г. 15:16 ч., Nicolas Bouchinet wrote:
>> Hi Nikolay,
>>
>> Thanks for you patch.
>>
>> Quoting Kees [1], Lockdown is "about creating a bright line between
>> uid-0 and ring-0".
>>
>> Having a bitmap enabled Lockdown would mean that Lockdown reasons could
>> be activated independently. I fear this would lead to a false sense of
>> security, locking one reason alone often permits Lockdown restrictions
>> bypass. i.e enforcing kernel module signature verification but not
>> blocking accesses to `/dev/{k,}mem` or authorizing gkdb which can be
>> used to disable the module signature enforcement.
>>
>> If one wants to restrict accesses to `/dev/mem`,
>> `security_locked_down(LOCKDOWN_DEV_MEM)` should be sufficient.
>>
>> My understanding of your problem is that this locks too much for your
>> usecase and you want to restrict reasons of Lockdown independently in
>> case it has not been enabled in "integrity" mode by default ?
>>
>> Can you elaborate more on the usecases for COCO ?
>
> Initially this patchset was supposed to allow us selectively disable
> /dev/iomem access in a CoCo context [0]. As evident from Dan's initial
> response that point pretty much became moot as the issue was fixed in a
> different way. However, later [1] he came back and said that actually
> this patch could be useful in a similar context. So This v2 is
> essentially following up on that.
Hi Nikolay,
I share a similar view with Nicolas, namely that using a bitmap
implementation would compromise the goal of Lockdown.
After reading the threads below, I understand you aim is to block user
access to /dev/mem, but without having Lockdown integrity mode enabled
to block other reasons, right? How about using BPF LSM? It seems it
could address your requirements.
>
>
> [0]
> https://lore.kernel.org/all/67f69600ed221_71fe2946f@dwillia2-xfh.jf.intel.com.notmuch/
>
> [1]
> https://lore.kernel.org/all/68226ad551afd_29032945b@dwillia2-xfh.jf.intel.com.notmuch/
>
> <snip>
^ permalink raw reply
* Re: [GIT PULL] AppArmor updates for 6.17-rc1
From: pr-tracker-bot @ 2025-08-04 16:05 UTC (permalink / raw)
To: John Johansen; +Cc: Linus Torvalds, LKLM, open list:SECURITY SUBSYSTEM
In-Reply-To: <8d0c22fd-330e-4c13-b9e3-32a927697667@canonical.com>
The pull request you sent on Mon, 4 Aug 2025 05:11:50 -0700:
> git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor tags/apparmor-pr-2025-08-04
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/8b45c6c90af6702b2ad716e148b8bcd5231a8070
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* [GIT PULL] AppArmor updates for 6.17-rc1
From: John Johansen @ 2025-08-04 12:11 UTC (permalink / raw)
To: Linus Torvalds; +Cc: LKLM, open list:SECURITY SUBSYSTEM
Hi Linus,
There are a few patches at the top of this PR for issues fixes that
came in while I was out (I know poor timing) that I pulled in last
week after I returned. I have waited a few days for feedback and
verification on those patches beyond my own testing.
These patches have all been merge, build, and regression tested
against your tree as of yesterday. The majority of the code has had
months of testing, both in linux-next and the Ubuntu kernels, and
everything else, except the fixes I pulled in last week, has had weeks
of testing.
This PR has one major feature, it pulls in a cleaned up version of
af_unix mediation that Ubuntu has been carrying for years. It is
placed behind a new abi to ensure that it does cause policy
regressions. With pulling in the af_unix mediation there have been
cleanups and some refactoring of network socket mediation. This
accounts for the majority of the changes in the diff.
In addition there are a few improvements providing minor code
optimizations. several code cleanups, and bug fixes.
There is one Revert in the patchset for code that Eric decided he
would rather take through the crypto tree.
thanks
- john
The following changes since commit 40384c840ea1944d7c5a392e8975ed088ecf0b37:
Linux 6.13-rc1 (2024-12-01 14:28:56 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor tags/apparmor-pr-2025-08-04
for you to fetch changes up to 5f49c2d1f422c660c726ac5e0499c66c901633c2:
apparmor: fix: oops when trying to free null ruleset (2025-08-04 01:14:56 -0700)
----------------------------------------------------------------
+ Features
- improve debug printing
- carry mediation check on label (optimization)
- improve ability for compiler to optimize __begin_current_label_crit_section
- transition for a linked list of rulesets to a vector of rulesets
- don't hardcode profile signal, allow it to be set by policy
- ability to mediate caps via the state machine instead of lut
- Add Ubuntu af_unix mediation, put it behind new v9 abi
+ Cleanups
- fix typos and spelling errors
- cleanup kernel doc and code inconsistencies
- remove redundant checks/code
- remove unused variables
- Use str_yes_no() helper function
- mark tables static where appropriate
- make all generated string array headers const char *const
- refactor to doc semantics of file_perm checks
- replace macro calls to network/socket fns with explicit calls
- refactor/cleanup socket mediation code preparing for finer grained
mediation of different network families
- several updates to kernel doc comments
+ Bug fixes
- apparmor: Fix incorrect profile->signal range check
- idmap mount fixes
- policy unpack unaligned access fixes
- kfree_sensitive() where appropriate
- fix oops when freeing policy
- fix conflicting attachment resolution
- fix exec table look-ups when stacking isn't first
- fix exec auditing
- mitigate userspace generating overly large xtables
----------------------------------------------------------------
Colin Ian King (1):
apparmor: Fix incorrect profile->signal range check
Eric Biggers (1):
apparmor: use SHA-256 library API instead of crypto_shash API
Gabriel Totev (2):
apparmor: shift ouid when mediating hard links in userns
apparmor: shift uid when mediating af_unix in userns
Helge Deller (2):
apparmor: Fix 8-byte alignment for initial dfa blob streams
apparmor: Fix unaligned memory accesses in KUnit test
Jiapeng Chong (3):
apparmor: Modify mismatched function name
apparmor: Modify mismatched function name
apparmor: Remove the unused variable rules
John Johansen (34):
apparmor: Improve debug print infrastructure
apparmor: cleanup: attachment perm lookup to use lookup_perms()
apparmor: remove redundant unconfined check.
apparmor: switch signal mediation to use RULE_MEDIATES
apparmor: ensure labels with more than one entry have correct flags
apparmor: remove explicit restriction that unconfined cannot use change_hat
apparmor: cleanup: refactor file_perm() to doc semantics of some checks
apparmor: carry mediation check on label
apparmor: add additional flags to extended permission.
apparmor: add support for profiles to define the kill signal
apparmor: fix x_table_lookup when stacking is not the first entry
apparmor: add ability to mediate caps with policy state machine
apparmor: remove af_select macro
apparmor: lift kernel socket check out of critical section
apparmor: in preparation for finer networking rules rework match_prot
apparmor: add fine grained af_unix mediation
apparmor: gate make fine grained unix mediation behind v9 abi
apparmor: fix dbus permission queries to v9 ABI
apparmor: make debug_values_table static
apparmor: Document that label must be last member in struct aa_profile
apparmor: mitigate parser generating large xtables
Revert "apparmor: use SHA-256 library API instead of crypto_shash API"
apparmor: update kernel doc comments for xxx_label_crit_section
apparmor: Remove use of the double lock
apparmor: fix af_unix auditing to include all address information
apparmor: fix AA_DEBUG_LABEL()
apparmor: fix regression in fs based unix sockets when using old abi
apparmor: make sure unix socket labeling is correctly updated.
apparmor: fix kernel doc warnings for kernel test robot
apparmor: transition from a list of rules to a vector of rules
apparmor: fix: accept2 being specifie even when permission table is presnt
apparmor: fix test error: WARNING in apparmor_unix_stream_connect
apparmor: fix Regression on linux-next (next-20250721)
apparmor: fix: oops when trying to free null ruleset
Mateusz Guzik (2):
apparmor: use the condition in AA_BUG_FMT even with debug disabled
apparmor: make __begin_current_label_crit_section() indicate whether put is needed
Nathan Chancellor (2):
apparmor: Fix checking address of an array in accum_label_info()
apparmor: Remove unused variable 'sock' in __file_sock_perm()
Peng Jiang (1):
apparmor: fix documentation mismatches in val_mask_to_str and socket functions
Randy Dunlap (1):
apparmor: fix some kernel-doc issues in header files
Ryan Lee (8):
apparmor: ensure WB_HISTORY_SIZE value is a power of 2
apparmor: fix loop detection used in conflicting attachment resolution
apparmor: make all generated string array headers const char *const
apparmor: force audit on unconfined exec if info is set by find_attach
apparmor: move the "conflicting profile attachments" infostr to a const declaration
apparmor: include conflicting attachment info for confined ix/ux fallback
apparmor: force auditing of conflicting attachment execs from confined
apparmor: remove redundant perms.allow MAY_EXEC bitflag set
Tanya Agarwal (1):
apparmor: fix typos and spelling errors
Thorsten Blum (1):
apparmor: Use str_yes_no() helper function
Zilin Guan (1):
security/apparmor: use kfree_sensitive() in unpack_secmark()
security/apparmor/Makefile | 6 +-
security/apparmor/af_unix.c | 799 +++++++++++++++++++++++++++++++++
security/apparmor/apparmorfs.c | 39 +-
security/apparmor/audit.c | 2 +-
security/apparmor/capability.c | 61 ++-
security/apparmor/domain.c | 203 ++++++---
security/apparmor/file.c | 92 ++--
security/apparmor/include/af_unix.h | 55 +++
security/apparmor/include/apparmor.h | 4 +-
security/apparmor/include/audit.h | 5 +-
security/apparmor/include/capability.h | 1 +
security/apparmor/include/cred.h | 31 +-
security/apparmor/include/file.h | 11 +-
security/apparmor/include/ipc.h | 3 +
security/apparmor/include/label.h | 51 ++-
security/apparmor/include/lib.h | 46 +-
security/apparmor/include/match.h | 10 +-
security/apparmor/include/net.h | 38 +-
security/apparmor/include/path.h | 1 +
security/apparmor/include/perms.h | 8 +-
security/apparmor/include/policy.h | 59 ++-
security/apparmor/include/sig_names.h | 6 +-
security/apparmor/include/signal.h | 19 +
security/apparmor/ipc.c | 13 +-
security/apparmor/label.c | 37 +-
security/apparmor/lib.c | 114 +++++
security/apparmor/lsm.c | 468 +++++++++++++++----
security/apparmor/match.c | 23 +-
security/apparmor/mount.c | 12 +-
security/apparmor/net.c | 189 ++++++--
security/apparmor/policy.c | 93 ++--
security/apparmor/policy_compat.c | 6 +-
security/apparmor/policy_ns.c | 2 +-
security/apparmor/policy_unpack.c | 67 ++-
security/apparmor/policy_unpack_test.c | 6 +-
security/apparmor/procattr.c | 6 +-
security/apparmor/resource.c | 11 +-
security/apparmor/task.c | 11 +-
38 files changed, 2178 insertions(+), 430 deletions(-)
create mode 100644 security/apparmor/af_unix.c
create mode 100644 security/apparmor/include/af_unix.h
create mode 100644 security/apparmor/include/signal.h
^ permalink raw reply
* [PATCH AUTOSEL 5.4 9/9] securityfs: don't pin dentries twice, once is enough...
From: Sasha Levin @ 2025-08-03 21:23 UTC (permalink / raw)
To: patches, stable
Cc: Al Viro, Sasha Levin, paul, jmorris, serge, linux-security-module
In-Reply-To: <20250803212309.3549683-1-sashal@kernel.org>
From: Al Viro <viro@zeniv.linux.org.uk>
[ Upstream commit 27cd1bf1240d482e4f02ca4f9812e748f3106e4f ]
incidentally, securityfs_recursive_remove() is broken without that -
it leaks dentries, since simple_recursive_removal() does not expect
anything of that sort. It could be worked around by dput() in
remove_one() callback, but it's easier to just drop that double-get
stuff.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a clear understanding of the issue. Let me analyze this:
**YES**
This commit should be backported to stable kernel trees. Here's the
extensive analysis:
## Analysis of the Double Dentry Reference Issue
The commit fixes a clear bug in securityfs where dentries are being
pinned twice:
1. **The Bug**: In `securityfs_create_dentry()` at line 162, there's a
`dget(dentry)` call immediately after `d_instantiate(dentry, inode)`.
This creates an extra reference count on the dentry that is never
properly balanced.
2. **Impact on `securityfs_remove()`**: The traditional
`securityfs_remove()` function does call `dput(dentry)` at line 309,
which would balance the extra `dget()`. However, this only happens
for the simple remove case.
3. **Critical Impact on `securityfs_recursive_remove()`**: The newly
added `securityfs_recursive_remove()` function (added in commit
7138679ff2a2) uses `simple_recursive_removal()` which expects normal
dentry reference counting. The `simple_recursive_removal()` function
in fs/libfs.c:
- Takes its own reference with `dget(dentry)` at the beginning
- Properly releases references with `dput()` calls throughout its
execution
- Does NOT expect the dentries to have extra references
4. **The Leak**: When `securityfs_recursive_remove()` is called, the
extra reference from the `dget()` in `securityfs_create_dentry()` is
never released because:
- `simple_recursive_removal()` only releases the references it takes
- The `remove_one()` callback only calls `simple_release_fs()` but
doesn't do any `dput()`
- This results in dentries being leaked with a refcount that never
reaches zero
5. **Pattern Consistency**: Looking at similar filesystem
implementations like debugfs (fs/debugfs/inode.c), they do NOT add an
extra `dget()` after `d_instantiate()`. The pattern is simply:
```c
d_instantiate(dentry, inode);
return end_creating(dentry);
```
No extra reference is taken.
6. **Backport Criteria Met**:
- **Fixes a real bug**: Memory leak of dentries when using
`securityfs_recursive_remove()`
- **Small and contained**: Single-line removal in two locations
- **No side effects**: Removing the extra `dget()` brings the code in
line with standard VFS patterns
- **Low regression risk**: The fix makes securityfs consistent with
other similar filesystems
- **Important for stability**: Memory leaks in kernel filesystem code
can accumulate over time
The commit message clearly identifies this as a bug fix
("securityfs_recursive_remove() is broken without that - it leaks
dentries"), and the fix is minimal and correct. This makes it an ideal
candidate for stable backporting.
security/inode.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/security/inode.c b/security/inode.c
index 6c326939750d..e6e07787eec9 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -159,7 +159,6 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
inode->i_fop = fops;
}
d_instantiate(dentry, inode);
- dget(dentry);
inode_unlock(dir);
return dentry;
@@ -306,7 +305,6 @@ void securityfs_remove(struct dentry *dentry)
simple_rmdir(dir, dentry);
else
simple_unlink(dir, dentry);
- dput(dentry);
}
inode_unlock(dir);
simple_release_fs(&mount, &mount_count);
--
2.39.5
^ permalink raw reply related
* [PATCH AUTOSEL 5.10 11/11] securityfs: don't pin dentries twice, once is enough...
From: Sasha Levin @ 2025-08-03 21:22 UTC (permalink / raw)
To: patches, stable
Cc: Al Viro, Sasha Levin, paul, jmorris, serge, linux-security-module
In-Reply-To: <20250803212242.3549318-1-sashal@kernel.org>
From: Al Viro <viro@zeniv.linux.org.uk>
[ Upstream commit 27cd1bf1240d482e4f02ca4f9812e748f3106e4f ]
incidentally, securityfs_recursive_remove() is broken without that -
it leaks dentries, since simple_recursive_removal() does not expect
anything of that sort. It could be worked around by dput() in
remove_one() callback, but it's easier to just drop that double-get
stuff.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a clear understanding of the issue. Let me analyze this:
**YES**
This commit should be backported to stable kernel trees. Here's the
extensive analysis:
## Analysis of the Double Dentry Reference Issue
The commit fixes a clear bug in securityfs where dentries are being
pinned twice:
1. **The Bug**: In `securityfs_create_dentry()` at line 162, there's a
`dget(dentry)` call immediately after `d_instantiate(dentry, inode)`.
This creates an extra reference count on the dentry that is never
properly balanced.
2. **Impact on `securityfs_remove()`**: The traditional
`securityfs_remove()` function does call `dput(dentry)` at line 309,
which would balance the extra `dget()`. However, this only happens
for the simple remove case.
3. **Critical Impact on `securityfs_recursive_remove()`**: The newly
added `securityfs_recursive_remove()` function (added in commit
7138679ff2a2) uses `simple_recursive_removal()` which expects normal
dentry reference counting. The `simple_recursive_removal()` function
in fs/libfs.c:
- Takes its own reference with `dget(dentry)` at the beginning
- Properly releases references with `dput()` calls throughout its
execution
- Does NOT expect the dentries to have extra references
4. **The Leak**: When `securityfs_recursive_remove()` is called, the
extra reference from the `dget()` in `securityfs_create_dentry()` is
never released because:
- `simple_recursive_removal()` only releases the references it takes
- The `remove_one()` callback only calls `simple_release_fs()` but
doesn't do any `dput()`
- This results in dentries being leaked with a refcount that never
reaches zero
5. **Pattern Consistency**: Looking at similar filesystem
implementations like debugfs (fs/debugfs/inode.c), they do NOT add an
extra `dget()` after `d_instantiate()`. The pattern is simply:
```c
d_instantiate(dentry, inode);
return end_creating(dentry);
```
No extra reference is taken.
6. **Backport Criteria Met**:
- **Fixes a real bug**: Memory leak of dentries when using
`securityfs_recursive_remove()`
- **Small and contained**: Single-line removal in two locations
- **No side effects**: Removing the extra `dget()` brings the code in
line with standard VFS patterns
- **Low regression risk**: The fix makes securityfs consistent with
other similar filesystems
- **Important for stability**: Memory leaks in kernel filesystem code
can accumulate over time
The commit message clearly identifies this as a bug fix
("securityfs_recursive_remove() is broken without that - it leaks
dentries"), and the fix is minimal and correct. This makes it an ideal
candidate for stable backporting.
security/inode.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/security/inode.c b/security/inode.c
index 6c326939750d..e6e07787eec9 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -159,7 +159,6 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
inode->i_fop = fops;
}
d_instantiate(dentry, inode);
- dget(dentry);
inode_unlock(dir);
return dentry;
@@ -306,7 +305,6 @@ void securityfs_remove(struct dentry *dentry)
simple_rmdir(dir, dentry);
else
simple_unlink(dir, dentry);
- dput(dentry);
}
inode_unlock(dir);
simple_release_fs(&mount, &mount_count);
--
2.39.5
^ permalink raw reply related
* [PATCH AUTOSEL 5.15 15/15] securityfs: don't pin dentries twice, once is enough...
From: Sasha Levin @ 2025-08-03 21:22 UTC (permalink / raw)
To: patches, stable
Cc: Al Viro, Sasha Levin, paul, jmorris, serge, linux-security-module
In-Reply-To: <20250803212206.3548990-1-sashal@kernel.org>
From: Al Viro <viro@zeniv.linux.org.uk>
[ Upstream commit 27cd1bf1240d482e4f02ca4f9812e748f3106e4f ]
incidentally, securityfs_recursive_remove() is broken without that -
it leaks dentries, since simple_recursive_removal() does not expect
anything of that sort. It could be worked around by dput() in
remove_one() callback, but it's easier to just drop that double-get
stuff.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a clear understanding of the issue. Let me analyze this:
**YES**
This commit should be backported to stable kernel trees. Here's the
extensive analysis:
## Analysis of the Double Dentry Reference Issue
The commit fixes a clear bug in securityfs where dentries are being
pinned twice:
1. **The Bug**: In `securityfs_create_dentry()` at line 162, there's a
`dget(dentry)` call immediately after `d_instantiate(dentry, inode)`.
This creates an extra reference count on the dentry that is never
properly balanced.
2. **Impact on `securityfs_remove()`**: The traditional
`securityfs_remove()` function does call `dput(dentry)` at line 309,
which would balance the extra `dget()`. However, this only happens
for the simple remove case.
3. **Critical Impact on `securityfs_recursive_remove()`**: The newly
added `securityfs_recursive_remove()` function (added in commit
7138679ff2a2) uses `simple_recursive_removal()` which expects normal
dentry reference counting. The `simple_recursive_removal()` function
in fs/libfs.c:
- Takes its own reference with `dget(dentry)` at the beginning
- Properly releases references with `dput()` calls throughout its
execution
- Does NOT expect the dentries to have extra references
4. **The Leak**: When `securityfs_recursive_remove()` is called, the
extra reference from the `dget()` in `securityfs_create_dentry()` is
never released because:
- `simple_recursive_removal()` only releases the references it takes
- The `remove_one()` callback only calls `simple_release_fs()` but
doesn't do any `dput()`
- This results in dentries being leaked with a refcount that never
reaches zero
5. **Pattern Consistency**: Looking at similar filesystem
implementations like debugfs (fs/debugfs/inode.c), they do NOT add an
extra `dget()` after `d_instantiate()`. The pattern is simply:
```c
d_instantiate(dentry, inode);
return end_creating(dentry);
```
No extra reference is taken.
6. **Backport Criteria Met**:
- **Fixes a real bug**: Memory leak of dentries when using
`securityfs_recursive_remove()`
- **Small and contained**: Single-line removal in two locations
- **No side effects**: Removing the extra `dget()` brings the code in
line with standard VFS patterns
- **Low regression risk**: The fix makes securityfs consistent with
other similar filesystems
- **Important for stability**: Memory leaks in kernel filesystem code
can accumulate over time
The commit message clearly identifies this as a bug fix
("securityfs_recursive_remove() is broken without that - it leaks
dentries"), and the fix is minimal and correct. This makes it an ideal
candidate for stable backporting.
security/inode.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/security/inode.c b/security/inode.c
index 6c326939750d..e6e07787eec9 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -159,7 +159,6 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
inode->i_fop = fops;
}
d_instantiate(dentry, inode);
- dget(dentry);
inode_unlock(dir);
return dentry;
@@ -306,7 +305,6 @@ void securityfs_remove(struct dentry *dentry)
simple_rmdir(dir, dentry);
else
simple_unlink(dir, dentry);
- dput(dentry);
}
inode_unlock(dir);
simple_release_fs(&mount, &mount_count);
--
2.39.5
^ permalink raw reply related
* [PATCH AUTOSEL 6.1 16/16] securityfs: don't pin dentries twice, once is enough...
From: Sasha Levin @ 2025-08-03 21:21 UTC (permalink / raw)
To: patches, stable
Cc: Al Viro, Sasha Levin, paul, jmorris, serge, linux-security-module
In-Reply-To: <20250803212127.3548367-1-sashal@kernel.org>
From: Al Viro <viro@zeniv.linux.org.uk>
[ Upstream commit 27cd1bf1240d482e4f02ca4f9812e748f3106e4f ]
incidentally, securityfs_recursive_remove() is broken without that -
it leaks dentries, since simple_recursive_removal() does not expect
anything of that sort. It could be worked around by dput() in
remove_one() callback, but it's easier to just drop that double-get
stuff.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a clear understanding of the issue. Let me analyze this:
**YES**
This commit should be backported to stable kernel trees. Here's the
extensive analysis:
## Analysis of the Double Dentry Reference Issue
The commit fixes a clear bug in securityfs where dentries are being
pinned twice:
1. **The Bug**: In `securityfs_create_dentry()` at line 162, there's a
`dget(dentry)` call immediately after `d_instantiate(dentry, inode)`.
This creates an extra reference count on the dentry that is never
properly balanced.
2. **Impact on `securityfs_remove()`**: The traditional
`securityfs_remove()` function does call `dput(dentry)` at line 309,
which would balance the extra `dget()`. However, this only happens
for the simple remove case.
3. **Critical Impact on `securityfs_recursive_remove()`**: The newly
added `securityfs_recursive_remove()` function (added in commit
7138679ff2a2) uses `simple_recursive_removal()` which expects normal
dentry reference counting. The `simple_recursive_removal()` function
in fs/libfs.c:
- Takes its own reference with `dget(dentry)` at the beginning
- Properly releases references with `dput()` calls throughout its
execution
- Does NOT expect the dentries to have extra references
4. **The Leak**: When `securityfs_recursive_remove()` is called, the
extra reference from the `dget()` in `securityfs_create_dentry()` is
never released because:
- `simple_recursive_removal()` only releases the references it takes
- The `remove_one()` callback only calls `simple_release_fs()` but
doesn't do any `dput()`
- This results in dentries being leaked with a refcount that never
reaches zero
5. **Pattern Consistency**: Looking at similar filesystem
implementations like debugfs (fs/debugfs/inode.c), they do NOT add an
extra `dget()` after `d_instantiate()`. The pattern is simply:
```c
d_instantiate(dentry, inode);
return end_creating(dentry);
```
No extra reference is taken.
6. **Backport Criteria Met**:
- **Fixes a real bug**: Memory leak of dentries when using
`securityfs_recursive_remove()`
- **Small and contained**: Single-line removal in two locations
- **No side effects**: Removing the extra `dget()` brings the code in
line with standard VFS patterns
- **Low regression risk**: The fix makes securityfs consistent with
other similar filesystems
- **Important for stability**: Memory leaks in kernel filesystem code
can accumulate over time
The commit message clearly identifies this as a bug fix
("securityfs_recursive_remove() is broken without that - it leaks
dentries"), and the fix is minimal and correct. This makes it an ideal
candidate for stable backporting.
security/inode.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/security/inode.c b/security/inode.c
index 6c326939750d..e6e07787eec9 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -159,7 +159,6 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
inode->i_fop = fops;
}
d_instantiate(dentry, inode);
- dget(dentry);
inode_unlock(dir);
return dentry;
@@ -306,7 +305,6 @@ void securityfs_remove(struct dentry *dentry)
simple_rmdir(dir, dentry);
else
simple_unlink(dir, dentry);
- dput(dentry);
}
inode_unlock(dir);
simple_release_fs(&mount, &mount_count);
--
2.39.5
^ permalink raw reply related
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