* Re: [PATCH v6 1/9] lsm: Add LSM hook security_unix_find
From: Mickaël Salaün @ 2026-03-18 8:48 UTC (permalink / raw)
To: John Johansen, Georgia Garcia
Cc: Paul Moore, Günther Noack, James Morris, Serge E . Hallyn,
Tingmao Wang, Justin Suess, linux-security-module,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Sebastian Andrzej Siewior, Kuniyuki Iwashima,
Simon Horman, netdev, Alexander Viro, Christian Brauner
In-Reply-To: <2697b9f672967b1318630f2ffa21914f@paul-moore.com>
On Tue, Mar 17, 2026 at 05:34:57PM -0400, Paul Moore wrote:
> On Mar 15, 2026 =?UTF-8?q?G=C3=BCnther=20Noack?= <gnoack3000@gmail.com> wrote:
> >
> > Add a LSM hook security_unix_find.
> >
> > This hook is called to check the path of a named unix socket before a
> > connection is initiated. The peer socket may be inspected as well.
> >
> > Why existing hooks are unsuitable:
> >
> > Existing socket hooks, security_unix_stream_connect(),
> > security_unix_may_send(), and security_socket_connect() don't provide
> > TOCTOU-free / namespace independent access to the paths of sockets.
> >
> > (1) We cannot resolve the path from the struct sockaddr in existing hooks.
> > This requires another path lookup. A change in the path between the
> > two lookups will cause a TOCTOU bug.
> >
> > (2) We cannot use the struct path from the listening socket, because it
> > may be bound to a path in a different namespace than the caller,
> > resulting in a path that cannot be referenced at policy creation time.
> >
> > Cc: Günther Noack <gnoack3000@gmail.com>
> > Cc: Tingmao Wang <m@maowtm.org>
> > Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> > ---
> > include/linux/lsm_hook_defs.h | 5 +++++
> > include/linux/security.h | 11 +++++++++++
> > net/unix/af_unix.c | 13 ++++++++++---
> > security/security.c | 20 ++++++++++++++++++++
> > 4 files changed, 46 insertions(+), 3 deletions(-)
>
> Some really minor nitpicky things (below), but nothing critical.
> However, as we discussed, I would like to see the AppArmor folks comment
> on the new hook before we merge anything as I know they have an interest
> here.
John, Georgia, we've been discussing this new hook for a few months now
but didn't hear from you yet. We plan to merge this patch series with
the 7.1 merge window (in a few weeks), so before that I'd like to merge
it in -next in a few days to get a broader coverage. I'm pretty sure
this hook will work well with AppArmor too, but could you please take
look to confirm?
^ permalink raw reply
* Re: [PATCH] lsm: Fix the crash issue in xfrm_decode_session
From: Feng Yang @ 2026-03-18 8:37 UTC (permalink / raw)
To: yangfeng59949; +Cc: jmorris, linux-kernel, linux-security-module, paul, serge
In-Reply-To: <20260318061925.134954-1-yangfeng59949@163.com>
> After hooking the following BPF program:
> SEC("lsm/xfrm_decode_session")
> int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall)
> {
> return 1; // Any non-zero value
There is an error here. Any error value between -4095 and -1 will trigger this bug,
while other values in the range [-4095, 0] will be rejected by the validator.
> }
^ permalink raw reply
* Re: [PATCH] apparmor: hide unused get_loaddata_common_ref() function
From: John Johansen @ 2026-03-18 6:33 UTC (permalink / raw)
To: Arnd Bergmann, Paul Moore, James Morris, Serge E. Hallyn,
Maxime Bélair, Cengiz Can, Georgia Garcia
Cc: Arnd Bergmann, Christian Brauner, Jeff Layton, NeilBrown,
apparmor, linux-security-module, linux-kernel
In-Reply-To: <20260316135935.3321551-1-arnd@kernel.org>
On 3/16/26 06:59, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The newly introduced function is only used in an #ifdef block,
> which causes a harmless warning:
>
> security/apparmor/apparmorfs.c:177:28: error: 'get_loaddata_common_ref' defined but not used [-Werror=unused-function]
> 177 | static struct aa_loaddata *get_loaddata_common_ref(struct aa_common_ref *ref)
>
> Move the definition next to the user to avoid the warning.
>
> Fixes: 8e135b8aee5a ("apparmor: fix race between freeing data and fs accessing it")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: John Johansen <john.johansen@canonical.com>
this has now been pulled into my tree
> ---
> Alternatively, the #ifdef checks could be replaced with an
> 'if(IS_ENABLED(CONFIG_SECURITY_APPARMOR_EXPORT_BINARY) return;'
> check in __aa_fs_create_rawdata(), relying on the compiler's
> dead code elimination.
> ---
> security/apparmor/apparmorfs.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index ededaf46f3ca..f762b101d682 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -174,14 +174,6 @@ static struct aa_proxy *get_proxy_common_ref(struct aa_common_ref *ref)
> return NULL;
> }
>
> -static struct aa_loaddata *get_loaddata_common_ref(struct aa_common_ref *ref)
> -{
> - if (ref)
> - return aa_get_i_loaddata(container_of(ref, struct aa_loaddata,
> - count));
> - return NULL;
> -}
> -
> static void aa_put_common_ref(struct aa_common_ref *ref)
> {
> if (!ref)
> @@ -1318,6 +1310,14 @@ static const struct file_operations seq_rawdata_ ##NAME ##_fops = { \
> .release = seq_rawdata_release, \
> } \
>
> +static struct aa_loaddata *get_loaddata_common_ref(struct aa_common_ref *ref)
> +{
> + if (ref)
> + return aa_get_i_loaddata(container_of(ref, struct aa_loaddata,
> + count));
> + return NULL;
> +}
> +
> static int seq_rawdata_open(struct inode *inode, struct file *file,
> int (*show)(struct seq_file *, void *))
> {
^ permalink raw reply
* Re: [PATCH RESEND] apparmor: Remove redundant if check in sk_peer_get_label
From: John Johansen @ 2026-03-18 6:32 UTC (permalink / raw)
To: Thorsten Blum, Paul Moore, James Morris, Serge E. Hallyn
Cc: apparmor, linux-security-module, linux-kernel
In-Reply-To: <20260318002141.3362-3-thorsten.blum@linux.dev>
On 3/17/26 17:21, Thorsten Blum wrote:
> Remove the redundant if check in sk_peer_get_label() and return
> ERR_PTR(-ENOPROTOOPT) directly.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
this has now been pulled into my tree
> ---
> security/apparmor/lsm.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index c1d42fc72fdb..f7bcfed40222 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1528,15 +1528,11 @@ static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
> static struct aa_label *sk_peer_get_label(struct sock *sk)
> {
> struct aa_sk_ctx *ctx = aa_sock(sk);
> - struct aa_label *label = ERR_PTR(-ENOPROTOOPT);
>
> if (rcu_access_pointer(ctx->peer))
> return aa_get_label_rcu(&ctx->peer);
>
> - if (sk->sk_family != PF_UNIX)
> - return ERR_PTR(-ENOPROTOOPT);
> -
> - return label;
> + return ERR_PTR(-ENOPROTOOPT);
> }
>
> /**
^ permalink raw reply
* Re: [PATCH RESEND] apparmor: Use sysfs_emit in param_get_{audit,mode}
From: John Johansen @ 2026-03-18 6:31 UTC (permalink / raw)
To: Thorsten Blum, Paul Moore, James Morris, Serge E. Hallyn
Cc: apparmor, linux-security-module, linux-kernel
In-Reply-To: <20260318000758.1568-3-thorsten.blum@linux.dev>
On 3/17/26 17:08, Thorsten Blum wrote:
> Replace sprintf() with sysfs_emit() in param_get_audit() and
> param_get_mode(). sysfs_emit() is preferred for formatting sysfs output
> because it provides safer bounds checking. Add terminating newlines as
> suggested by checkpatch.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
this has now been pulled into my tree
> ---
> security/apparmor/lsm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index c1d42fc72fdb..cdf19a5e7626 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -17,6 +17,7 @@
> #include <linux/ptrace.h>
> #include <linux/ctype.h>
> #include <linux/sysctl.h>
> +#include <linux/sysfs.h>
> #include <linux/audit.h>
> #include <linux/user_namespace.h>
> #include <linux/netfilter_ipv4.h>
> @@ -2073,7 +2074,7 @@ static int param_get_audit(char *buffer, const struct kernel_param *kp)
> return -EINVAL;
> if (apparmor_initialized && !aa_current_policy_view_capable(NULL))
> return -EPERM;
> - return sprintf(buffer, "%s", audit_mode_names[aa_g_audit]);
> + return sysfs_emit(buffer, "%s\n", audit_mode_names[aa_g_audit]);
> }
>
> static int param_set_audit(const char *val, const struct kernel_param *kp)
> @@ -2101,8 +2102,7 @@ static int param_get_mode(char *buffer, const struct kernel_param *kp)
> return -EINVAL;
> if (apparmor_initialized && !aa_current_policy_view_capable(NULL))
> return -EPERM;
> -
> - return sprintf(buffer, "%s", aa_profile_mode_names[aa_g_profile_mode]);
> + return sysfs_emit(buffer, "%s\n", aa_profile_mode_names[aa_g_profile_mode]);
> }
>
> static int param_set_mode(const char *val, const struct kernel_param *kp)
^ permalink raw reply
* Re: [PATCH RESEND] apparmor: Replace memcpy + NUL termination with kmemdup_nul in do_setattr
From: John Johansen @ 2026-03-18 6:31 UTC (permalink / raw)
To: Thorsten Blum, Paul Moore, James Morris, Serge E. Hallyn
Cc: apparmor, linux-security-module, linux-kernel
In-Reply-To: <20260309224150.84575-3-thorsten.blum@linux.dev>
On 3/9/26 15:41, Thorsten Blum wrote:
> Use kmemdup_nul() to copy 'value' instead of using memcpy() followed by
> a manual NUL termination. No functional changes.
>
> Reviewed-by: Serge Hallyn <serge@hallyn.com>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
sorry this got lost in the mess of the last month, this has
now been pulled into my tree
> ---
> security/apparmor/lsm.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index c1d42fc72fdb..49aa6ad68838 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -858,12 +858,9 @@ static int do_setattr(u64 attr, void *value, size_t size)
>
> /* AppArmor requires that the buffer must be null terminated atm */
> if (args[size - 1] != '\0') {
> - /* null terminate */
> - largs = args = kmalloc(size + 1, GFP_KERNEL);
> + largs = args = kmemdup_nul(value, size, GFP_KERNEL);
> if (!args)
> return -ENOMEM;
> - memcpy(args, value, size);
> - args[size] = '\0';
> }
>
> error = -EINVAL;
^ permalink raw reply
* Re: [PATCH] apparmor: Use sysfs_emit in param_get_{audit,mode}
From: John Johansen @ 2026-03-18 6:29 UTC (permalink / raw)
To: Thorsten Blum, Paul Moore, James Morris, Serge E. Hallyn
Cc: apparmor, linux-security-module, linux-kernel
In-Reply-To: <20260222214038.287814-1-thorsten.blum@linux.dev>
On 2/22/26 13:40, Thorsten Blum wrote:
> Replace sprintf() with sysfs_emit() in param_get_audit() and
> param_get_mode(). sysfs_emit() is preferred for formatting sysfs output
> because it provides safer bounds checking. Add terminating newlines as
> suggested by checkpatch.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Acked-by: John Johansen <john.johansen@canoncal.com>
I have pulled this into my tree
> ---
> security/apparmor/lsm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index a87cd60ed206..1250192f7b12 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -17,6 +17,7 @@
> #include <linux/ptrace.h>
> #include <linux/ctype.h>
> #include <linux/sysctl.h>
> +#include <linux/sysfs.h>
> #include <linux/audit.h>
> #include <linux/user_namespace.h>
> #include <linux/netfilter_ipv4.h>
> @@ -2081,7 +2082,7 @@ static int param_get_audit(char *buffer, const struct kernel_param *kp)
> return -EINVAL;
> if (apparmor_initialized && !aa_current_policy_view_capable(NULL))
> return -EPERM;
> - return sprintf(buffer, "%s", audit_mode_names[aa_g_audit]);
> + return sysfs_emit(buffer, "%s\n", audit_mode_names[aa_g_audit]);
> }
>
> static int param_set_audit(const char *val, const struct kernel_param *kp)
> @@ -2109,8 +2110,7 @@ static int param_get_mode(char *buffer, const struct kernel_param *kp)
> return -EINVAL;
> if (apparmor_initialized && !aa_current_policy_view_capable(NULL))
> return -EPERM;
> -
> - return sprintf(buffer, "%s", aa_profile_mode_names[aa_g_profile_mode]);
> + return sysfs_emit(buffer, "%s\n", aa_profile_mode_names[aa_g_profile_mode]);
> }
>
> static int param_set_mode(const char *val, const struct kernel_param *kp)
^ permalink raw reply
* Re: [PATCH RESEND] apparmor: Replace memcpy + NUL termination with kmemdup_nul in do_setattr
From: John Johansen @ 2026-03-18 6:26 UTC (permalink / raw)
To: Thorsten Blum, Paul Moore, James Morris, Serge E. Hallyn
Cc: apparmor, linux-security-module, linux-kernel
In-Reply-To: <20260222204645.285727-1-thorsten.blum@linux.dev>
On 2/22/26 12:46, Thorsten Blum wrote:
> Use kmemdup_nul() to copy 'value' instead of using memcpy() followed by
> a manual NUL termination. No functional changes.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
sorry this got dropped. It has been now pulled into my tree
> ---
> security/apparmor/lsm.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index c1d42fc72fdb..49aa6ad68838 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -858,12 +858,9 @@ static int do_setattr(u64 attr, void *value, size_t size)
>
> /* AppArmor requires that the buffer must be null terminated atm */
> if (args[size - 1] != '\0') {
> - /* null terminate */
> - largs = args = kmalloc(size + 1, GFP_KERNEL);
> + largs = args = kmemdup_nul(value, size, GFP_KERNEL);
> if (!args)
> return -ENOMEM;
> - memcpy(args, value, size);
> - args[size] = '\0';
> }
>
> error = -EINVAL;
^ permalink raw reply
* [PATCH] lsm: Fix the crash issue in xfrm_decode_session
From: Feng Yang @ 2026-03-18 6:19 UTC (permalink / raw)
To: paul, jmorris, serge; +Cc: linux-security-module, linux-kernel
From: Feng Yang <yangfeng@kylinos.cn>
After hooking the following BPF program:
SEC("lsm/xfrm_decode_session")
int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall)
{
return 1; // Any non-zero value
}
Subsequent packet transmission triggers will cause a kernel panic:
[ 112.838874] ------------[ cut here ]------------
[ 112.838895] kernel BUG at security/security.c:5282!
[ 112.838902] invalid opcode: 0000 [#1] PREEMPT SMP PTI
[ 112.838905] CPU: 5 PID: 4962 Comm: test Kdump: loaded Not tainted 6.19.0-rc5-gae23bc81ddf7 #2 PREEMPT(full)
[ 112.838907] Source Version: 55e2f799c748c8e195569363edbd1d6a4159675a
[ 112.838908] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 112.838909] RIP: 0010:security_skb_classify_flow+0x3f/0x50
[ 112.838914] Code: 85 db 74 28 49 89 fc 48 8d 6e 14 eb 08 48 8b 1b 48 85 db 74 17 31 d2 48 8b 43 18 48 89 ee 4c 89 e7 e8 05 33 86 00 85 c0 74 e3 <0f> 0b 5b 5d 41 5c c3 cc cc cc cc 66 0f 1f 44 00 00 90 90 90 90 90
[ 112.838915] RSP: 0018:ffffc28400200b10 EFLAGS: 00010202
[ 112.838918] RAX: 0000000000000001 RBX: ffffffff91d346d8 RCX: 0000000000000000
[ 112.838919] RDX: ffffa0890f5eaf80 RSI: 0000000000000001 RDI: ffffa0890f5eaf80
[ 112.838920] RBP: ffffc28400200d04 R08: 00000000000000c7 R09: 0000000000000002
[ 112.838922] R10: 0000000000000000 R11: 000000000000000f R12: ffffa089086dedc0
[ 112.838923] R13: ffffc28400200cf0 R14: ffffa08901ab2000 R15: 0000000000000000
[ 112.838926] FS: 00007fb087dd2680(0000) GS:ffffa0891ba80000(0000) knlGS:0000000000000000
[ 112.838927] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 112.838929] CR2: 00007fb087d1b940 CR3: 0000000107520006 CR4: 00000000000706e0
[ 112.838930] Call Trace:
[ 112.838931] <IRQ>
[ 112.838933] icmp_route_lookup.constprop.0+0xd7/0x460
[ 112.838941] ? switch_hrtimer_base+0x135/0x180
[ 112.838944] ? update_sg_lb_stats+0x9c/0x440
[ 112.838949] __icmp_send+0x3d3/0x740
[ 112.838952] ? __udp4_lib_rcv+0x427/0x6f0
[ 112.838955] __udp4_lib_rcv+0x427/0x6f0
[ 112.838957] ip_protocol_deliver_rcu+0xb7/0x170
[ 112.838960] ip_local_deliver_finish+0x76/0xa0
[ 112.838961] __netif_receive_skb_one_core+0x89/0xa0
[ 112.838967] process_backlog+0x95/0x140
[ 112.838969] __napi_poll+0x2b/0x1c0
[ 112.838971] net_rx_action+0x2aa/0x3a0
[ 112.838972] ? swake_up_one+0x41/0x70
[ 112.838974] ? kvm_sched_clock_read+0x11/0x20
[ 112.838977] handle_softirqs+0xe3/0x2e0
[ 112.838980] do_softirq+0x43/0x60
[ 112.838982] </IRQ>
[ 112.838982] <TASK>
[ 112.838983] __local_bh_enable_ip+0x68/0x70
[ 112.838985] __dev_queue_xmit+0x1c4/0x820
[ 112.838987] ? nf_hook_slow+0x45/0xd0
[ 112.838989] ip_finish_output2+0x1da/0x4a0
[ 112.838992] ip_send_skb+0x86/0x90
[ 112.838994] udp_send_skb+0x15e/0x380
[ 112.838996] udp_sendmsg+0xb9a/0xf80
[ 112.838998] ? __pfx_ip_generic_getfrag+0x10/0x10
[ 112.839003] ? __sys_sendto+0x1e4/0x210
[ 112.839005] __sys_sendto+0x1e4/0x210
[ 112.839007] ? __handle_mm_fault+0x2fc/0x6c0
[ 112.839013] __x64_sys_sendto+0x24/0x30
[ 112.839014] do_syscall_64+0x5f/0x270
[ 112.839017] entry_SYSCALL_64_after_hwframe+0x76/0xe0
[ 112.839020] RIP: 0033:0x7fb087cfdb17
[ 112.839021] Code: 0c 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 80 3d 55 c8 0c 00 00 41 89 ca 74 10 b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 71 c3 55 48 83 ec 30 44 89 4c 24 2c 4c 89 44
[ 112.839023] RSP: 002b:00007ffea64704e8 EFLAGS: 00000202 ORIG_RAX: 000000000000002c
[ 112.839025] RAX: ffffffffffffffda RBX: 00007ffea6470638 RCX: 00007fb087cfdb17
[ 112.839026] RDX: 0000000000000008 RSI: 00007ffea64704f8 RDI: 0000000000000003
[ 112.839027] RBP: 00007ffea6470520 R08: 00007ffea6470500 R09: 0000000000000010
[ 112.839029] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000000
[ 112.839030] R13: 00007ffea6470648 R14: 0000000000403df0 R15: 00007fb087e15000
[ 112.839032] </TASK>
This BUG_ON was first mentioned in [1], but I could not find any explanatory record of why this check is needed.
[1] https://lore.kernel.org/all/Pine.LNX.4.64.0607122149070.573@d.namei/
In the existing LSM_HOOK_INIT(xfrm_decode_session, selinux_xfrm_decode_session),
when the `ckall` parameter of the `selinux_xfrm_decode_session` function is 0,
it can only return 0 and will not trigger BUG_ON.
Therefore, remove the BUG_ON check to fix this issue.
Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
Closes: https://lore.kernel.org/all/4c4d04ba.6c12b.19c039b69e6.Coremail.kaiyanm@hust.edu.cn/
Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
---
security/security.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/security/security.c b/security/security.c
index 67af9228c4e9..198f650070da 100644
--- a/security/security.c
+++ b/security/security.c
@@ -4991,10 +4991,7 @@ int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid)
void security_skb_classify_flow(struct sk_buff *skb, struct flowi_common *flic)
{
- int rc = call_int_hook(xfrm_decode_session, skb, &flic->flowic_secid,
- 0);
-
- BUG_ON(rc);
+ call_int_hook(xfrm_decode_session, skb, &flic->flowic_secid, 0);
}
EXPORT_SYMBOL(security_skb_classify_flow);
#endif /* CONFIG_SECURITY_NETWORK_XFRM */
--
2.43.0
^ permalink raw reply related
* Re: [apparmor][PATCH] apparmor: fix incorrect success return value in unpack_tag_headers()
From: John Johansen @ 2026-03-18 5:53 UTC (permalink / raw)
To: Massimiliano Pellizzer; +Cc: apparmor, linux-security-module, linux-kernel
In-Reply-To: <20260210172159.535137-1-mpellizzer.dev@gmail.com>
On 2/10/26 09:21, Massimiliano Pellizzer wrote:
> unpack_tag_headers() returns `true` (1) on success instead of 0.
> Since it's caller unpack_tags() checks the return value with
> `if (error)`, a non-zero success value is incorrectly treated as
> a failure, causing tag header unpacking to always even if the data
> is well-formed.
>
> Change the success return in unpack_tag_headers() from `true` to 0.
>
> Fixes: 3d28e2397af7 ("apparmor: add support loading per permission tagging")
> Signed-off-by: Massimiliano Pellizzer <mpellizzer.dev@gmail.com>
sorry, my reply to this seems to have failed. This was pulled in for the
7.0 PR
Acked-by: John Johansen <john.johansen@canonical.com>
> ---
> security/apparmor/policy_unpack.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> index dc908e1f5a88..221208788025 100644
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@ -825,7 +825,7 @@ static int unpack_tag_headers(struct aa_ext *e, struct aa_tags_struct *tags)
> tags->hdrs.size = size;
> tags->hdrs.table = hdrs;
> AA_DEBUG(DEBUG_UNPACK, "headers %ld size %d", (long) hdrs, size);
> - return true;
> + return 0;
>
> fail:
> kfree_sensitive(hdrs);
^ permalink raw reply
* Re: [PATCH] apparmor: Remove redundant if check in sk_peer_get_label
From: John Johansen @ 2026-03-18 5:59 UTC (permalink / raw)
To: Thorsten Blum, Paul Moore, James Morris, Serge E. Hallyn
Cc: apparmor, linux-security-module, linux-kernel
In-Reply-To: <20260204220734.1008069-2-thorsten.blum@linux.dev>
On 2/4/26 14:07, Thorsten Blum wrote:
> Remove the redundant if check in sk_peer_get_label() and return
> ERR_PTR(-ENOPROTOOPT) directly.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Acked-by: John Johansen <john.johansen@canonical.com>
I have pulled this into my tree
> ---
> security/apparmor/lsm.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index a87cd60ed206..54343f7c96a4 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1536,15 +1536,11 @@ static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
> static struct aa_label *sk_peer_get_label(struct sock *sk)
> {
> struct aa_sk_ctx *ctx = aa_sock(sk);
> - struct aa_label *label = ERR_PTR(-ENOPROTOOPT);
>
> if (rcu_access_pointer(ctx->peer))
> return aa_get_label_rcu(&ctx->peer);
>
> - if (sk->sk_family != PF_UNIX)
> - return ERR_PTR(-ENOPROTOOPT);
> -
> - return label;
> + return ERR_PTR(-ENOPROTOOPT);
> }
>
> /**
^ permalink raw reply
* Re: [PATCH] apparmor: Replace memcpy + NUL termination with kmemdup_nul in do_setattr
From: John Johansen @ 2026-03-18 5:57 UTC (permalink / raw)
To: Thorsten Blum, Paul Moore, James Morris, Serge E. Hallyn
Cc: apparmor, linux-security-module, linux-kernel
In-Reply-To: <20260125210014.154432-2-thorsten.blum@linux.dev>
On 1/25/26 13:00, Thorsten Blum wrote:
> Use kmemdup_nul() to copy 'value' instead of using memcpy() followed by
> a manual NUL termination. No functional changes.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Acked-by: John Johansen <john.johansen@canonical.com>
I have pulled this into my tree
> ---
> security/apparmor/lsm.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index a87cd60ed206..98b92af5890e 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -866,12 +866,9 @@ static int do_setattr(u64 attr, void *value, size_t size)
>
> /* AppArmor requires that the buffer must be null terminated atm */
> if (args[size - 1] != '\0') {
> - /* null terminate */
> - largs = args = kmalloc(size + 1, GFP_KERNEL);
> + largs = args = kmemdup_nul(value, size, GFP_KERNEL);
> if (!args)
> return -ENOMEM;
> - memcpy(args, value, size);
> - args[size] = '\0';
> }
>
> error = -EINVAL;
^ permalink raw reply
* Re: [PATCH v7 1/9] lsm: Add LSM hook security_unix_find
From: Paul Moore @ 2026-03-18 1:28 UTC (permalink / raw)
To: Justin Suess
Cc: bigeasy, brauner, demiobenour, fahimitahera, gnoack3000, hi,
horms, ivanov.mikhail1, jannh, jmorris, john.johansen,
konstantin.meskhidze, kuniyu, linux-security-module, m, matthieu,
mic, netdev, samasth.norway.ananda, serge, viro
In-Reply-To: <20260317232041.330576-1-utilityemal77@gmail.com>
On Tue, Mar 17, 2026 at 7:21 PM Justin Suess <utilityemal77@gmail.com> wrote:
>
> Paul,
>
> I updated the hook placement as per your suggestions. Moving the hook into
> the block does require duplicate stubs, but I don't see another way to move the
> stub into that block and properly handle the case where CONFIG_SECURITY_PATH is
> defined but CONFIG_SECURITY_NETWORK isn't. If the stub is moved into that #else
> block it will never be defined in that case.
Oof, yes, my apologies, I must have still been thinking about the
LSM_HOOK() change and didn't think through the problems with moving
the declaration. If you aren't too upset about changing it back, I
would prefer it back the way you had it in security.h originally.
Sorry for the noise :/
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 83a646d72f6f..3f8c23ad1199 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1641,6 +1641,14 @@ static inline int security_watch_key(struct key *key)
> int security_netlink_send(struct sock *sk, struct sk_buff *skb);
> int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk);
> int security_unix_may_send(struct socket *sock, struct socket *other);
> +#ifdef CONFIG_SECURITY_PATH
> +int security_unix_find(const struct path *path, struct sock *other, int flags);
> +#else /* CONFIG_SECURITY_PATH */
> +static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_SECURITY_PATH */
> int security_socket_create(int family, int type, int protocol, int kern);
> int security_socket_post_create(struct socket *sock, int family,
> int type, int protocol, int kern);
> @@ -1712,6 +1720,11 @@ static inline int security_unix_may_send(struct socket *sock,
> return 0;
> }
>
> +static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
> +{
> + return 0;
> +}
> +
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC bpf-next 0/4] audit: Expose audit subsystem to BPF LSM programs via BPF kfuncs
From: Alexei Starovoitov @ 2026-03-18 1:15 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: Frederick Lawler, Paul Moore, James Morris, Serge E. Hallyn,
Eric Paris, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Shuah Khan, Mickaël Salaün, Günther Noack, LKML,
LSM List, audit, bpf, open list:KERNEL SELFTEST FRAMEWORK,
kernel-team
In-Reply-To: <CAP01T77VyW=5SHDvM3HXPNHaxRdzs8H__MOh2zx1dQ6STeAUtg@mail.gmail.com>
On Mon, Mar 16, 2026 at 7:44 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Wed, 11 Mar 2026 at 22:31, Frederick Lawler <fred@cloudflare.com> wrote:
> >
> > The motivation behind the change is to give BPF LSM developers the
> > ability to report accesses via the audit subsystem much like how LSMs
> > operate today.
Sure, but bpf lsm-s don't need to follow such conventions.
audit is nothing but a message passing from kernel to user space
and done in a very inefficient way by wrapping strings into skb/netlink.
bpf progs can do this message passing already via various ways:
perfbuf, ringbuf, streams.
Teach your user space to consume one of them.
^ permalink raw reply
* [PATCH RESEND] apparmor: Remove redundant if check in sk_peer_get_label
From: Thorsten Blum @ 2026-03-18 0:21 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
Cc: Thorsten Blum, apparmor, linux-security-module, linux-kernel
Remove the redundant if check in sk_peer_get_label() and return
ERR_PTR(-ENOPROTOOPT) directly.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/apparmor/lsm.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index c1d42fc72fdb..f7bcfed40222 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1528,15 +1528,11 @@ static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
static struct aa_label *sk_peer_get_label(struct sock *sk)
{
struct aa_sk_ctx *ctx = aa_sock(sk);
- struct aa_label *label = ERR_PTR(-ENOPROTOOPT);
if (rcu_access_pointer(ctx->peer))
return aa_get_label_rcu(&ctx->peer);
- if (sk->sk_family != PF_UNIX)
- return ERR_PTR(-ENOPROTOOPT);
-
- return label;
+ return ERR_PTR(-ENOPROTOOPT);
}
/**
^ permalink raw reply related
* [PATCH RESEND] apparmor: Use sysfs_emit in param_get_{audit,mode}
From: Thorsten Blum @ 2026-03-18 0:08 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
Cc: Thorsten Blum, apparmor, linux-security-module, linux-kernel
Replace sprintf() with sysfs_emit() in param_get_audit() and
param_get_mode(). sysfs_emit() is preferred for formatting sysfs output
because it provides safer bounds checking. Add terminating newlines as
suggested by checkpatch.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/apparmor/lsm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index c1d42fc72fdb..cdf19a5e7626 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -17,6 +17,7 @@
#include <linux/ptrace.h>
#include <linux/ctype.h>
#include <linux/sysctl.h>
+#include <linux/sysfs.h>
#include <linux/audit.h>
#include <linux/user_namespace.h>
#include <linux/netfilter_ipv4.h>
@@ -2073,7 +2074,7 @@ static int param_get_audit(char *buffer, const struct kernel_param *kp)
return -EINVAL;
if (apparmor_initialized && !aa_current_policy_view_capable(NULL))
return -EPERM;
- return sprintf(buffer, "%s", audit_mode_names[aa_g_audit]);
+ return sysfs_emit(buffer, "%s\n", audit_mode_names[aa_g_audit]);
}
static int param_set_audit(const char *val, const struct kernel_param *kp)
@@ -2101,8 +2102,7 @@ static int param_get_mode(char *buffer, const struct kernel_param *kp)
return -EINVAL;
if (apparmor_initialized && !aa_current_policy_view_capable(NULL))
return -EPERM;
-
- return sprintf(buffer, "%s", aa_profile_mode_names[aa_g_profile_mode]);
+ return sysfs_emit(buffer, "%s\n", aa_profile_mode_names[aa_g_profile_mode]);
}
static int param_set_mode(const char *val, const struct kernel_param *kp)
^ permalink raw reply related
* [PATCH v7 1/9] lsm: Add LSM hook security_unix_find
From: Justin Suess @ 2026-03-17 23:20 UTC (permalink / raw)
To: paul
Cc: bigeasy, brauner, demiobenour, fahimitahera, gnoack3000, hi,
horms, ivanov.mikhail1, jannh, jmorris, john.johansen,
konstantin.meskhidze, kuniyu, linux-security-module, m, matthieu,
mic, netdev, samasth.norway.ananda, serge, utilityemal77, viro
In-Reply-To: <2697b9f672967b1318630f2ffa21914f@paul-moore.com>
Add a LSM hook security_unix_find.
This hook is called to check the path of a named unix socket before a
connection is initiated. The peer socket may be inspected as well.
Why existing hooks are unsuitable:
Existing socket hooks, security_unix_stream_connect(),
security_unix_may_send(), and security_socket_connect() don't provide
TOCTOU-free / namespace independent access to the paths of sockets.
(1) We cannot resolve the path from the struct sockaddr in existing hooks.
This requires another path lookup. A change in the path between the
two lookups will cause a TOCTOU bug.
(2) We cannot use the struct path from the listening socket, because it
may be bound to a path in a different namespace than the caller,
resulting in a path that cannot be referenced at policy creation time.
Consumers of the hook wishing to reference @other are responsible
for acquiring the unix_state_lock and checking for the SOCK_DEAD flag
therein, ensuring the socket hasn't died since lookup.
Cc: Günther Noack <gnoack3000@gmail.com>
Cc: Tingmao Wang <m@maowtm.org>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
Paul,
I updated the hook placement as per your suggestions. Moving the hook into
the block does require duplicate stubs, but I don't see another way to move the
stub into that block and properly handle the case where CONFIG_SECURITY_PATH is
defined but CONFIG_SECURITY_NETWORK isn't. If the stub is moved into that #else
block it will never be defined in that case.
I removed the self-evident comment as well from security_unix_find and added
the whitespace.
I also updated the comments and commit message with respect to locking.
include/linux/lsm_hook_defs.h | 6 ++++++
include/linux/security.h | 13 +++++++++++++
net/unix/af_unix.c | 10 +++++++---
security/security.c | 19 +++++++++++++++++++
4 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 8c42b4bde09c..0017a540c2fb 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -321,6 +321,12 @@ LSM_HOOK(int, 0, watch_key, struct key *key)
LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
struct sock *newsk)
LSM_HOOK(int, 0, unix_may_send, struct socket *sock, struct socket *other)
+
+#ifdef CONFIG_SECURITY_PATH
+LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other,
+ int flags)
+#endif /* CONFIG_SECURITY_PATH */
+
LSM_HOOK(int, 0, socket_create, int family, int type, int protocol, int kern)
LSM_HOOK(int, 0, socket_post_create, struct socket *sock, int family, int type,
int protocol, int kern)
diff --git a/include/linux/security.h b/include/linux/security.h
index 83a646d72f6f..3f8c23ad1199 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1641,6 +1641,14 @@ static inline int security_watch_key(struct key *key)
int security_netlink_send(struct sock *sk, struct sk_buff *skb);
int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk);
int security_unix_may_send(struct socket *sock, struct socket *other);
+#ifdef CONFIG_SECURITY_PATH
+int security_unix_find(const struct path *path, struct sock *other, int flags);
+#else /* CONFIG_SECURITY_PATH */
+static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
+{
+ return 0;
+}
+#endif /* CONFIG_SECURITY_PATH */
int security_socket_create(int family, int type, int protocol, int kern);
int security_socket_post_create(struct socket *sock, int family,
int type, int protocol, int kern);
@@ -1712,6 +1720,11 @@ static inline int security_unix_may_send(struct socket *sock,
return 0;
}
+static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
+{
+ return 0;
+}
+
static inline int security_socket_create(int family, int type,
int protocol, int kern)
{
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 3756a93dc63a..5ef3c2e31757 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1231,11 +1231,15 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
goto path_put;
err = -EPROTOTYPE;
- if (sk->sk_type == type)
- touch_atime(&path);
- else
+ if (sk->sk_type != type)
goto sock_put;
+ err = security_unix_find(&path, sk, flags);
+ if (err)
+ goto sock_put;
+
+ touch_atime(&path);
+
path_put(&path);
return sk;
diff --git a/security/security.c b/security/security.c
index 67af9228c4e9..f8df5e1b55e6 100644
--- a/security/security.c
+++ b/security/security.c
@@ -4073,6 +4073,25 @@ int security_unix_may_send(struct socket *sock, struct socket *other)
}
EXPORT_SYMBOL(security_unix_may_send);
+#ifdef CONFIG_SECURITY_PATH
+/**
+ * security_unix_find() - Check if a named AF_UNIX socket can connect
+ * @path: path of the socket being connected to
+ * @other: peer sock
+ * @flags: flags associated with the socket
+ *
+ * This hook is called to check permissions before connecting to a named
+ * AF_UNIX socket. The caller does not hold any locks on @other.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_unix_find(const struct path *path, struct sock *other, int flags)
+{
+ return call_int_hook(unix_find, path, other, flags);
+}
+EXPORT_SYMBOL(security_unix_find);
+#endif /* CONFIG_SECURITY_PATH */
+
/**
* security_socket_create() - Check if creating a new socket is allowed
* @family: protocol family
--
2.51.0
^ permalink raw reply related
* Re: [PATCH 2/3] lsm: add the security_mmap_backing_file() hook
From: Paul Moore @ 2026-03-17 22:42 UTC (permalink / raw)
To: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs
Cc: Amir Goldstein, Gao Xiang
In-Reply-To: <20260316213606.374109-7-paul@paul-moore.com>
On Mon, Mar 16, 2026 at 5:36 PM Paul Moore <paul@paul-moore.com> wrote:
>
> Add the security_mmap_backing_file() hook to allow LSMs to properly
> enforce access controls on mmap() operations on stacked filesystems
> such as overlayfs.
>
> The existing security_mmap_file() hook exists as an access control point
> for mmap() but on stacked filesystems it only provides a way to enforce
> access controls on the user visible file. In order to enforce access
> controls on the underlying backing file, the new
> security_mmap_backing_file() hook is needed.
>
> In addition the LSM hook additions, this patch also constifies the file
> struct field in the LSM common_audit_data struct to better support LSMs
> that will likely need to pass a const file struct pointer from the new
> backing_file_user_path_file() API into the common LSM audit code.
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
> fs/backing-file.c | 8 +++++++-
> fs/erofs/ishare.c | 6 ++++++
> include/linux/lsm_audit.h | 2 +-
> include/linux/lsm_hook_defs.h | 2 ++
> include/linux/security.h | 10 ++++++++++
> security/security.c | 25 +++++++++++++++++++++++++
> 6 files changed, 51 insertions(+), 2 deletions(-)
...
> diff --git a/fs/erofs/ishare.c b/fs/erofs/ishare.c
> index 17a4941d4518..d66c3a935d83 100644
> --- a/fs/erofs/ishare.c
> +++ b/fs/erofs/ishare.c
> @@ -150,8 +150,14 @@ static ssize_t erofs_ishare_file_read_iter(struct kiocb *iocb,
> static int erofs_ishare_mmap(struct file *file, struct vm_area_struct *vma)
> {
> struct file *realfile = file->private_data;
> + int err;
>
> vma_set_file(vma, realfile);
> +
> + err = security_mmap_backing_file(vma, realfile, file);
> + if (err)
> + return err;
> +
> return generic_file_readonly_mmap(file, vma);
> }
The kernel test robot helpfully pointed out that this patch was
missing a security.h include for the newly added LSM hook. The fixup
below has been applied to the patch in lsm/stable-7.0.
diff --git a/fs/erofs/ishare.c b/fs/erofs/ishare.c
index d66c3a935d83..f80925b66599 100644
--- a/fs/erofs/ishare.c
+++ b/fs/erofs/ishare.c
@@ -4,6 +4,7 @@
*/
#include <linux/xxhash.h>
#include <linux/mount.h>
+#include <linux/security.h>
#include "internal.h"
#include "xattr.h"
--
paul-moore.com
^ permalink raw reply related
* Re: [PATCH v6 1/9] lsm: Add LSM hook security_unix_find
From: Paul Moore @ 2026-03-17 21:34 UTC (permalink / raw)
To: Günther Noack, Mickaël Salaün, John Johansen,
James Morris, Serge E . Hallyn
Cc: Günther Noack, Tingmao Wang, Justin Suess,
linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Jann Horn, Tahera Fahimi, Sebastian Andrzej Siewior,
Kuniyuki Iwashima, Simon Horman, netdev, Alexander Viro,
Christian Brauner
In-Reply-To: <20260315222150.121952-2-gnoack3000@gmail.com>
On Mar 15, 2026 =?UTF-8?q?G=C3=BCnther=20Noack?= <gnoack3000@gmail.com> wrote:
>
> Add a LSM hook security_unix_find.
>
> This hook is called to check the path of a named unix socket before a
> connection is initiated. The peer socket may be inspected as well.
>
> Why existing hooks are unsuitable:
>
> Existing socket hooks, security_unix_stream_connect(),
> security_unix_may_send(), and security_socket_connect() don't provide
> TOCTOU-free / namespace independent access to the paths of sockets.
>
> (1) We cannot resolve the path from the struct sockaddr in existing hooks.
> This requires another path lookup. A change in the path between the
> two lookups will cause a TOCTOU bug.
>
> (2) We cannot use the struct path from the listening socket, because it
> may be bound to a path in a different namespace than the caller,
> resulting in a path that cannot be referenced at policy creation time.
>
> Cc: Günther Noack <gnoack3000@gmail.com>
> Cc: Tingmao Wang <m@maowtm.org>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
> include/linux/lsm_hook_defs.h | 5 +++++
> include/linux/security.h | 11 +++++++++++
> net/unix/af_unix.c | 13 ++++++++++---
> security/security.c | 20 ++++++++++++++++++++
> 4 files changed, 46 insertions(+), 3 deletions(-)
Some really minor nitpicky things (below), but nothing critical.
However, as we discussed, I would like to see the AppArmor folks comment
on the new hook before we merge anything as I know they have an interest
here.
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 8c42b4bde09c..7a0fd3dbfa29 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -317,6 +317,11 @@ LSM_HOOK(int, 0, post_notification, const struct cred *w_cred,
> LSM_HOOK(int, 0, watch_key, struct key *key)
> #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other,
> + int flags)
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
I'd suggest moving this into the CONFIG_SECURITY_NETWORK that is directly
below this block so you only have to check the CONFIG_SECURITY_PATH
state. You can place it directly after the existing security_unix*()
hooks.
> #ifdef CONFIG_SECURITY_NETWORK
> LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
> struct sock *newsk)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 83a646d72f6f..99a33d8eb28d 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1931,6 +1931,17 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
> }
> #endif /* CONFIG_SECURITY_NETWORK */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +
> +int security_unix_find(const struct path *path, struct sock *other, int flags);
> +
> +#else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
Similar to above, I would suggest moving this into the
CONFIG_SECURITY_NETWORK block directly above this so you only need to
check for CONFIG_SECURITY_PATH when declaring the security_unix_find()
hook.
Extra bonus points if you locate it next to the existing security_unix*()
hooks.
> #ifdef CONFIG_SECURITY_INFINIBAND
> int security_ib_pkey_access(void *sec, u64 subnet_prefix, u16 pkey);
> int security_ib_endport_manage_subnet(void *sec, const char *name, u8 port_num);
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 3756a93dc63a..aced28179bac 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1231,11 +1231,18 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
> goto path_put;
>
> err = -EPROTOTYPE;
> - if (sk->sk_type == type)
> - touch_atime(&path);
> - else
> + if (sk->sk_type != type)
> goto sock_put;
>
> + /*
> + * We call the hook because we know that the inode is a socket and we
> + * hold a valid reference to it via the path.
> + */
I'm not entirely sure that this comment is necessary as it doesn't tell
us anything we don't already know from a quick glance at the code. Is
there something sneaky, or hard to see, that we should know about?
> + err = security_unix_find(&path, sk, flags);
> + if (err)
> + goto sock_put;
> + touch_atime(&path);
> +
This is hyper nitpicky, but I'd probably put one line of vertical
whitespace before the touch_atime() call as it has nothing to do with
the LSM hook being called.
> path_put(&path);
>
> return sk;
> diff --git a/security/security.c b/security/security.c
> index 67af9228c4e9..c73196b8db4b 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4731,6 +4731,26 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
>
> #endif /* CONFIG_SECURITY_NETWORK */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +/**
> + * security_unix_find() - Check if a named AF_UNIX socket can connect
> + * @path: path of the socket being connected to
> + * @other: peer sock
> + * @flags: flags associated with the socket
> + *
> + * This hook is called to check permissions before connecting to a named
> + * AF_UNIX socket.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_unix_find(const struct path *path, struct sock *other, int flags)
> +{
> + return call_int_hook(unix_find, path, other, flags);
> +}
> +EXPORT_SYMBOL(security_unix_find);
> +
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
You can probably guess that I'm going to suggest placing this inside the
existing CONFIG_SECURITY_NETWORK block, right after the existing UNIX
hooks :)
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v5 2/9] landlock: Control pathname UNIX domain socket resolution by path
From: Mickaël Salaün @ 2026-03-17 21:14 UTC (permalink / raw)
To: Günther Noack
Cc: Günther Noack, John Johansen, Tingmao Wang, Justin Suess,
Jann Horn, linux-security-module, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
Demi Marie Obenour, Alyssa Ross, Tahera Fahimi
In-Reply-To: <20260314.f83f9a697865@gnoack.org>
On Sun, Mar 15, 2026 at 12:15:10AM +0100, Günther Noack wrote:
> On Sun, Mar 08, 2026 at 12:50:06PM +0100, Mickaël Salaün wrote:
> > On Sun, Mar 08, 2026 at 10:09:52AM +0100, Mickaël Salaün wrote:
> > > On Thu, Feb 19, 2026 at 02:59:38PM +0100, Günther Noack wrote:
> > > > On Thu, Feb 19, 2026 at 10:45:44AM +0100, Mickaël Salaün wrote:
> > > > > On Wed, Feb 18, 2026 at 10:37:16AM +0100, Mickaël Salaün wrote:
> > > > > > On Sun, Feb 15, 2026 at 11:51:50AM +0100, Günther Noack wrote:
> > > > > > > * Add a new access right LANDLOCK_ACCESS_FS_RESOLVE_UNIX, which
> > > > > > > controls the look up operations for named UNIX domain sockets. The
> > > > > > > resolution happens during connect() and sendmsg() (depending on
> > > > > > > socket type).
> > > > > > > * Hook into the path lookup in unix_find_bsd() in af_unix.c, using a
> > > > > > > LSM hook. Make policy decisions based on the new access rights
> > > > > > > * Increment the Landlock ABI version.
> > > > > > > * Minor test adaptions to keep the tests working.
> > > > > > >
> > > > > > > With this access right, access is granted if either of the following
> > > > > > > conditions is met:
> > > > > > >
> > > > > > > * The target socket's filesystem path was allow-listed using a
> > > > > > > LANDLOCK_RULE_PATH_BENEATH rule, *or*:
> > > > > > > * The target socket was created in the same Landlock domain in which
> > > > > > > LANDLOCK_ACCESS_FS_RESOLVE_UNIX was restricted.
> > > > > > >
> > > > > > > In case of a denial, connect() and sendmsg() return EACCES, which is
> > > > > > > the same error as it is returned if the user does not have the write
> > > > > > > bit in the traditional Unix file system permissions of that file.
> > > > > > >
> > > > > > > This feature was created with substantial discussion and input from
> > > > > > > Justin Suess, Tingmao Wang and Mickaël Salaün.
> > > > > > >
> > > > > > > Cc: Tingmao Wang <m@maowtm.org>
> > > > > > > Cc: Justin Suess <utilityemal77@gmail.com>
> > > > > > > Cc: Mickaël Salaün <mic@digikod.net>
> > > > > > > Suggested-by: Jann Horn <jannh@google.com>
> > > > > > > Link: https://github.com/landlock-lsm/linux/issues/36
> > > > > > > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > > > > > > ---
> > > > > > > include/uapi/linux/landlock.h | 10 ++
> > > > > > > security/landlock/access.h | 11 +-
> > > > > > > security/landlock/audit.c | 1 +
> > > > > > > security/landlock/fs.c | 102 ++++++++++++++++++-
> > > > > > > security/landlock/limits.h | 2 +-
> > > > > > > security/landlock/syscalls.c | 2 +-
> > > > > > > tools/testing/selftests/landlock/base_test.c | 2 +-
> > > > > > > tools/testing/selftests/landlock/fs_test.c | 5 +-
> > > > > > > 8 files changed, 128 insertions(+), 7 deletions(-)
> > > > >
> > > > > > > index 60ff217ab95b..8d0edf94037d 100644
> > > > > > > --- a/security/landlock/audit.c
> > > > > > > +++ b/security/landlock/audit.c
> > > > > > > @@ -37,6 +37,7 @@ static const char *const fs_access_strings[] = {
> > > > > > > [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer",
> > > > > > > [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate",
> > > > > > > [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev",
> > > > > > > + [BIT_INDEX(LANDLOCK_ACCESS_FS_RESOLVE_UNIX)] = "fs.resolve_unix",
> > > > > > > };
> > > > > > >
> > > > > > > static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
> > > > > > > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> > > > > > > index e764470f588c..76035c6f2bf1 100644
> > > > > > > --- a/security/landlock/fs.c
> > > > > > > +++ b/security/landlock/fs.c
> > > > > > > @@ -27,6 +27,7 @@
> > > > > > > #include <linux/lsm_hooks.h>
> > > > > > > #include <linux/mount.h>
> > > > > > > #include <linux/namei.h>
> > > > > > > +#include <linux/net.h>
> > > > > > > #include <linux/path.h>
> > > > > > > #include <linux/pid.h>
> > > > > > > #include <linux/rcupdate.h>
> > > > > > > @@ -314,7 +315,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> > > > > > > LANDLOCK_ACCESS_FS_WRITE_FILE | \
> > > > > > > LANDLOCK_ACCESS_FS_READ_FILE | \
> > > > > > > LANDLOCK_ACCESS_FS_TRUNCATE | \
> > > > > > > - LANDLOCK_ACCESS_FS_IOCTL_DEV)
> > > > > > > + LANDLOCK_ACCESS_FS_IOCTL_DEV | \
> > > > > > > + LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
> > > > > > > /* clang-format on */
> > > > > > >
> > > > > > > /*
> > > > > > > @@ -1561,6 +1563,103 @@ static int hook_path_truncate(const struct path *const path)
> > > > > > > return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
> > > > > > > }
> > > > > > >
> > > > > > > +/**
> > > > > > > + * unmask_scoped_access - Remove access right bits in @masks in all layers
> > > > > > > + * where @client and @server have the same domain
> > > > > > > + *
> > > > > > > + * This does the same as domain_is_scoped(), but unmasks bits in @masks.
> > > > > > > + * It can not return early as domain_is_scoped() does.
> > > > >
> > > > > Why can't we use the same logic as for other scopes?
> > > >
> > > > The other scopes, for which this is implemented in domain_is_scoped(),
> > > > do not need to do this layer-by-layer.
> > > >
> > > > I have to admit, in my initial implementation, I was using
> > > > domain_is_scoped() directly, and the logic at the end of the hook was
> > > > roughly:
> > > >
> > > > --- BUGGY CODE START ---
> > > > // ...
> > > >
> > > > if (!domain_is_scoped(..., ..., LANDLOCK_ACCESS_FS_RESOLVE_UNIX))
> > > > return 0; /* permitted */
> > > >
> > > > return current_check_access_path(path, LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
> > > > }
> > > > --- BUGGY CODE END ---
> > > >
> > > > Unfortunately, that is a logic error though -- it implements the formula
> > > >
> > > > Access granted if:
> > > > (FOR-ALL l ∈ layers scoped-access-ok(l)) OR (FOR-ALL l ∈ layers path-access-ok(l)) (WRONG!)
> > > >
> > > > but the formula we want is:
> > > >
> > > > Access granted if:
> > > > FOR-ALL l ∈ layers (scoped-access-ok(l) OR path-access-ok(l)) (CORRECT!)
> > >
> > > It is worth it to add this explanation to the unmask_scoped_access()
> > > description, also pointing to the test that check this case.
> > >
> > > >
> > > > This makes a difference in the case where (pseudocode):
> > > >
> > > > 1. landlock_restrict_self(RESOLVE_UNIX) // d1
> > > > 2. create_unix_server("./sock")
> > > > 3. landlock_restrict_self(RESOLVE_UNIX, rule=Allow(".", RESOLVE_UNIX)) // d2
> > > > 4. connect_unix("./sock")
> > > >
> > > > ,------------------------------------------------d1--,
> > > > | |
> > > > | ./sock server |
> > > > | ^ |
> > > > | | |
> > > > | ,------------------------------------------d2--, |
> > > > | | | | |
> > > > | | client | |
> > > > | | | |
> > > > | '----------------------------------------------' |
> > > > | |
> > > > '----------------------------------------------------'
> > > >
> > > > (BTW, this scenario is covered in the selftests, that is why there is
> > > > a variant of these selftests where instead of applying "no domain", we
> > > > apply a domain with an exception rule like in step 3 in the pseudocode
> > > > above. Applying that domain should behave the same as applying no
> > > > domain at all.)
> > > >
> > > > Intuitively, it is clear that the access should be granted:
> > > >
> > > > - d1 does not restrict access to the server,
> > > > because the socket was created within d1 itself.
> > > > - d2 does not restrict access to the server,
> > > > because it has a rule to allow it
> > > >
> > > > But the "buggy code" logic above comes to a different conclusion:
> > > >
> > > > - the domain_is_scoped() check denies the access, because the server
> > > > is in a more privileged domain relative to the client domain.
> > > > - the current_check_access_path() check denies the access as well,
> > > > because the socket's path is not allow-listed in d1.
> > > >
> > > > In the 'intuitive' reasoning above, we are checking d1 and d2
> > > > independently of each other. While Landlock is not implemented like
> > > > that internally, we need to stay consistent with it so that domains
> > > > compose correctly. The way to do that is to track is access check
> > > > results on a per-layer basis again, and that is why
> > > > unmask_scoped_access() uses a layer mask for tracking. The original
> > > > domain_is_scoped() does not use a layer mask, but that also means that
> > > > it can return early in some scenarios -- if for any of the relevant
> > > > layer depths, the client and server domains are not the same, it exits
> > > > early with failure because it's overall not fulfillable any more. In
> > > > the RESOLVE_UNIX case though, we need to remember in which layers we
> > > > failed (both high an low ones), because these layers can still be
> > > > fulfilled with a PATH_BENEATH rule later.
> > > >
> > > > Summary:
> > > >
> > > > Option 1: We *can* unify this if you want. It just might come at a
> > > > small performance penalty for domain_is_scoped(), which now uses the
> > > > larger layer mask data structure and can't do the same early returns
> > > > any more as before.
> > > >
> > > > Option 2: Alternatively, if we move the two functions into the same
> > > > module, we can keep them separate but still test them against each
> > > > other to make sure they are in-line:
> > > >
> > > > This invocation should return true...
> > > >
> > > > domain_is_scoped(cli, srv, access)
> > > >
> > > > ...in the exactly the same situations where this invocation leaves any
> > > > bits set in layer_masks:
> > > >
> > > > landlock_init_layer_masks(dom, access, &layer_masks, LL_KEY_INODE);
> > > > unmask_scoped_access(cli, srv, &layer_masks, access);
> > > >
> > > > What do you prefer?
> > >
> > > I was thinking about factoring out domain_is_scoped() with
> > > unmask_scoped_access() but, after some tests, it is not worth it. Your
> > > approach is simple and good.
> > >
> > > >
> > > >
> > > > > > > + *
> > > > > > > + * @client: Client domain
> > > > > > > + * @server: Server domain
> > > > > > > + * @masks: Layer access masks to unmask
> > > > > > > + * @access: Access bit that controls scoping
> > > > > > > + */
> > > > > > > +static void unmask_scoped_access(const struct landlock_ruleset *const client,
> > > > > > > + const struct landlock_ruleset *const server,
> > > > > > > + struct layer_access_masks *const masks,
> > > > > > > + const access_mask_t access)
> > > > > >
> > > > > > This helper should be moved to task.c and factored out with
> > > > > > domain_is_scoped(). This should be a dedicated patch.
> > > > >
> > > > > Well, if domain_is_scoped() can be refactored and made generic, it would
> > > > > make more sense to move it to domain.c
> > > > >
> > > > > >
> > > > > > > +{
> > > > > > > + int client_layer, server_layer;
> > > > > > > + const struct landlock_hierarchy *client_walker, *server_walker;
> > > > > > > +
> > > > > > > + if (WARN_ON_ONCE(!client))
> > > > > > > + return; /* should not happen */
> > >
> > > Please no comment after ";"
> > >
> > > > > > > +
> > > > > > > + if (!server)
> > > > > > > + return; /* server has no Landlock domain; nothing to clear */
> > > > > > > +
> > > > > > > + client_layer = client->num_layers - 1;
> > > > > > > + client_walker = client->hierarchy;
> > > > > > > + server_layer = server->num_layers - 1;
> > > > > > > + server_walker = server->hierarchy;
> > > > > > > +
> > > > > > > + /*
> > > > > > > + * Clears the access bits at all layers where the client domain is the
> > > > > > > + * same as the server domain. We start the walk at min(client_layer,
> > > > > > > + * server_layer). The layer bits until there can not be cleared because
> > > > > > > + * either the client or the server domain is missing.
> > > > > > > + */
> > > > > > > + for (; client_layer > server_layer; client_layer--)
> > > > > > > + client_walker = client_walker->parent;
> > > > > > > +
> > > > > > > + for (; server_layer > client_layer; server_layer--)
> > > > > > > + server_walker = server_walker->parent;
> > > > > > > +
> > > > > > > + for (; client_layer >= 0; client_layer--) {
> > > > > > > + if (masks->access[client_layer] & access &&
> > > > > > > + client_walker == server_walker)
> > >
> > > I'd prefer to first check client_walker == server_walker and then the
> > > access. My main concern is that only one bit of access matching
> > > masks->access[client_layer] clear all the access request bits. In
> > > practice there is only one, for now, but this code should be more strict
> > > by following a defensive approach.
>
> This function works even if multiple access request bits with
> "scope-like" semantics were being checked in parallel; if you consider
> the logic in:
>
> if (masks->access[client_layer] & access &&
> client_walker == server_walker)
> masks->access[client_layer] &= ~access;
>
> you'll realize that the check for "masks->access[client_layer] &
> access" is technically irrelevant - if that check fails, all the
Correct
> affected bits are already zero, so clearing them is a no-op. This
> code is equivalent, but might perform slightly more writes (although
> it likely does not make a performance difference in practice):
>
> if (client_walker == server_walker)
> masks->access[client_layer] &= ~access;
>
> With that code it's a bit easier to see that "access" is actually only
> used to decide which bits to clear. This works both with one and with
> multiple access rights.
>
> This follows the same logic as outlined in the comment above in the
> code, where it says:
>
> Clears the access bits at all layers where the client domain is the
> same as the server domain. We start the walk at min(client_layer,
> server_layer). The layer bits until there can not be cleared because
> either the client or the server domain is missing.
>
> Clearing bits that aren't there is a no-op
>
>
>
> <Optional Math>
>
> I found it helpful to visualize the scoping logic, this is directly
> from my notes: (Web version is at https://wiki.gnoack.org/LandlockDomainIsScoped)
>
> The domain_is_scoped() helper implements the following predicate:
>
> ∀ l ∈ (0,16): (hasbit(self, l) implies-that domain(self, l) == domain(other, l))
>
> That is, we require for each layer l nesting depth that:
>
> * **If** scoping is active at the layer,
> * **Then** the domains of self and other are the same
> at the given nesting depth.
>
> For example:
>
> [ ]
> |
> [x] self and other have the same domain at this depth
> |
> [ ]
> / \
> [x] [ ] self and other have differing domains at this depth
> | |
> [ ] [ ]
> |
> [ ] "other" "x" marks a domain where "self" has
> set the scoping bit
> "self"
>
> </Optional Math>
>
>
> > > > > > > + masks->access[client_layer] &= ~access;
> >
> > Actually, why not removing the access argument and just reset
> > masks->access[client_layer]? The doc would need some updates.
>
> It would feel brittle to me if this function were to clear out
> unrelated access rights. It receives a struct layer_access_masks after
> all, where it is normally expected that multiple kinds of access
> rights are set. In my understanding, the bit masking does not cost
> much extra performance compared to clearing it out entirely, so I'd
> prefer to have clearer semantics and only operate on the access rights
> that it's about, even when the other bits are all zero at the moment.
>
> (For full disclosure, I have contemplated for a bit whether
> hook_unix_find() should take a layer_mask_t-like type where each bit
> indicates whether a given access right
> (LANDLOCK_ACCESS_FS_RESOLVE_UNIX, in this case) is set at a given
> layer, and then it would only clear out the bits there. That would be
> in some ways simpler, but then the caller would still need to convert
> back and forth to a layer mask anyway, because that's what the other
> functions there take. So it didn't seem like a good option in the
> bigger scheme (and I would also prefer to not re-introduce
> layer_mask_t after we just removed it).)
>
> Maybe I did not understand your remark fully though;
> Does my argument sound reasonable?
Yes! Thanks for the deep explanation.
^ permalink raw reply
* Re: [PATCH v6 1/9] lsm: Add LSM hook security_unix_find
From: Mickaël Salaün @ 2026-03-17 21:14 UTC (permalink / raw)
To: Paul Moore, Sebastian Andrzej Siewior, Kuniyuki Iwashima,
Simon Horman, Jakub Kicinski, netdev
Cc: Günther Noack, John Johansen, James Morris, Serge E . Hallyn,
Tingmao Wang, Justin Suess, linux-security-module,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Alexander Viro, Christian Brauner
In-Reply-To: <20260315222150.121952-2-gnoack3000@gmail.com>
Paul and netdev folks, I think this new hook is now good. I'd like to
push this series to -next this week. Please let us know if you find any
issue, otherwise I guess we'll get more eyes and bots in the -next tree.
On Sun, Mar 15, 2026 at 11:21:42PM +0100, Günther Noack wrote:
> From: Justin Suess <utilityemal77@gmail.com>
>
> Add a LSM hook security_unix_find.
>
> This hook is called to check the path of a named unix socket before a
> connection is initiated. The peer socket may be inspected as well.
>
> Why existing hooks are unsuitable:
>
> Existing socket hooks, security_unix_stream_connect(),
> security_unix_may_send(), and security_socket_connect() don't provide
> TOCTOU-free / namespace independent access to the paths of sockets.
>
> (1) We cannot resolve the path from the struct sockaddr in existing hooks.
> This requires another path lookup. A change in the path between the
> two lookups will cause a TOCTOU bug.
>
> (2) We cannot use the struct path from the listening socket, because it
> may be bound to a path in a different namespace than the caller,
> resulting in a path that cannot be referenced at policy creation time.
>
> Cc: Günther Noack <gnoack3000@gmail.com>
> Cc: Tingmao Wang <m@maowtm.org>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
> include/linux/lsm_hook_defs.h | 5 +++++
> include/linux/security.h | 11 +++++++++++
> net/unix/af_unix.c | 13 ++++++++++---
> security/security.c | 20 ++++++++++++++++++++
> 4 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 8c42b4bde09c..7a0fd3dbfa29 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -317,6 +317,11 @@ LSM_HOOK(int, 0, post_notification, const struct cred *w_cred,
> LSM_HOOK(int, 0, watch_key, struct key *key)
> #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other,
> + int flags)
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
> #ifdef CONFIG_SECURITY_NETWORK
> LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
> struct sock *newsk)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 83a646d72f6f..99a33d8eb28d 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1931,6 +1931,17 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
> }
> #endif /* CONFIG_SECURITY_NETWORK */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +
> +int security_unix_find(const struct path *path, struct sock *other, int flags);
> +
> +#else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
> #ifdef CONFIG_SECURITY_INFINIBAND
> int security_ib_pkey_access(void *sec, u64 subnet_prefix, u16 pkey);
> int security_ib_endport_manage_subnet(void *sec, const char *name, u8 port_num);
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 3756a93dc63a..aced28179bac 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1231,11 +1231,18 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
> goto path_put;
>
> err = -EPROTOTYPE;
> - if (sk->sk_type == type)
> - touch_atime(&path);
> - else
> + if (sk->sk_type != type)
> goto sock_put;
>
> + /*
> + * We call the hook because we know that the inode is a socket and we
> + * hold a valid reference to it via the path.
> + */
> + err = security_unix_find(&path, sk, flags);
> + if (err)
> + goto sock_put;
> + touch_atime(&path);
> +
> path_put(&path);
>
> return sk;
> diff --git a/security/security.c b/security/security.c
> index 67af9228c4e9..c73196b8db4b 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4731,6 +4731,26 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
>
> #endif /* CONFIG_SECURITY_NETWORK */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +/**
> + * security_unix_find() - Check if a named AF_UNIX socket can connect
> + * @path: path of the socket being connected to
> + * @other: peer sock
> + * @flags: flags associated with the socket
> + *
> + * This hook is called to check permissions before connecting to a named
> + * AF_UNIX socket.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_unix_find(const struct path *path, struct sock *other, int flags)
> +{
> + return call_int_hook(unix_find, path, other, flags);
> +}
> +EXPORT_SYMBOL(security_unix_find);
> +
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
> #ifdef CONFIG_SECURITY_INFINIBAND
> /**
> * security_ib_pkey_access() - Check if access to an IB pkey is allowed
> --
> 2.53.0
>
>
^ permalink raw reply
* Re: [PATCH] securityfs: use kstrdup_const() to manage symlink targets
From: Paul Moore @ 2026-03-17 21:13 UTC (permalink / raw)
To: Dmitry Antipov, James Morris, Serge E. Hallyn
Cc: linux-security-module, Dmitry Antipov
In-Reply-To: <20260317141135.133339-1-dmantipov@yandex.ru>
On Mar 17, 2026 Dmitry Antipov <dmantipov@yandex.ru> wrote:
>
> Since 'target' argument of 'securityfs_create_symlink()' is (for
> now at least) a compile-time constant, it may be reasonable to
> use 'kstrdup_const()' / 'kree_const()' to manage 'i_link' member
> of the corresponding inode in attempt to reuse .rodata instance
> rather than making a copy.
>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> security/inode.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
Seems reasonable, we can easily back this out if we ever need to support
non-const link target strings. Merged into lsm/dev, thanks.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 3/3] ima: Add support for staging measurements for deletion
From: Mimi Zohar @ 2026-03-17 21:03 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260311171956.2317781-3-roberto.sassu@huaweicloud.com>
Hi Roberto,
On Wed, 2026-03-11 at 18:19 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduce the ability of staging the IMA measurement list for deletion.
> Staging means moving the current content of the measurement list to a
> separate location, and allowing users to read and delete it. This causes
> the measurement list to be atomically truncated before new measurements can
> be added.
I really like this design of atomically moving and subsequently deleting the
measurement list. However this is a solution, not the motivation for the patch.
Please include the motivation for the patch, before describing the solution.
> Staging can be done only once at a time. In the event of kexec(),
> staging is reverted and staged entries will be carried over to the new
> kernel.
>
> Staged measurements can be deleted entirely, or partially, with the
> non-deleted ones added back to the IMA measurements list.
This patch description is really long, which is an indication that the patch
needs to be split up. Adding support for partially deleting the measurement
list records, by prepending the remaining measurement records, should be a
separate patch.
> This allows the
> remote attestation agents to easily separate the measurements that where
> verified (staged and deleted) from those that weren't due to the race
> between taking a TPM quote and reading the measurements list.
>
> User space is responsible to concatenate the staged IMA measurements list
> portions (excluding the measurements added back to the IMA measurements
> list) following the temporal order in which the operations were done,
> together with the current measurement list. Then, it can send the collected
> data to the remote verifiers.
This belongs in a Documentation patch.
>
> The benefit of staging and deleting is the ability to free precious kernel
> memory,
This is the motivation for the patch.
> in exchange of delegating user space to reconstruct the full
> measurement list from the chunks. No trust needs to be given to user space,
> since the integrity of the measurement list is protected by the TPM.
Agreed the measurement list, itself, is protected by the TPM. However, relying
on userspace to reassemble the chunks is another concern. Support for staging
and deleting the measurement list should be configurable. Defining a Kconfig
should be part of this initial patch.
> By default, staging the measurements list does not alter the hash table.
> When staging and deleting are done, IMA is still able to detect collisions
> on the staged and later deleted measurement entries, by keeping the entry
> digests (only template data are freed).
>
> However, since during the measurements list serialization only the SHA1
> digest is passed, and since there are no template data to recalculate the
> other digests from, the hash table is currently not populated with digests
> from staged/deleted entries after kexec().
>
> Introduce the new kernel option ima_flush_htable to decide whether or not
> the digests of staged measurement entries are flushed from the hash table,
> when they are deleted. Flushing the hash table is supported only when
> deleting all the staged measurements, since in that case the old hash table
> can be quickly swapped with a blank one (otherwise entries would have to be
> removed one by one for partial deletion).
Allowing the hash table to be deleted would be an example of another patch.
>
> Then, introduce ascii_runtime_measurements_<algo>_staged and
> binary_runtime_measurements_<algo>_staged interfaces to stage and delete
> the measurements. Use 'echo A > <IMA interface>' and
> 'echo D > <IMA interface>' to respectively stage and delete the entire
> measurements list. Use 'echo N > <IMA interface>', with N between 1 and
> ULONG_MAX - 1, to delete the selected staged portion of the measurements
> list.
>
> The ima_measure_users counter (protected by the ima_measure_mutex mutex)
> has been introduced to protect access to the measurements list and the
> staged part. The open method of all the measurement interfaces has been
> extended to allow only one writer at a time or, in alternative, multiple
> readers. The write permission is used to stage and delete the measurements,
> the read permission to read them. Write requires also the CAP_SYS_ADMIN
> capability.
Yes, this is part of the initial patch that adds support for staging the
measurement list.
>
> Finally, introduce the binary_lists enum and make binary_runtime_size
> and ima_num_entries as arrays, to keep track of their values for the
> current IMA measurements list (BINARY), current list plus staged
> measurements (BINARY_STAGED) and the cumulative list since IMA
> initialization (BINARY_FULL).
>
> Use BINARY in ima_show_measurements_count(), BINARY_STAGED in
> ima_add_kexec_buffer() and BINARY_FULL in ima_measure_kexec_event().
>
> It should be noted that the BINARY_FULL counter is not passed through
> kexec. Thus, the number of entries included in the kexec critical data
> records refers to the entries since the previous kexec records.
>
> Note: This code derives from the Alt-IMA Huawei project, whose license is
> GPL-2.0 OR MIT.
>
> Link: https://github.com/linux-integrity/linux/issues/1
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
The design looks good. As I mentioned above, this patch description is quite
long, which is an indication that the patch needs to be split up. One method of
breaking it up would be:
- (Basic) support for staging measurements for deletion (based on a Kconfig)
- Support for removing the hash table
- Support for deleting N measurement records (and pre-pending the remaining
measurement records)
- Adding documentation
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v3 1/3] ima: Remove ima_h_table structure
From: Mimi Zohar @ 2026-03-17 19:15 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260311171956.2317781-1-roberto.sassu@huaweicloud.com>
On Wed, 2026-03-11 at 18:19 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> With the upcoming change of dynamically allocating and replacing the hash
> table, we would need to keep the counters for number of measurements
> entries and violations.
>
> Since anyway, those counters don't belong there, remove the ima_h_table
> structure instead and move the counters and the hash table as a separate
> variables.
There's no cover letter or motivation in this patch description for needing to
"dynamically allocating or replacing the existing hash table."
Saying that the htable, number of records in the measurement list, and violation
counter don't belong grouped together is insufficient. There must have been a
valid reason for why they were grouped together originally (e.g. never removed
or reset).
Please provide a motivation for removing the ima_h_table struct and its usage
and defining them independently of each other.
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH RFC 1/4] audit: Implement bpf_audit_log_*() wrappers
From: Mickaël Salaün @ 2026-03-17 19:12 UTC (permalink / raw)
To: David Windsor
Cc: fred, paul, bpf, linux-security-module, audit, ast, daniel,
andrii, martin.lau, kpsingh
In-Reply-To: <20260316221440.2043299-1-dwindsor@gmail.com>
On Mon, Mar 16, 2026 at 06:14:40PM -0400, David Windsor wrote:
> Hi Frederick,
>
> On Wed, Mar 11, 2026 at 04:31:17PM -0500, Frederick Lawler wrote:
> > +__bpf_kfunc int bpf_audit_log_cause(struct bpf_audit_context *ac,
> > + const char *cause__str)
> > +{
> > + if (log_once(ac, BIT_ULL(LSM_AUDIT_DATA_CAUSE)))
> > + return -EINVAL;
> > +
> > + audit_log_format(ac->ab, " cause=");
> > + audit_log_untrustedstring(ac->ab, cause__str);
> > + return 0;
> > +}
>
> Rather than putting everything in the cause field, could we perhaps
> have a separate kfunc here that appends normal stringpairs (not
> format strings) to the audit record:
>
> bpf_audit_log_str(ac, "result", "denied");
> bpf_audit_log_str(ac, "op", "read");
> bpf_audit_log_str(ac, "scontext", ctx_str);
That would mean arbitrary audit keys (and values), which would not be
acceptable (i.e. no consistency).
>
> I know you didn't want to wrap audit_log_format(), which makes sense,
> this would be a midway point between that and stuffing everything in
> one field.
^ 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