* Re: [PATCH v5 2/3] lsm: introduce security_lsm_config_*_policy hooks
From: Paul Moore @ 2025-10-10 14:59 UTC (permalink / raw)
To: Casey Schaufler
Cc: Mickaël Salaün, Maxime Bélair,
linux-security-module, john.johansen, jmorris, serge, kees,
stephen.smalley.work, takedakn, penguin-kernel, song, rdunlap,
linux-api, apparmor, linux-kernel
In-Reply-To: <0c7a19cb-d270-403f-9f97-354405aba746@schaufler-ca.com>
On Wed, Aug 20, 2025 at 11:30 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 8/20/2025 7:21 AM, Mickaël Salaün wrote:
> > On Wed, Jul 09, 2025 at 10:00:55AM +0200, Maxime Bélair wrote:
> >> Define two new LSM hooks: security_lsm_config_self_policy and
> >> security_lsm_config_system_policy and wire them into the corresponding
> >> lsm_config_*_policy() syscalls so that LSMs can register a unified
> >> interface for policy management. This initial, minimal implementation
> >> only supports the LSM_POLICY_LOAD operation to limit changes.
> >>
> >> Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
> >> ---
> >> include/linux/lsm_hook_defs.h | 4 +++
> >> include/linux/security.h | 20 ++++++++++++
> >> include/uapi/linux/lsm.h | 8 +++++
> >> security/lsm_syscalls.c | 17 ++++++++--
> >> security/security.c | 60 +++++++++++++++++++++++++++++++++++
> >> 5 files changed, 107 insertions(+), 2 deletions(-)
...
> >> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
> >> index 938593dfd5da..2b9432a30cdc 100644
> >> --- a/include/uapi/linux/lsm.h
> >> +++ b/include/uapi/linux/lsm.h
> >> @@ -90,4 +90,12 @@ struct lsm_ctx {
> >> */
> >> #define LSM_FLAG_SINGLE 0x0001
> >>
> >> +/*
> >> + * LSM_POLICY_XXX definitions identify the different operations
> >> + * to configure LSM policies
> >> + */
> >> +
> >> +#define LSM_POLICY_UNDEF 0
> >> +#define LSM_POLICY_LOAD 100
> > Why the gap between 0 and 100?
>
> It's conventional in LSM syscalls to start identifiers at 100.
> No compelling reason other than to appease the LSM maintainer.
If you guys make me repeat all the reasons why, I'm going to get even
crankier than usual :-P
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v2] nbd: override creds to kernel when calling sock_{send,recv}msg()
From: Paul Moore @ 2025-10-10 15:00 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Josef Bacik, Jens Axboe, linux-block, nbd, linux-security-module,
selinux, Ming Lei
In-Reply-To: <20251010080900.1680512-1-omosnace@redhat.com>
On Fri, Oct 10, 2025 at 4:09 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> sock_{send,recv}msg() internally calls security_socket_{send,recv}msg(),
> which does security checks (e.g. SELinux) for socket access against the
> current task. However, _sock_xmit() in drivers/block/nbd.c may be called
> indirectly from a userspace syscall, where the NBD socket access would
> be incorrectly checked against the calling userspace task (which simply
> tries to read/write a file that happens to reside on an NBD device).
>
> To fix this, temporarily override creds to kernel ones before calling
> the sock_*() functions. This allows the security modules to recognize
> this as internal access by the kernel, which will normally be allowed.
>
> A way to trigger the issue is to do the following (on a system with
> SELinux set to enforcing):
>
> ### Create nbd device:
> truncate -s 256M /tmp/testfile
> nbd-server localhost:10809 /tmp/testfile
>
> ### Connect to the nbd server:
> nbd-client localhost
>
> ### Create mdraid array
> mdadm --create -l 1 -n 2 /dev/md/testarray /dev/nbd0 missing
>
> After these steps, assuming the SELinux policy doesn't allow the
> unexpected access pattern, errors will be visible on the kernel console:
>
> [ 142.204243] nbd0: detected capacity change from 0 to 524288
> [ 165.189967] md: async del_gendisk mode will be removed in future, please upgrade to mdadm-4.5+
> [ 165.252299] md/raid1:md127: active with 1 out of 2 mirrors
> [ 165.252725] md127: detected capacity change from 0 to 522240
> [ 165.255434] block nbd0: Send control failed (result -13)
> [ 165.255718] block nbd0: Request send failed, requeueing
> [ 165.256006] block nbd0: Dead connection, failed to find a fallback
> [ 165.256041] block nbd0: Receive control failed (result -32)
> [ 165.256423] block nbd0: shutting down sockets
> [ 165.257196] I/O error, dev nbd0, sector 2048 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2
> [ 165.257736] Buffer I/O error on dev md127, logical block 0, async page read
> [ 165.258263] I/O error, dev nbd0, sector 2048 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2
> [ 165.259376] Buffer I/O error on dev md127, logical block 0, async page read
> [ 165.259920] I/O error, dev nbd0, sector 2048 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2
> [ 165.260628] Buffer I/O error on dev md127, logical block 0, async page read
> [ 165.261661] ldm_validate_partition_table(): Disk read failed.
> [ 165.262108] I/O error, dev nbd0, sector 2048 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2
> [ 165.262769] Buffer I/O error on dev md127, logical block 0, async page read
> [ 165.263697] I/O error, dev nbd0, sector 2048 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2
> [ 165.264412] Buffer I/O error on dev md127, logical block 0, async page read
> [ 165.265412] I/O error, dev nbd0, sector 2048 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2
> [ 165.265872] Buffer I/O error on dev md127, logical block 0, async page read
> [ 165.266378] I/O error, dev nbd0, sector 2048 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2
> [ 165.267168] Buffer I/O error on dev md127, logical block 0, async page read
> [ 165.267564] md127: unable to read partition table
> [ 165.269581] I/O error, dev nbd0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2
> [ 165.269960] Buffer I/O error on dev nbd0, logical block 0, async page read
> [ 165.270316] I/O error, dev nbd0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2
> [ 165.270913] Buffer I/O error on dev nbd0, logical block 0, async page read
> [ 165.271253] I/O error, dev nbd0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2
> [ 165.271809] Buffer I/O error on dev nbd0, logical block 0, async page read
> [ 165.272074] ldm_validate_partition_table(): Disk read failed.
> [ 165.272360] nbd0: unable to read partition table
> [ 165.289004] ldm_validate_partition_table(): Disk read failed.
> [ 165.289614] nbd0: unable to read partition table
>
> The corresponding SELinux denial on Fedora/RHEL will look like this
> (assuming it's not silenced):
> type=AVC msg=audit(1758104872.510:116): avc: denied { write } for pid=1908 comm="mdadm" laddr=::1 lport=32772 faddr=::1 fport=10809 scontext=system_u:system_r:mdadm_t:s0-s0:c0.c1023 tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=tcp_socket permissive=0
>
> The respective backtrace looks like this:
> @security[mdadm, -13,
> handshake_exit+221615650
> handshake_exit+221615650
> handshake_exit+221616465
> security_socket_sendmsg+5
> sock_sendmsg+106
> handshake_exit+221616150
> sock_sendmsg+5
> __sock_xmit+162
> nbd_send_cmd+597
> nbd_handle_cmd+377
> nbd_queue_rq+63
> blk_mq_dispatch_rq_list+653
> __blk_mq_do_dispatch_sched+184
> __blk_mq_sched_dispatch_requests+333
> blk_mq_sched_dispatch_requests+38
> blk_mq_run_hw_queue+239
> blk_mq_dispatch_plug_list+382
> blk_mq_flush_plug_list.part.0+55
> __blk_flush_plug+241
> __submit_bio+353
> submit_bio_noacct_nocheck+364
> submit_bio_wait+84
> __blkdev_direct_IO_simple+232
> blkdev_read_iter+162
> vfs_read+591
> ksys_read+95
> do_syscall_64+92
> entry_SYSCALL_64_after_hwframe+120
> ]: 1
>
> The issue has started to appear since commit 060406c61c7c ("block: add
> plug while submitting IO").
>
> Cc: Ming Lei <ming.lei@redhat.com>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2348878
> Fixes: 060406c61c7c ("block: add plug while submitting IO")
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>
> Changes in v2:
> * Move put_cred() after destroy_workqueue() in nbd_cleanup() to avoid a UAF
> * Add some more details into the commit message
> * Add a Fixes: tag
>
> v1: https://lore.kernel.org/linux-block/20251009134542.1529148-1-omosnace@redhat.com/
>
> drivers/block/nbd.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
Acked-by: Paul Moore <paul@paul-moore.com>
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH 2/2] LSM: Allow reservation of netlabel
From: Casey Schaufler @ 2025-10-10 15:08 UTC (permalink / raw)
To: Stephen Smalley
Cc: paul, linux-security-module, jmorris, serge, keescook,
john.johansen, penguin-kernel, linux-kernel, selinux,
Casey Schaufler
In-Reply-To: <CAEjxPJ48PiZ5ZOZbZjka5YeiBxaWFsCufoGcY_jEztM+wtEUCA@mail.gmail.com>
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:
>> 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>
>> ---
>> diff --git a/security/security.c b/security/security.c
>> index e59e3d403de6..9eca10844b56 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -289,6 +289,12 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
>> else
>> blob_sizes.lbs_secmark = true;
>> }
>> + if (needed->lbs_netlabel) {
>> + if (blob_sizes.lbs_netlabel)
>> + needed->lbs_netlabel = false;
>> + else
>> + blob_sizes.lbs_netlabel = true;
>> +
> Same principle here - if a LSM wants to use netlabel, it may want to
> guarantee that it truly has exclusive access to it no matter what the
> LSM order is.
And if SELinux and Smack both shout "I gotta have it!" who wins?
Does the system fail to boot? Do you assign it to the first requestor,
as this patch does explicitly?
If LSMs can't be equal in the eyes of the infrastructure, If one (e.g. SELinux)
always gets its way regardless of the end user intent, I have a problem with
the whole thing.
^ permalink raw reply
* Re: [PATCH] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Kees Cook @ 2025-10-10 15:12 UTC (permalink / raw)
To: Thorsten Blum
Cc: Mimi Zohar, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn, linux-hardening, linux-integrity,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20251009180316.394708-3-thorsten.blum@linux.dev>
On Thu, Oct 09, 2025 at 08:03:17PM +0200, Thorsten Blum wrote:
> strncpy() is deprecated for NUL-terminated destination buffers; use
> strscpy_pad() instead.
Remember for strncpy->strscpy conversions, the commit message needs
to include:
- how did you determine this was a NUL-terminated destination?
- how did you determine the need for padding or not?
- how do you know that final byte truncation is not a problem?
-Kees
>
> Link: https://github.com/KSPP/linux/issues/90
> Signed-off-by: Thorsten Blum <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
>
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v6 5/5] Smack: add support for lsm_config_self_policy and lsm_config_system_policy
From: Casey Schaufler @ 2025-10-10 15:15 UTC (permalink / raw)
To: Maxime Bélair, linux-security-module
Cc: john.johansen, paul, jmorris, serge, mic, kees,
stephen.smalley.work, takedakn, penguin-kernel, song, rdunlap,
linux-api, apparmor, linux-kernel, Casey Schaufler
In-Reply-To: <20251010132610.12001-6-maxime.belair@canonical.com>
On 10/10/2025 6:25 AM, Maxime Bélair wrote:
> Enable users to manage Smack policies through the new hooks
> lsm_config_self_policy and lsm_config_system_policy.
>
> lsm_config_self_policy allows adding Smack policies for the current cred.
> For now it remains restricted to CAP_MAC_ADMIN.
>
> lsm_config_system_policy allows adding globabl Smack policies. This is
> restricted to CAP_MAC_ADMIN.
>
> Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
I will be reviewing these patches, but will not be able to do so
until early November. I know how frustrating review delays can be,
but it really can't be helped this time around. Thank you for your
patience.
> ---
> security/smack/smack.h | 8 +++++
> security/smack/smack_lsm.c | 73 ++++++++++++++++++++++++++++++++++++++
> security/smack/smackfs.c | 2 +-
> 3 files changed, 82 insertions(+), 1 deletion(-)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index bf6a6ed3946c..3e3d30dfdcf7 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -275,6 +275,14 @@ struct smk_audit_info {
> #endif
> };
>
> +/*
> + * This function is in smackfs.c
> + */
> +ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos,
> + struct list_head *rule_list,
> + struct mutex *rule_lock, int format);
> +
> /*
> * These functions are in smack_access.c
> */
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 99833168604e..bf4bb2242768 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -5027,6 +5027,76 @@ static int smack_uring_cmd(struct io_uring_cmd *ioucmd)
>
> #endif /* CONFIG_IO_URING */
>
> +/**
> + * smack_lsm_config_system_policy - Configure a system smack policy
> + * @op: operation to perform. Currently, only LSM_POLICY_LOAD is supported
> + * @buf: User-supplied buffer in the form "<fmt><policy>"
> + * <fmt> is the 1-byte format of <policy>
> + * <policy> is the policy to load
> + * @size: size of @buf
> + * @flags: reserved for future use; must be zero
> + *
> + * Returns: number of written rules on success, negative value on error
> + */
> +static int smack_lsm_config_system_policy(u32 op, void __user *buf, size_t size,
> + u32 flags)
> +{
> + loff_t pos = 0;
> + u8 fmt;
> +
> + if (op != LSM_POLICY_LOAD || flags)
> + return -EOPNOTSUPP;
> +
> + if (size < 2)
> + return -EINVAL;
> +
> + if (get_user(fmt, (uint8_t *)buf))
> + return -EFAULT;
> +
> + return smk_write_rules_list(NULL, buf + 1, size - 1, &pos, NULL, NULL, fmt);
> +}
> +
> +/**
> + * smack_lsm_config_self_policy - Configure a smack policy for the current cred
> + * @op: operation to perform. Currently, only LSM_POLICY_LOAD is supported
> + * @buf: User-supplied buffer in the form "<fmt><policy>"
> + * <fmt> is the 1-byte format of <policy>
> + * <policy> is the policy to load
> + * @size: size of @buf
> + * @flags: reserved for future use; must be zero
> + *
> + * Returns: number of written rules on success, negative value on error
> + */
> +static int smack_lsm_config_self_policy(u32 op, void __user *buf, size_t size,
> + u32 flags)
> +{
> + loff_t pos = 0;
> + u8 fmt;
> + struct task_smack *tsp;
> +
> + if (op != LSM_POLICY_LOAD || flags)
> + return -EOPNOTSUPP;
> +
> + if (size < 2)
> + return -EINVAL;
> +
> + if (get_user(fmt, (uint8_t *)buf))
> + return -EFAULT;
> + /**
> + * smk_write_rules_list could be used to gain privileges.
> + * This function is thus restricted to CAP_MAC_ADMIN.
> + * TODO: Ensure that the new rule does not give extra privileges
> + * before dropping this CAP_MAC_ADMIN check.
> + */
> + if (!capable(CAP_MAC_ADMIN))
> + return -EPERM;
> +
> +
> + tsp = smack_cred(current_cred());
> + return smk_write_rules_list(NULL, buf + 1, size - 1, &pos, &tsp->smk_rules,
> + &tsp->smk_rules_lock, fmt);
> +}
> +
> struct lsm_blob_sizes smack_blob_sizes __ro_after_init = {
> .lbs_cred = sizeof(struct task_smack),
> .lbs_file = sizeof(struct smack_known *),
> @@ -5203,6 +5273,9 @@ static struct security_hook_list smack_hooks[] __ro_after_init = {
> LSM_HOOK_INIT(uring_sqpoll, smack_uring_sqpoll),
> LSM_HOOK_INIT(uring_cmd, smack_uring_cmd),
> #endif
> + LSM_HOOK_INIT(lsm_config_self_policy, smack_lsm_config_self_policy),
> + LSM_HOOK_INIT(lsm_config_system_policy, smack_lsm_config_system_policy),
> +
> };
>
>
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 90a67e410808..ed1814588d56 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -441,7 +441,7 @@ static ssize_t smk_parse_long_rule(char *data, struct smack_parsed_rule *rule,
> * "subject<whitespace>object<whitespace>
> * acc_enable<whitespace>acc_disable[<whitespace>...]"
> */
> -static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
> +ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
> size_t count, loff_t *ppos,
> struct list_head *rule_list,
> struct mutex *rule_lock, int format)
^ permalink raw reply
* Re: [PATCH 1/2] LSM: Exclusive secmark usage
From: Casey Schaufler @ 2025-10-10 15:02 UTC (permalink / raw)
To: Stephen Smalley
Cc: paul, linux-security-module, jmorris, serge, keescook,
john.johansen, penguin-kernel, linux-kernel, selinux,
Casey Schaufler
In-Reply-To: <CAEjxPJ7M4qySg+ZMujTqMQFSncWNbG21W+kpLzji6c4F+hyprA@mail.gmail.com>
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. The limitation on a secmark being a u32 is a huge problem,
and Paul has battled with the netdev people over it for years.
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: James Bottomley @ 2025-10-10 15:53 UTC (permalink / raw)
To: Alexei Starovoitov, Paul Moore
Cc: Alexei Starovoitov, KP Singh, Linus Torvalds, Blaise Boscaccy,
bpf, LSM List, K. Y. Srinivasan, Daniel Borkmann, Andrii Nakryiko,
wufan, Quentin Monnet
In-Reply-To: <CAADnVQLfyh=qby02AFe+MfJYr2sPExEU0YGCLV9jJk=cLoZoaA@mail.gmail.com>
On Thu, 2025-10-09 at 18:00 -0700, Alexei Starovoitov wrote:
[...]
> James's concern is valid though:
>
> > However, the rub for LSM is that the verification of the program
> > map by the loader happens *after* the security_bpf_prog_load() hook
> > has been called.
>
> I understand the discomfort, but that's what the kernel module
> loading process is doing as well, so you should be concerned with
> both. Since both are doing pretty much the same work.
OK, so let me push on this one point because I don't agree with what
you say here. The way kernel modules and eBPF load is not equivalent.
The kernel module signatures go over a relocateable elf binary which is
subsequently relocated after signature verification in the kernel by
the ELF loader. You can regard the ELF loader as being equivalent to
the eBPF loader in terms of function, absolutely. However for security
purposes the ELF loader is a trusted part of the kernel security
envelope and its integrity is part of the kernel integrity and we have
a this single trusted loader for every module. In security terms
verification of the ELF object signature is sufficient to guarantee
integrity of the module because the integrity of the ELF loader is
already checked.
The eBPF loader, by contrast, because it contains all the relocations,
is different for every eBPF light skeleton. This means it's not a
trusted part of the kernel and has to be integrity checked as well.
Thus for eBPF, the integrity check must be over both the loader and the
program; integrity checking is not complete until the integrity of both
has been verified. If you sign only the loader and embed the hash of
the program into the loader that is a different way of doing things,
but the integrity check is not complete until the loader does the hash
verification which, as has been stated many times before, is *after*
the load LSM hook has run.
There are two potential solutions to this: complete the integrity check
before running the load hook (Blaise's patch) or add a LSM hook to
collect the integrity information from the run of the loader. Neither
of these is present in the scheme you put upstream.
Regards,
James
^ permalink raw reply
* [PATCH v2] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Thorsten Blum @ 2025-10-10 16:13 UTC (permalink / raw)
To: Kees Cook, Mimi Zohar, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn
Cc: linux-hardening, Thorsten Blum, linux-integrity, keyrings,
linux-security-module, linux-kernel
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.
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.
Link: https://github.com/KSPP/linux/issues/90
Signed-off-by: Thorsten Blum <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
* Re: [PATCH v4 31/34] ima,evm: move initcalls to the LSM framework
From: Mimi Zohar @ 2025-10-10 16:53 UTC (permalink / raw)
To: Paul Moore
Cc: selinux, linux-integrity, linux-security-module, John Johansen,
Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <CAHC9VhQCmFJQ1=Eyu1D+Mcg2FVDByrk8QcwV5HaZdB95esiA7Q@mail.gmail.com>
On Tue, 2025-09-30 at 16:11 -0400, Paul Moore wrote:
> On Tue, Sep 16, 2025 at 6:14 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> >
> > This patch converts IMA and EVM to use the LSM frameworks's initcall
> > mechanism. It moved the integrity_fs_init() call to ima_fs_init() and
> > evm_init_secfs(), to work around the fact that there is no "integrity" LSM,
> > and introduced integrity_fs_fini() to remove the integrity directory, if
> > empty. Both integrity_fs_init() and integrity_fs_fini() support the
> > scenario of being called by both the IMA and EVM LSMs.
> >
> > This patch does not touch any of the platform certificate code that
> > lives under the security/integrity/platform_certs directory as the
> > IMA/EVM developers would prefer to address that in a future patchset.
> >
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > [PM: adjust description as discussed over email]
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > ---
> > security/integrity/evm/evm_main.c | 3 +--
> > security/integrity/evm/evm_secfs.c | 11 +++++++++--
> > security/integrity/iint.c | 14 ++++++++++++--
> > security/integrity/ima/ima_fs.c | 11 +++++++++--
> > security/integrity/ima/ima_main.c | 4 ++--
> > security/integrity/integrity.h | 2 ++
> > 6 files changed, 35 insertions(+), 10 deletions(-)
>
> I appreciate you reviewing most (all?) of the other patches in this
> patchset, but any chance you could review the IMA/EVM from Roberto?
> This is the only patch that really needs your review ...
Paul, I'm sorry for the long delay in reviewing and testing this patch set. It
wasn't enough to just review this one patch, but it needed to be reviewed in
context.
The initcall ordering is extremely important for IMA. IMA-measurement needs to
be initialized after the TPM, otherwise IMA goes into TPM-bypass mode. As
expected, the initcall ordering seems to be fine. However this patch set
modifies the initcall debugging.
The kernel boot command line option "initcall_debug" outputs "entering initcall
level:" messages for each of the initcall levels, and "calling ...." and
"initcall ..." messages for the individual initcalls.
For example,
[ 0.896556] entering initcall level: arch
[ 0.896556] calling report_snp_info+0x0/0xd0 @ 1
[ 0.896556] initcall report_snp_info+0x0/0xd0 returned 0 after 0 usecs
With this patch set, the "calling ..." and "initcall ..." messages will not be
emitted for the LSMs. In lieu of these messages, the patch set defines a new
boot command line option "lsm.debug", which outputs "LSM: entering ....
initcall".
For example,
[ 2.225821] calling security_initcall_late+0x0/0xc0 @ 1
[ 2.225825] LSM: running ima late initcall
Regardless as to whether the performance information is actually necessary, the
initcall debugging change should probably be documented. Maybe update
initcall_debug to reference lsm.debug in Documentation/admin-guide/kernel-
parameters.txt.
Mimi
^ permalink raw reply
* Re: [PATCH v2] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Jarkko Sakkinen @ 2025-10-10 17:44 UTC (permalink / raw)
To: Thorsten Blum
Cc: Kees Cook, Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, linux-hardening, linux-integrity, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <20251010161340.458707-2-thorsten.blum@linux.dev>
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.
>
> 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.
It's a functional change (for better!) because it transforms to safer
semantics ;-) And yeah as years pass by commit messages like these
have more value than code changes themselves (as far backtracking
and bisecting is concerned).
So if you don't mind, I'll delete the very last one sentence paragraph,
and with that
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Thank you.
>
> Link: https://github.com/KSPP/linux/issues/90
> Signed-off-by: Thorsten Blum <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
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v6 1/5] Wire up lsm_config_self_policy and lsm_config_system_policy syscalls
From: Song Liu @ 2025-10-10 18:06 UTC (permalink / raw)
To: Maxime Bélair
Cc: linux-security-module, john.johansen, paul, jmorris, serge, mic,
kees, stephen.smalley.work, casey, takedakn, penguin-kernel, song,
rdunlap, linux-api, apparmor, linux-kernel
In-Reply-To: <20251010132610.12001-2-maxime.belair@canonical.com>
On Fri, Oct 10, 2025 at 6:27 AM Maxime Bélair
<maxime.belair@canonical.com> wrote:
[...]
> --- a/security/lsm_syscalls.c
> +++ b/security/lsm_syscalls.c
> @@ -118,3 +118,15 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, u32 __user *, size,
>
> return lsm_active_cnt;
> }
> +
> +SYSCALL_DEFINE6(lsm_config_self_policy, u32, lsm_id, u32, op, void __user *,
> + buf, u32 __user, size, u32, common_flags, u32, flags)
> +{
> + return 0;
> +}
> +
> +SYSCALL_DEFINE6(lsm_config_system_policy, u32, lsm_id, u32, op, void __user *,
> + buf, u32 __user, size, u32, common_flags, u32, flags)
> +{
> + return 0;
> +}
These two APIs look the same. Why not just keep one API and use
one bit in the flag to differentiate "self" vs. "system"?
Thanks,
Song
^ permalink raw reply
* Re: [PATCH v4 31/34] ima,evm: move initcalls to the LSM framework
From: Paul Moore @ 2025-10-10 19:21 UTC (permalink / raw)
To: Mimi Zohar
Cc: selinux, linux-integrity, linux-security-module, John Johansen,
Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <282070d5c0dd68140ae221833ea8c5ba4baada4f.camel@linux.ibm.com>
On Fri, Oct 10, 2025 at 12:54 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Tue, 2025-09-30 at 16:11 -0400, Paul Moore wrote:
> > On Tue, Sep 16, 2025 at 6:14 PM Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > >
> > > This patch converts IMA and EVM to use the LSM frameworks's initcall
> > > mechanism. It moved the integrity_fs_init() call to ima_fs_init() and
> > > evm_init_secfs(), to work around the fact that there is no "integrity" LSM,
> > > and introduced integrity_fs_fini() to remove the integrity directory, if
> > > empty. Both integrity_fs_init() and integrity_fs_fini() support the
> > > scenario of being called by both the IMA and EVM LSMs.
> > >
> > > This patch does not touch any of the platform certificate code that
> > > lives under the security/integrity/platform_certs directory as the
> > > IMA/EVM developers would prefer to address that in a future patchset.
> > >
> > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > > [PM: adjust description as discussed over email]
> > > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > > ---
> > > security/integrity/evm/evm_main.c | 3 +--
> > > security/integrity/evm/evm_secfs.c | 11 +++++++++--
> > > security/integrity/iint.c | 14 ++++++++++++--
> > > security/integrity/ima/ima_fs.c | 11 +++++++++--
> > > security/integrity/ima/ima_main.c | 4 ++--
> > > security/integrity/integrity.h | 2 ++
> > > 6 files changed, 35 insertions(+), 10 deletions(-)
> >
> > I appreciate you reviewing most (all?) of the other patches in this
> > patchset, but any chance you could review the IMA/EVM from Roberto?
> > This is the only patch that really needs your review ...
>
> Paul, I'm sorry for the long delay in reviewing and testing this patch set. It
> wasn't enough to just review this one patch, but it needed to be reviewed in
> context.
>
> The initcall ordering is extremely important for IMA. IMA-measurement needs to
> be initialized after the TPM, otherwise IMA goes into TPM-bypass mode. As
> expected, the initcall ordering seems to be fine. However this patch set
> modifies the initcall debugging.
>
> The kernel boot command line option "initcall_debug" outputs "entering initcall
> level:" messages for each of the initcall levels, and "calling ...." and
> "initcall ..." messages for the individual initcalls.
>
> For example,
> [ 0.896556] entering initcall level: arch
> [ 0.896556] calling report_snp_info+0x0/0xd0 @ 1
> [ 0.896556] initcall report_snp_info+0x0/0xd0 returned 0 after 0 usecs
>
> With this patch set, the "calling ..." and "initcall ..." messages will not be
> emitted for the LSMs. In lieu of these messages, the patch set defines a new
> boot command line option "lsm.debug" ...
This is not a new kernel command line option, oddly enough Kees wrote
the patch exactly seven years ago today and we first saw it released
in Linux v4.20.
> which outputs "LSM: entering ....
> initcall".
>
> For example,
> [ 2.225821] calling security_initcall_late+0x0/0xc0 @ 1
> [ 2.225825] LSM: running ima late initcall
>
> Regardless as to whether the performance information is actually necessary, the
> initcall debugging change should probably be documented. Maybe update
> initcall_debug to reference lsm.debug in Documentation/admin-guide/kernel-
> parameters.txt.
Thank you for the input, but I would just prefer to leave it as-is.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: Paul Moore @ 2025-10-10 19:39 UTC (permalink / raw)
To: James Bottomley
Cc: Alexei Starovoitov, Alexei Starovoitov, KP Singh, Linus Torvalds,
Blaise Boscaccy, bpf, LSM List, K. Y. Srinivasan, Daniel Borkmann,
Andrii Nakryiko, wufan, Quentin Monnet
In-Reply-To: <88703f00d5b7a779728451008626efa45e42db3d.camel@HansenPartnership.com>
On Fri, Oct 10, 2025 at 11:53 AM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Thu, 2025-10-09 at 18:00 -0700, Alexei Starovoitov wrote:
> [...]
> > James's concern is valid though:
> >
> > > However, the rub for LSM is that the verification of the program
> > > map by the loader happens *after* the security_bpf_prog_load() hook
> > > has been called.
> >
> > I understand the discomfort, but that's what the kernel module
> > loading process is doing as well, so you should be concerned with
> > both. Since both are doing pretty much the same work.
>
> OK, so let me push on this one point because I don't agree with what
> you say here. The way kernel modules and eBPF load is not equivalent.
> The kernel module signatures go over a relocateable elf binary which is
> subsequently relocated after signature verification in the kernel by
> the ELF loader. You can regard the ELF loader as being equivalent to
> the eBPF loader in terms of function, absolutely. However for security
> purposes the ELF loader is a trusted part of the kernel security
> envelope and its integrity is part of the kernel integrity and we have
> a this single trusted loader for every module. In security terms
> verification of the ELF object signature is sufficient to guarantee
> integrity of the module because the integrity of the ELF loader is
> already checked.
>
> The eBPF loader, by contrast, because it contains all the relocations,
> is different for every eBPF light skeleton. This means it's not a
> trusted part of the kernel and has to be integrity checked as well.
> Thus for eBPF, the integrity check must be over both the loader and the
> program; integrity checking is not complete until the integrity of both
> has been verified. If you sign only the loader and embed the hash of
> the program into the loader that is a different way of doing things,
> but the integrity check is not complete until the loader does the hash
> verification which, as has been stated many times before, is *after*
> the load LSM hook has run.
>
> There are two potential solutions to this: complete the integrity check
> before running the load hook (Blaise's patch) or add a LSM hook to
> collect the integrity information from the run of the loader. Neither
> of these is present in the scheme you put upstream.
As a bit of background for those who weren't following the related
threads earlier this year, the idea of an additional hook was
discussed this spring and it was rejected by Alexei.
https://lore.kernel.org/linux-security-module/CAADnVQ+wE5cGhy6tgmWgUwkNutueEsrhh6UR8N2fzrZjt-vb4g@mail.gmail.com/
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH 2/2] LSM: Allow reservation of netlabel
From: Stephen Smalley @ 2025-10-10 19:53 UTC (permalink / raw)
To: Casey Schaufler
Cc: paul, linux-security-module, jmorris, serge, keescook,
john.johansen, penguin-kernel, linux-kernel, selinux
In-Reply-To: <ec89959d-c3a0-403d-bfb0-7405639eb0cf@schaufler-ca.com>
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:
> >> 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>
> >> ---
> >> diff --git a/security/security.c b/security/security.c
> >> index e59e3d403de6..9eca10844b56 100644
> >> --- a/security/security.c
> >> +++ b/security/security.c
> >> @@ -289,6 +289,12 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
> >> else
> >> blob_sizes.lbs_secmark = true;
> >> }
> >> + if (needed->lbs_netlabel) {
> >> + if (blob_sizes.lbs_netlabel)
> >> + needed->lbs_netlabel = false;
> >> + else
> >> + blob_sizes.lbs_netlabel = true;
> >> +
> > Same principle here - if a LSM wants to use netlabel, it may want to
> > guarantee that it truly has exclusive access to it no matter what the
> > LSM order is.
>
> And if SELinux and Smack both shout "I gotta have it!" who wins?
> Does the system fail to boot? Do you assign it to the first requestor,
> as this patch does explicitly?
>
> If LSMs can't be equal in the eyes of the infrastructure, If one (e.g. SELinux)
> always gets its way regardless of the end user intent, I have a problem with
> the whole thing.
End user intent is unlikely to be expressed as a silent side effect of
LSM order.
If a security module supports its use without the use of secmark
and/or netlabel and the end user wants to assign one or both to
another security module, that's fine.
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.
^ permalink raw reply
* Re: [PATCH v6 1/5] Wire up lsm_config_self_policy and lsm_config_system_policy syscalls
From: Casey Schaufler @ 2025-10-10 21:13 UTC (permalink / raw)
To: Song Liu, Maxime Bélair
Cc: linux-security-module, john.johansen, paul, jmorris, serge, mic,
kees, stephen.smalley.work, takedakn, penguin-kernel, rdunlap,
linux-api, apparmor, linux-kernel, Casey Schaufler
In-Reply-To: <CAHzjS_uBq8xGCSmHC_kBWi0j8DCdwsy4XtfkH2iH6NygCcChNw@mail.gmail.com>
On 10/10/2025 11:06 AM, Song Liu wrote:
> On Fri, Oct 10, 2025 at 6:27 AM Maxime Bélair
> <maxime.belair@canonical.com> wrote:
> [...]
>> --- a/security/lsm_syscalls.c
>> +++ b/security/lsm_syscalls.c
>> @@ -118,3 +118,15 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, u32 __user *, size,
>>
>> return lsm_active_cnt;
>> }
>> +
>> +SYSCALL_DEFINE6(lsm_config_self_policy, u32, lsm_id, u32, op, void __user *,
>> + buf, u32 __user, size, u32, common_flags, u32, flags)
>> +{
>> + return 0;
>> +}
>> +
>> +SYSCALL_DEFINE6(lsm_config_system_policy, u32, lsm_id, u32, op, void __user *,
>> + buf, u32 __user, size, u32, common_flags, u32, flags)
>> +{
>> + return 0;
>> +}
> These two APIs look the same. Why not just keep one API and use
> one bit in the flag to differentiate "self" vs. "system"?
I think that's a valid point.
>
> Thanks,
> Song
>
^ permalink raw reply
* Re: [PATCH 2/2] LSM: Allow reservation of netlabel
From: Casey Schaufler @ 2025-10-10 21:10 UTC (permalink / raw)
To: Stephen Smalley
Cc: paul, linux-security-module, jmorris, serge, keescook,
john.johansen, penguin-kernel, linux-kernel, selinux,
Casey Schaufler
In-Reply-To: <CAEjxPJ5N+vGS4rhBJmCfoW+rUnjPm7TVAC9reRmu6YCaJWTO+Q@mail.gmail.com>
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:
>>>> 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>
>>>> ---
>>>> diff --git a/security/security.c b/security/security.c
>>>> index e59e3d403de6..9eca10844b56 100644
>>>> --- a/security/security.c
>>>> +++ b/security/security.c
>>>> @@ -289,6 +289,12 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
>>>> else
>>>> blob_sizes.lbs_secmark = true;
>>>> }
>>>> + if (needed->lbs_netlabel) {
>>>> + if (blob_sizes.lbs_netlabel)
>>>> + needed->lbs_netlabel = false;
>>>> + else
>>>> + blob_sizes.lbs_netlabel = true;
>>>> +
>>> Same principle here - if a LSM wants to use netlabel, it may want to
>>> guarantee that it truly has exclusive access to it no matter what the
>>> LSM order is.
>> And if SELinux and Smack both shout "I gotta have it!" who wins?
>> Does the system fail to boot? Do you assign it to the first requestor,
>> as this patch does explicitly?
>>
>> If LSMs can't be equal in the eyes of the infrastructure, If one (e.g. SELinux)
>> always gets its way regardless of the end user intent, I have a problem with
>> the whole thing.
> End user intent is unlikely to be expressed as a silent side effect of
> LSM order.
But that's what we have now with the "first exclusive LSM" rule.
And the patch doesn't have a "silent" side effect. An LSM is informed
at initialization whether it can use secmarks. An LSM could even
decide to use secmarks if it has been told not to. That would be wrong,
and probably not upstream acceptable, but in security the wild and wacky
happens all too often.
> If a security module supports its use without the use of secmark
> and/or netlabel and the end user wants to assign one or both to
> another security module, that's fine.
That is what this patch implements.
> 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.
I'm open to suggestions. Would boot options lsm.secmark and lsm.netlabel
be sufficient to address your concern?
^ permalink raw reply
* Re: [PATCH v2] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Kees Cook @ 2025-10-10 22:48 UTC (permalink / raw)
To: Thorsten Blum
Cc: Mimi Zohar, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn, linux-hardening, linux-integrity,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20251010161340.458707-2-thorsten.blum@linux.dev>
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.
>
> 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.
>
> 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
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: Alexei Starovoitov @ 2025-10-10 23:06 UTC (permalink / raw)
To: James Bottomley
Cc: Paul Moore, Alexei Starovoitov, KP Singh, Linus Torvalds,
Blaise Boscaccy, bpf, LSM List, K. Y. Srinivasan, Daniel Borkmann,
Andrii Nakryiko, wufan, Quentin Monnet
In-Reply-To: <88703f00d5b7a779728451008626efa45e42db3d.camel@HansenPartnership.com>
On Fri, Oct 10, 2025 at 8:53 AM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> On Thu, 2025-10-09 at 18:00 -0700, Alexei Starovoitov wrote:
> [...]
> > James's concern is valid though:
> >
> > > However, the rub for LSM is that the verification of the program
> > > map by the loader happens *after* the security_bpf_prog_load() hook
> > > has been called.
> >
> > I understand the discomfort, but that's what the kernel module
> > loading process is doing as well, so you should be concerned with
> > both. Since both are doing pretty much the same work.
>
> OK, so let me push on this one point because I don't agree with what
> you say here. The way kernel modules and eBPF load is not equivalent.
> The kernel module signatures go over a relocateable elf binary which is
> subsequently relocated after signature verification in the kernel by
> the ELF loader. You can regard the ELF loader as being equivalent to
> the eBPF loader in terms of function, absolutely. However for security
> purposes the ELF loader is a trusted part of the kernel security
> envelope and its integrity is part of the kernel integrity and we have
> a this single trusted loader for every module. In security terms
> verification of the ELF object signature is sufficient to guarantee
> integrity of the module because the integrity of the ELF loader is
> already checked.
"integrity of ELF loader" is _not_ checked. It's part of the kernel
and you trust that the kernel is valid, because you trust the
build tools that compiled that kernel.
The kmod signature only covers the contents of the kmod.
Now, kmods are typically targeted one specific kernel version
compiled with a specific config, but some folks do load the same
kmod on different kernels. So by checking integrity of kmod only
you're skipping on the loader. If symbols are not versioned
and crc checked bad things can happen (obviously no one should
be doing that), but signature doesn't protect against that.
Compare that with the bpf signature. The whole package is signed.
The loader and what it is loading with one signature.
I argue that this is a more secure approach than kmod signatures.
Think of it as a self-extracting zip archive.
The whole .zip is covered by one signature. Inside it has the code
to self extract plus all the files inside.
The extracting code is a loader prog. Which is a normal bpf
prog that is subject to the same verification rules.
The files are other bpf progs that are also subject to the verification.
> The eBPF loader, by contrast, because it contains all the relocations,
> is different for every eBPF light skeleton. This means it's not a
> trusted part of the kernel and has to be integrity checked as well.
...and the existing mechanism already does that.
> Thus for eBPF, the integrity check must be over both the loader and the
> program; integrity checking is not complete until the integrity of both
> has been verified.
The signature covers all components: loader, the map that assists
the loading and all progs and maps that are encoded inside that
loader/map tuple.
> If you sign only the loader and embed the hash of
> the program into the loader that is a different way of doing things,
That's simply not true.
Please read the current code more carefully. There is cover letter
that describes what's happening. There are no hashes of programs.
> There are two potential solutions to this: complete the integrity check
> before running the load hook (Blaise's patch)
That's not what it's doing! Read his patch. It's adding pointless
signature to the loader/map tuple. It does nothing to progs, maps,
relocations that will be created at the end when loader completes.
You need to realize that single loader plus single map is
an implementation choice of tools/lib/bpf/gen_loader.c.
It can do the same job with a single prog and no additional map.
Hence any kinda hard coded extra map signature makes no sense.
We're not going to burden the kernel with one specific implementation
detail of gen_loader.
Tomorrow we might change the gen_loader to use a triple:
prog+map+btf or any other form.
The existing approach allows all that extensibility and freedom
to change the gen_loader.
> or add a LSM hook to
> collect the integrity information from the run of the loader. Neither
> of these is present in the scheme you put upstream.
Neither is in cards as was explained countless times.
^ permalink raw reply
* Re: [PATCH v6 1/5] Wire up lsm_config_self_policy and lsm_config_system_policy syscalls
From: kernel test robot @ 2025-10-11 12:07 UTC (permalink / raw)
To: Maxime Bélair, linux-security-module
Cc: oe-kbuild-all, john.johansen, paul, jmorris, serge, mic, kees,
stephen.smalley.work, casey, takedakn, penguin-kernel, song,
rdunlap, linux-api, apparmor, linux-kernel, Maxime Bélair
In-Reply-To: <20251010132610.12001-2-maxime.belair@canonical.com>
Hi Maxime,
kernel test robot noticed the following build errors:
[auto build test ERROR on 9c32cda43eb78f78c73aee4aa344b777714e259b]
url: https://github.com/intel-lab-lkp/linux/commits/Maxime-B-lair/Wire-up-lsm_config_self_policy-and-lsm_config_system_policy-syscalls/20251010-213606
base: 9c32cda43eb78f78c73aee4aa344b777714e259b
patch link: https://lore.kernel.org/r/20251010132610.12001-2-maxime.belair%40canonical.com
patch subject: [PATCH v6 1/5] Wire up lsm_config_self_policy and lsm_config_system_policy syscalls
config: sh-randconfig-001-20251011 (https://download.01.org/0day-ci/archive/20251011/202510111947.0ObJ6YUH-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251011/202510111947.0ObJ6YUH-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510111947.0ObJ6YUH-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from kernel/umh.c:9:0:
>> include/linux/syscalls.h:994:45: error: expected ';', ',' or ')' before 'u32'
u32 __user size, u32 common_flags u32 flags);
^~~
--
In file included from kernel/fork.c:56:0:
>> include/linux/syscalls.h:994:45: error: expected ';', ',' or ')' before 'u32'
u32 __user size, u32 common_flags u32 flags);
^~~
kernel/fork.c: In function '__do_sys_clone3':
kernel/fork.c:3135:2: warning: #warning clone3() entry point is missing, please fix [-Wcpp]
#warning clone3() entry point is missing, please fix
^~~~~~~
vim +994 include/linux/syscalls.h
817
818 /* CONFIG_MMU only */
819 asmlinkage long sys_swapon(const char __user *specialfile, int swap_flags);
820 asmlinkage long sys_swapoff(const char __user *specialfile);
821 asmlinkage long sys_mprotect(unsigned long start, size_t len,
822 unsigned long prot);
823 asmlinkage long sys_msync(unsigned long start, size_t len, int flags);
824 asmlinkage long sys_mlock(unsigned long start, size_t len);
825 asmlinkage long sys_munlock(unsigned long start, size_t len);
826 asmlinkage long sys_mlockall(int flags);
827 asmlinkage long sys_munlockall(void);
828 asmlinkage long sys_mincore(unsigned long start, size_t len,
829 unsigned char __user * vec);
830 asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
831 asmlinkage long sys_process_madvise(int pidfd, const struct iovec __user *vec,
832 size_t vlen, int behavior, unsigned int flags);
833 asmlinkage long sys_process_mrelease(int pidfd, unsigned int flags);
834 asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
835 unsigned long prot, unsigned long pgoff,
836 unsigned long flags);
837 asmlinkage long sys_mseal(unsigned long start, size_t len, unsigned long flags);
838 asmlinkage long sys_mbind(unsigned long start, unsigned long len,
839 unsigned long mode,
840 const unsigned long __user *nmask,
841 unsigned long maxnode,
842 unsigned flags);
843 asmlinkage long sys_get_mempolicy(int __user *policy,
844 unsigned long __user *nmask,
845 unsigned long maxnode,
846 unsigned long addr, unsigned long flags);
847 asmlinkage long sys_set_mempolicy(int mode, const unsigned long __user *nmask,
848 unsigned long maxnode);
849 asmlinkage long sys_migrate_pages(pid_t pid, unsigned long maxnode,
850 const unsigned long __user *from,
851 const unsigned long __user *to);
852 asmlinkage long sys_move_pages(pid_t pid, unsigned long nr_pages,
853 const void __user * __user *pages,
854 const int __user *nodes,
855 int __user *status,
856 int flags);
857 asmlinkage long sys_rt_tgsigqueueinfo(pid_t tgid, pid_t pid, int sig,
858 siginfo_t __user *uinfo);
859 asmlinkage long sys_perf_event_open(
860 struct perf_event_attr __user *attr_uptr,
861 pid_t pid, int cpu, int group_fd, unsigned long flags);
862 asmlinkage long sys_accept4(int, struct sockaddr __user *, int __user *, int);
863 asmlinkage long sys_recvmmsg(int fd, struct mmsghdr __user *msg,
864 unsigned int vlen, unsigned flags,
865 struct __kernel_timespec __user *timeout);
866 asmlinkage long sys_recvmmsg_time32(int fd, struct mmsghdr __user *msg,
867 unsigned int vlen, unsigned flags,
868 struct old_timespec32 __user *timeout);
869 asmlinkage long sys_wait4(pid_t pid, int __user *stat_addr,
870 int options, struct rusage __user *ru);
871 asmlinkage long sys_prlimit64(pid_t pid, unsigned int resource,
872 const struct rlimit64 __user *new_rlim,
873 struct rlimit64 __user *old_rlim);
874 asmlinkage long sys_fanotify_init(unsigned int flags, unsigned int event_f_flags);
875 #if defined(CONFIG_ARCH_SPLIT_ARG64)
876 asmlinkage long sys_fanotify_mark(int fanotify_fd, unsigned int flags,
877 unsigned int mask_1, unsigned int mask_2,
878 int dfd, const char __user * pathname);
879 #else
880 asmlinkage long sys_fanotify_mark(int fanotify_fd, unsigned int flags,
881 u64 mask, int fd,
882 const char __user *pathname);
883 #endif
884 asmlinkage long sys_name_to_handle_at(int dfd, const char __user *name,
885 struct file_handle __user *handle,
886 void __user *mnt_id, int flag);
887 asmlinkage long sys_open_by_handle_at(int mountdirfd,
888 struct file_handle __user *handle,
889 int flags);
890 asmlinkage long sys_clock_adjtime(clockid_t which_clock,
891 struct __kernel_timex __user *tx);
892 asmlinkage long sys_clock_adjtime32(clockid_t which_clock,
893 struct old_timex32 __user *tx);
894 asmlinkage long sys_syncfs(int fd);
895 asmlinkage long sys_setns(int fd, int nstype);
896 asmlinkage long sys_pidfd_open(pid_t pid, unsigned int flags);
897 asmlinkage long sys_sendmmsg(int fd, struct mmsghdr __user *msg,
898 unsigned int vlen, unsigned flags);
899 asmlinkage long sys_process_vm_readv(pid_t pid,
900 const struct iovec __user *lvec,
901 unsigned long liovcnt,
902 const struct iovec __user *rvec,
903 unsigned long riovcnt,
904 unsigned long flags);
905 asmlinkage long sys_process_vm_writev(pid_t pid,
906 const struct iovec __user *lvec,
907 unsigned long liovcnt,
908 const struct iovec __user *rvec,
909 unsigned long riovcnt,
910 unsigned long flags);
911 asmlinkage long sys_kcmp(pid_t pid1, pid_t pid2, int type,
912 unsigned long idx1, unsigned long idx2);
913 asmlinkage long sys_finit_module(int fd, const char __user *uargs, int flags);
914 asmlinkage long sys_sched_setattr(pid_t pid,
915 struct sched_attr __user *attr,
916 unsigned int flags);
917 asmlinkage long sys_sched_getattr(pid_t pid,
918 struct sched_attr __user *attr,
919 unsigned int size,
920 unsigned int flags);
921 asmlinkage long sys_renameat2(int olddfd, const char __user *oldname,
922 int newdfd, const char __user *newname,
923 unsigned int flags);
924 asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
925 void __user *uargs);
926 asmlinkage long sys_getrandom(char __user *buf, size_t count,
927 unsigned int flags);
928 asmlinkage long sys_memfd_create(const char __user *uname_ptr, unsigned int flags);
929 asmlinkage long sys_bpf(int cmd, union bpf_attr __user *attr, unsigned int size);
930 asmlinkage long sys_execveat(int dfd, const char __user *filename,
931 const char __user *const __user *argv,
932 const char __user *const __user *envp, int flags);
933 asmlinkage long sys_userfaultfd(int flags);
934 asmlinkage long sys_membarrier(int cmd, unsigned int flags, int cpu_id);
935 asmlinkage long sys_mlock2(unsigned long start, size_t len, int flags);
936 asmlinkage long sys_copy_file_range(int fd_in, loff_t __user *off_in,
937 int fd_out, loff_t __user *off_out,
938 size_t len, unsigned int flags);
939 asmlinkage long sys_preadv2(unsigned long fd, const struct iovec __user *vec,
940 unsigned long vlen, unsigned long pos_l, unsigned long pos_h,
941 rwf_t flags);
942 asmlinkage long sys_pwritev2(unsigned long fd, const struct iovec __user *vec,
943 unsigned long vlen, unsigned long pos_l, unsigned long pos_h,
944 rwf_t flags);
945 asmlinkage long sys_pkey_mprotect(unsigned long start, size_t len,
946 unsigned long prot, int pkey);
947 asmlinkage long sys_pkey_alloc(unsigned long flags, unsigned long init_val);
948 asmlinkage long sys_pkey_free(int pkey);
949 asmlinkage long sys_statx(int dfd, const char __user *path, unsigned flags,
950 unsigned mask, struct statx __user *buffer);
951 asmlinkage long sys_rseq(struct rseq __user *rseq, uint32_t rseq_len,
952 int flags, uint32_t sig);
953 asmlinkage long sys_open_tree(int dfd, const char __user *path, unsigned flags);
954 asmlinkage long sys_open_tree_attr(int dfd, const char __user *path,
955 unsigned flags,
956 struct mount_attr __user *uattr,
957 size_t usize);
958 asmlinkage long sys_move_mount(int from_dfd, const char __user *from_path,
959 int to_dfd, const char __user *to_path,
960 unsigned int ms_flags);
961 asmlinkage long sys_mount_setattr(int dfd, const char __user *path,
962 unsigned int flags,
963 struct mount_attr __user *uattr, size_t usize);
964 asmlinkage long sys_fsopen(const char __user *fs_name, unsigned int flags);
965 asmlinkage long sys_fsconfig(int fs_fd, unsigned int cmd, const char __user *key,
966 const void __user *value, int aux);
967 asmlinkage long sys_fsmount(int fs_fd, unsigned int flags, unsigned int ms_flags);
968 asmlinkage long sys_fspick(int dfd, const char __user *path, unsigned int flags);
969 asmlinkage long sys_pidfd_send_signal(int pidfd, int sig,
970 siginfo_t __user *info,
971 unsigned int flags);
972 asmlinkage long sys_pidfd_getfd(int pidfd, int fd, unsigned int flags);
973 asmlinkage long sys_landlock_create_ruleset(const struct landlock_ruleset_attr __user *attr,
974 size_t size, __u32 flags);
975 asmlinkage long sys_landlock_add_rule(int ruleset_fd, enum landlock_rule_type rule_type,
976 const void __user *rule_attr, __u32 flags);
977 asmlinkage long sys_landlock_restrict_self(int ruleset_fd, __u32 flags);
978 asmlinkage long sys_memfd_secret(unsigned int flags);
979 asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
980 unsigned long home_node,
981 unsigned long flags);
982 asmlinkage long sys_cachestat(unsigned int fd,
983 struct cachestat_range __user *cstat_range,
984 struct cachestat __user *cstat, unsigned int flags);
985 asmlinkage long sys_map_shadow_stack(unsigned long addr, unsigned long size, unsigned int flags);
986 asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx __user *ctx,
987 u32 __user *size, u32 flags);
988 asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx __user *ctx,
989 u32 size, u32 flags);
990 asmlinkage long sys_lsm_list_modules(u64 __user *ids, u32 __user *size, u32 flags);
991 asmlinkage long sys_lsm_config_self_policy(u32 lsm_id, u32 op, void __user *buf,
992 u32 __user size, u32 common_flags, u32 flags);
993 asmlinkage long sys_lsm_config_system_policy(u32 lsm_id, u32 op, void __user *buf,
> 994 u32 __user size, u32 common_flags u32 flags);
995
996
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH] keys: Remove redundant less-than-zero checks
From: Thorsten Blum @ 2025-10-11 14:48 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, James Bottomley
Cc: Thorsten Blum, keyrings, linux-security-module, linux-kernel,
linux-integrity
The local variables 'size_t datalen' are unsigned and cannot be less
than zero. Remove the redundant conditions.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/keys/big_key.c | 2 +-
security/keys/encrypted-keys/encrypted.c | 4 ++--
security/keys/trusted-keys/trusted_core.c | 4 ++--
security/keys/user_defined.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index c3367622c683..d46862ab90d6 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -66,7 +66,7 @@ int big_key_preparse(struct key_preparsed_payload *prep)
BUILD_BUG_ON(sizeof(*payload) != sizeof(prep->payload.data));
- if (datalen <= 0 || datalen > 1024 * 1024 || !prep->data)
+ if (datalen == 0 || datalen > 1024 * 1024 || !prep->data)
return -EINVAL;
/* Set an arbitrary quota */
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index d70f71d37f5f..57f88ae000ba 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -786,7 +786,7 @@ static int encrypted_instantiate(struct key *key,
size_t datalen = prep->datalen;
int ret;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
datablob = kmalloc(datalen + 1, GFP_KERNEL);
@@ -847,7 +847,7 @@ static int encrypted_update(struct key *key, struct key_preparsed_payload *prep)
if (key_is_negative(key))
return -ENOKEY;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
buf = kmalloc(datalen + 1, GFP_KERNEL);
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index e2d9644efde1..b1680ee53f86 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -157,7 +157,7 @@ static int trusted_instantiate(struct key *key,
int key_cmd;
size_t key_len;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
orig_datablob = datablob = kmalloc(datalen + 1, GFP_KERNEL);
@@ -240,7 +240,7 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
p = key->payload.data[0];
if (!p->migratable)
return -EPERM;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
orig_datablob = datablob = kmalloc(datalen + 1, GFP_KERNEL);
diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index 749e2a4dcb13..686d56e4cc85 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -61,7 +61,7 @@ int user_preparse(struct key_preparsed_payload *prep)
struct user_key_payload *upayload;
size_t datalen = prep->datalen;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
+ if (datalen == 0 || datalen > 32767 || !prep->data)
return -EINVAL;
upayload = kmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: James Bottomley @ 2025-10-11 14:52 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Paul Moore, Alexei Starovoitov, KP Singh, Linus Torvalds,
Blaise Boscaccy, bpf, LSM List, K. Y. Srinivasan, Daniel Borkmann,
Andrii Nakryiko, wufan, Quentin Monnet
In-Reply-To: <CAADnVQKdsF5_9Vb_J+z27y5Of3P6J3gPNZ=hXKFi=APm6AHX3w@mail.gmail.com>
On Fri, 2025-10-10 at 16:06 -0700, Alexei Starovoitov wrote:
> On Fri, Oct 10, 2025 at 8:53 AM James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> >
> > On Thu, 2025-10-09 at 18:00 -0700, Alexei Starovoitov wrote:
> > [...]
> > > James's concern is valid though:
> > >
> > > > However, the rub for LSM is that the verification of the
> > > > program map by the loader happens *after* the
> > > > security_bpf_prog_load() hook has been called.
> > >
> > > I understand the discomfort, but that's what the kernel module
> > > loading process is doing as well, so you should be concerned with
> > > both. Since both are doing pretty much the same work.
> >
> > OK, so let me push on this one point because I don't agree with
> > what you say here. The way kernel modules and eBPF load is not
> > equivalent. The kernel module signatures go over a relocateable elf
> > binary which is subsequently relocated after signature verification
> > in the kernel by the ELF loader. You can regard the ELF loader as
> > being equivalent to the eBPF loader in terms of function,
> > absolutely. However for security purposes the ELF loader is a
> > trusted part of the kernel security envelope and its integrity is
> > part of the kernel integrity and we have a this single trusted
> > loader for every module. In security terms verification of the ELF
> > object signature is sufficient to guarantee integrity of the module
> > because the integrity of the ELF loader is already checked.
>
> "integrity of ELF loader" is _not_ checked. It's part of the kernel
> and you trust that the kernel is valid, because you trust the
> build tools that compiled that kernel.
No, I trust my kernel is valid because the bootloader checks its
signature on every boot (in case some tampering occurs after build).
There are a variety of integrity mechanisms for the kernel itself, but
they all essentially run a check on boot meaning the ELF loader *is*
integrity checked.
[...]
> > Thus for eBPF, the integrity check must be over both the loader and
> > the program; integrity checking is not complete until the integrity
> > of both has been verified.
>
> The signature covers all components: loader, the map that assists
> the loading and all progs and maps that are encoded inside that
> loader/map tuple.
The pkcs7 signature that is verified hashes over the loader only.
Other mechanisms cover the rest.
> > If you sign only the loader and embed the hash of the program into
> > the loader that is a different way of doing things,
>
> That's simply not true. Please read the current code more carefully.
> There is cover letter that describes what's happening. There are no
> hashes of programs.
Is this a semantic quibble over the meaning of the word "sign"? Fine,
I'll be more precise. Quoting from:
https://lore.kernel.org/bpf/20250921160120.9711-2-kpsingh@kernel.org/
> + bpf_dynptr_init(&insns_ptr, prog->insnsi,
> BPF_DYNPTR_TYPE_LOCAL, 0,
> + prog->len * sizeof(struct bpf_insn));
> +
> + err = bpf_verify_pkcs7_signature((struct bpf_dynptr
> *)&insns_ptr,
> + (struct bpf_dynptr
> *)&sig_ptr, key);
for a light skeleton prog->insnsi points to the loader and the second
statement is verifying the pkcs7 signature of the loader only.
> > There are two potential solutions to this: complete the integrity
> > check before running the load hook (Blaise's patch)
>
> That's not what it's doing! Read his patch. It's adding pointless
> signature to the loader/map tuple. It does nothing to progs, maps,
> relocations that will be created at the end when loader completes.
It doesn't need to, once we check both the loader and the map, the
integrity is verified and the loader can be trusted to run and relocate
the map into the bpf program (since you like the module parallel: we
have the integrity verification of the module object and the ELF
loader, so we can just let the relocation run: the output has integrity
because all inputs did).
> You need to realize that single loader plus single map is
> an implementation choice of tools/lib/bpf/gen_loader.c.
> It can do the same job with a single prog and no additional map.
Yes, and if the light skeleton scheme embedded the relocation and the
program itself into prog->insnsi then we wouldn't need the additional
map verification before the load hook because the pkcs7 signature check
would fully verify the integrity.
> Hence any kinda hard coded extra map signature makes no sense.
> We're not going to burden the kernel with one specific implementation
> detail of gen_loader.
> Tomorrow we might change the gen_loader to use a triple:
> prog+map+btf or any other form.
> The existing approach allows all that extensibility and freedom
> to change the gen_loader.
If we're whatiffing about the future: in the event that you need a
separate btf I assume it will come down as a map; bpftool gen -S -M
would compute a signature over the loader and both maps and the in-
kernel piece of Blaise's patch would just work.
Regards,
James
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: Alexei Starovoitov @ 2025-10-11 16:31 UTC (permalink / raw)
To: James Bottomley
Cc: Paul Moore, Alexei Starovoitov, KP Singh, Linus Torvalds,
Blaise Boscaccy, bpf, LSM List, K. Y. Srinivasan, Daniel Borkmann,
Andrii Nakryiko, wufan, Quentin Monnet
In-Reply-To: <42bc677e031ed3df4f379cd3d6c9b3e1e8fadd87.camel@HansenPartnership.com>
On Sat, Oct 11, 2025 at 7:52 AM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> It doesn't need to, once we check both the loader and the map, the
> integrity is verified and the loader can be trusted to run and relocate
> the map into the bpf program
You should read KP's cover letter again and then research trusted
hash chains. Here is a quote from the first googled link:
"A trusted hash chain is a cryptographic process used to verify the
integrity and authenticity of data by creating a sequence of hash
values, where each hash is linked to the next".
In addition KP's algorithm was vetted by various security teams.
There is nothing novel here. It's a classic algorithm used
to verify integrity and that's what was implemented.
> > You need to realize that single loader plus single map is
> > an implementation choice of tools/lib/bpf/gen_loader.c.
> > It can do the same job with a single prog and no additional map.
>
> Yes, and if the light skeleton scheme embedded the relocation and the
> program itself into prog->insnsi then we wouldn't need the additional
> map verification before the load hook because the pkcs7 signature check
> would fully verify the integrity.
I'm fine if you want to hack gen_loader.c to produce the loader prog
without a map, but we're not going to pollute the kernel with
pointless apis, because you don't understand hash chains.
^ permalink raw reply
* [PATCH] apparmor: Replace sprintf/strcpy with scnprintf/strscpy in aa_policy_init
From: Thorsten Blum @ 2025-10-11 16:46 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
Cc: linux-hardening, Thorsten Blum, apparmor, linux-security-module,
linux-kernel
strcpy() is deprecated and sprintf() does not perform bounds checking
either. Although an overflow is unlikely, it's better to proactively
avoid it by using the safer strscpy() and scnprintf(), respectively.
Additionally, unify memory allocation for 'hname' to simplify and
improve aa_policy_init().
Link: https://github.com/KSPP/linux/issues/88
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/apparmor/lib.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
index 82dbb97ad406..acf7f5189bec 100644
--- a/security/apparmor/lib.c
+++ b/security/apparmor/lib.c
@@ -478,19 +478,17 @@ bool aa_policy_init(struct aa_policy *policy, const char *prefix,
const char *name, gfp_t gfp)
{
char *hname;
+ size_t hname_sz;
+ hname_sz = (prefix ? strlen(prefix) + 2 : 0) + strlen(name) + 1;
/* freed by policy_free */
- if (prefix) {
- hname = aa_str_alloc(strlen(prefix) + strlen(name) + 3, gfp);
- if (hname)
- sprintf(hname, "%s//%s", prefix, name);
- } else {
- hname = aa_str_alloc(strlen(name) + 1, gfp);
- if (hname)
- strcpy(hname, name);
- }
+ hname = aa_str_alloc(hname_sz, gfp);
if (!hname)
return false;
+ if (prefix)
+ scnprintf(hname, hname_sz, "%s//%s", prefix, name);
+ else
+ strscpy(hname, name, hname_sz);
policy->hname = hname;
/* base.name is a substring of fqname */
policy->name = basename(policy->hname);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: James Bottomley @ 2025-10-11 17:09 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Paul Moore, Alexei Starovoitov, KP Singh, Linus Torvalds,
Blaise Boscaccy, bpf, LSM List, K. Y. Srinivasan, Daniel Borkmann,
Andrii Nakryiko, wufan, Quentin Monnet
In-Reply-To: <CAADnVQ+M+_zLaqmd6As0z95A5BwGR8n8oFto-X-i4BgMvuhrXQ@mail.gmail.com>
On Sat, 2025-10-11 at 09:31 -0700, Alexei Starovoitov wrote:
> On Sat, Oct 11, 2025 at 7:52 AM James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> >
> > It doesn't need to, once we check both the loader and the map, the
> > integrity is verified and the loader can be trusted to run and
> > relocate the map into the bpf program
>
> You should read KP's cover letter again and then research trusted
> hash chains. Here is a quote from the first googled link:
>
> "A trusted hash chain is a cryptographic process used to verify the
> integrity and authenticity of data by creating a sequence of hash
> values, where each hash is linked to the next".
>
> In addition KP's algorithm was vetted by various security teams.
> There is nothing novel here. It's a classic algorithm used
> to verify integrity and that's what was implemented.
Both KP and Blaise's patch sets are implementations of trusted hash
chains. The security argument isn't about whether the hash chain
algorithm works, it's about where, in relation to the LSM hook, the
hash chain verification completes.
Regards,
James
^ permalink raw reply
* 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