public inbox for linux-security-module@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/1] lsm: Add hook unix_path_connect
@ 2025-12-31 21:33 Justin Suess
  2025-12-31 21:33 ` [RFC PATCH 1/1] " Justin Suess
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Justin Suess @ 2025-12-31 21:33 UTC (permalink / raw)
  To: Paul Moore, James Morris, Serge E . Hallyn, Kuniyuki Iwashima
  Cc: Simon Horman, Mickaël Salaün, Günther Noack,
	linux-security-module, Tingmao Wang, netdev, Justin Suess

Hi,

This patch introduces a new LSM hook unix_path_connect.

The idea for this patch and the hook came from Günther Noack, who
is cc'd. Much credit to him for the idea and discussion.

This patch is based on the lsm next branch.

Motivation
---

For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
identifying object from a policy perspective is the path passed to
connect(2). However, this operation currently restricts LSMs that rely
on VFS-based mediation, because the pathname resolved during connect()
is not preserved in a form visible to existing hooks before connection
establishment. As a result, LSMs such as Landlock cannot currently
restrict connections to named UNIX domain sockets by their VFS path.

This gap has been discussed previously (e.g. in the context of Landlock's
path-based access controls). [1] [2]

I've cc'd the netdev folks as well on this, as the placement of this hook is
important and in a core unix socket function.

Design Choices
---

The hook is called in net/unix/af_unix.c in the function unix_find_bsd().

The hook takes a single parameter, a const struct path* to the named unix
socket to which the connection is being established.

The hook takes place after normal permissions checks, and after the
inode is determined to be a socket. It however, takes place before
the socket is actually connected to.

If the hook returns non-zero it will do a put on the path, and return.

References
---

[1]: https://github.com/landlock-lsm/linux/issues/36#issue-2354007438
[2]: https://lore.kernel.org/linux-security-module/cover.1767115163.git.m@maowtm.org/

Kind Regards,
Justin Suess

Justin Suess (1):
  lsm: Add hook unix_path_connect

 include/linux/lsm_hook_defs.h |  1 +
 include/linux/security.h      |  6 ++++++
 net/unix/af_unix.c            |  8 ++++++++
 security/security.c           | 16 ++++++++++++++++
 4 files changed, 31 insertions(+)


base-commit: 1c0860d4415d52f3ad1c8e0a15c1272869278a06
-- 
2.51.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [RFC PATCH 1/1] lsm: Add hook unix_path_connect
  2025-12-31 21:33 [RFC PATCH 0/1] lsm: Add hook unix_path_connect Justin Suess
@ 2025-12-31 21:33 ` Justin Suess
  2026-01-01 12:13   ` Günther Noack
  2026-01-01  9:46 ` [syzbot ci] " syzbot ci
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Justin Suess @ 2025-12-31 21:33 UTC (permalink / raw)
  To: Paul Moore, James Morris, Serge E . Hallyn, Kuniyuki Iwashima
  Cc: Simon Horman, Mickaël Salaün, Günther Noack,
	linux-security-module, Tingmao Wang, netdev, Justin Suess,
	Günther Noack

Adds an LSM hook unix_path_connect.

This hook is called to check the path of a named unix socket before a
connection is initiated.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
Cc: Günther Noack <gnoack3000@gmail.com>
---
 include/linux/lsm_hook_defs.h |  1 +
 include/linux/security.h      |  6 ++++++
 net/unix/af_unix.c            |  8 ++++++++
 security/security.c           | 16 ++++++++++++++++
 4 files changed, 31 insertions(+)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 8c42b4bde09c..a42d1aaf3b8a 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -318,6 +318,7 @@ LSM_HOOK(int, 0, watch_key, struct key *key)
 #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
 
 #ifdef CONFIG_SECURITY_NETWORK
+LSM_HOOK(int, 0, unix_path_connect, const struct path *path)
 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)
diff --git a/include/linux/security.h b/include/linux/security.h
index 83a646d72f6f..ab66f22f7e5a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1638,6 +1638,7 @@ static inline int security_watch_key(struct key *key)
 
 #ifdef CONFIG_SECURITY_NETWORK
 
+int security_unix_path_connect(const struct path *path);
 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);
@@ -1699,6 +1700,11 @@ static inline int security_netlink_send(struct sock *sk, struct sk_buff *skb)
 	return 0;
 }
 
+static inline int security_unix_path_connect(const struct path *path)
+{
+	return 0;
+}
+
 static inline int security_unix_stream_connect(struct sock *sock,
 					       struct sock *other,
 					       struct sock *newsk)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 55cdebfa0da0..af1a6083a69b 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1226,6 +1226,14 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
 	if (!S_ISSOCK(inode->i_mode))
 		goto path_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_path_connect(&path);
+	if (err)
+		goto path_put;
+
 	sk = unix_find_socket_byinode(inode);
 	if (!sk)
 		goto path_put;
diff --git a/security/security.c b/security/security.c
index 31a688650601..17af5d0ddf28 100644
--- a/security/security.c
+++ b/security/security.c
@@ -4047,6 +4047,22 @@ int security_unix_stream_connect(struct sock *sock, struct sock *other,
 }
 EXPORT_SYMBOL(security_unix_stream_connect);
 
+/*
+ * security_unix_path_connect() - Check if a named AF_UNIX socket can connect
+ * @path: Path of the socket being connected to
+ *
+ * This hook is called to check permissions before connecting to a named
+ * AF_UNIX socket. This is necessary because it was not possible to check the
+ * VFS inode of the target socket before the connection is made.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_unix_path_connect(const struct path *path)
+{
+	return call_int_hook(unix_path_connect, path);
+}
+EXPORT_SYMBOL(security_unix_path_connect);
+
 /**
  * security_unix_may_send() - Check if AF_UNIX socket can send datagrams
  * @sock: originating sock
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [syzbot ci] Re: lsm: Add hook unix_path_connect
  2025-12-31 21:33 [RFC PATCH 0/1] lsm: Add hook unix_path_connect Justin Suess
  2025-12-31 21:33 ` [RFC PATCH 1/1] " Justin Suess
@ 2026-01-01  9:46 ` syzbot ci
  2026-01-01 11:56 ` [RFC PATCH 0/1] " Günther Noack
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 20+ messages in thread
From: syzbot ci @ 2026-01-01  9:46 UTC (permalink / raw)
  To: gnoack3000, gnoack, horms, jmorris, kuniyu, linux-security-module,
	m, mic, netdev, paul, serge, utilityemal77
  Cc: syzbot, syzkaller-bugs

syzbot ci has tested the following series

[v1] lsm: Add hook unix_path_connect
https://lore.kernel.org/all/20251231213314.2979118-1-utilityemal77@gmail.com
* [RFC PATCH 1/1] lsm: Add hook unix_path_connect

and found the following issues:
* KASAN: null-ptr-deref Read in unix_dgram_connect
* general protection fault in apparmor_socket_sock_rcv_skb
* general protection fault in unix_stream_connect

Full report is available here:
https://ci.syzbot.org/series/c288b2d0-af95-47d8-b359-79ff653da27b

***

KASAN: null-ptr-deref Read in unix_dgram_connect

tree:      net-next
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net-next.git
base:      dbf8fe85a16a33d6b6bd01f2bc606fc017771465
arch:      amd64
compiler:  Debian clang version 21.1.8 (++20251202083448+f68f64eb8130-1~exp1~20251202083504.46), Debian LLD 21.1.8
config:    https://ci.syzbot.org/builds/9f532f02-856f-4157-a897-26c37e30c537/config
C repro:   https://ci.syzbot.org/findings/d6706e7e-97bc-45af-910c-fb20c328ac02/c_repro
syz repro: https://ci.syzbot.org/findings/d6706e7e-97bc-45af-910c-fb20c328ac02/syz_repro

==================================================================
BUG: KASAN: null-ptr-deref in instrument_atomic_read include/linux/instrumented.h:68 [inline]
BUG: KASAN: null-ptr-deref in _test_bit include/asm-generic/bitops/instrumented-non-atomic.h:141 [inline]
BUG: KASAN: null-ptr-deref in sock_flag include/net/sock.h:1048 [inline]
BUG: KASAN: null-ptr-deref in unix_dgram_connect+0x356/0xc20 net/unix/af_unix.c:1550
Read of size 8 at addr 0000000000000060 by task syz.0.17/5977

CPU: 0 UID: 0 PID: 5977 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Call Trace:
 <TASK>
 dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
 kasan_report+0x117/0x150 mm/kasan/report.c:595
 check_region_inline mm/kasan/generic.c:-1 [inline]
 kasan_check_range+0x264/0x2c0 mm/kasan/generic.c:200
 instrument_atomic_read include/linux/instrumented.h:68 [inline]
 _test_bit include/asm-generic/bitops/instrumented-non-atomic.h:141 [inline]
 sock_flag include/net/sock.h:1048 [inline]
 unix_dgram_connect+0x356/0xc20 net/unix/af_unix.c:1550
 __sys_connect_file net/socket.c:2089 [inline]
 __sys_connect+0x312/0x450 net/socket.c:2108
 __do_sys_connect net/socket.c:2114 [inline]
 __se_sys_connect net/socket.c:2111 [inline]
 __x64_sys_connect+0x7a/0x90 net/socket.c:2111
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xe2/0xf80 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f78e4f9acb9
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffc8b576858 EFLAGS: 00000246 ORIG_RAX: 000000000000002a
RAX: ffffffffffffffda RBX: 00007f78e5205fa0 RCX: 00007f78e4f9acb9
RDX: 000000000000006e RSI: 0000200000000280 RDI: 0000000000000005
RBP: 00007f78e5008bf7 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f78e5205fac R14: 00007f78e5205fa0 R15: 00007f78e5205fa0
 </TASK>
==================================================================


***

general protection fault in apparmor_socket_sock_rcv_skb

tree:      net-next
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net-next.git
base:      dbf8fe85a16a33d6b6bd01f2bc606fc017771465
arch:      amd64
compiler:  Debian clang version 21.1.8 (++20251202083448+f68f64eb8130-1~exp1~20251202083504.46), Debian LLD 21.1.8
config:    https://ci.syzbot.org/builds/9f532f02-856f-4157-a897-26c37e30c537/config
C repro:   https://ci.syzbot.org/findings/7e600b6d-75f6-48a2-a2a0-c69a3aadaa9b/c_repro
syz repro: https://ci.syzbot.org/findings/7e600b6d-75f6-48a2-a2a0-c69a3aadaa9b/syz_repro

Oops: general protection fault, probably for non-canonical address 0xdffffc0000000096: 0000 [#1] SMP KASAN PTI
KASAN: null-ptr-deref in range [0x00000000000004b0-0x00000000000004b7]
CPU: 0 UID: 0 PID: 5987 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:aa_sock security/apparmor/include/net.h:57 [inline]
RIP: 0010:apparmor_socket_sock_rcv_skb+0x3a/0x350 security/apparmor/lsm.c:1513
Code: ec 10 49 89 f7 48 89 fb 49 bd 00 00 00 00 00 fc ff df e8 99 57 6f fd 48 89 5c 24 08 48 81 c3 b0 04 00 00 48 89 d8 48 c1 e8 03 <42> 80 3c 28 00 74 08 48 89 df e8 17 54 d6 fd 44 8b 25 20 70 74 09
RSP: 0018:ffffc90004237530 EFLAGS: 00010206
RAX: 0000000000000096 RBX: 00000000000004b0 RCX: ffff88816872d7c0
RDX: 0000000000000000 RSI: ffff888110f4e600 RDI: 0000000000000000
RBP: ffffc900042376d0 R08: ffffffff82447acc R09: ffffffff8e341b20
R10: dffffc0000000000 R11: ffffed102388124c R12: ffff888110f4e67e
R13: dffffc0000000000 R14: 0000000000000000 R15: ffff888110f4e600
FS:  00007f36295926c0(0000) GS:ffff88818e40e000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f3629591ff8 CR3: 0000000113938000 CR4: 00000000000006f0
Call Trace:
 <TASK>
 security_sock_rcv_skb+0x8f/0x270 security/security.c:4333
 sk_filter_trim_cap+0x19b/0xd90 net/core/filter.c:156
 sk_filter include/linux/filter.h:1102 [inline]
 unix_dgram_sendmsg+0x7bc/0x17b0 net/unix/af_unix.c:2173
 sock_sendmsg_nosec net/socket.c:727 [inline]
 __sock_sendmsg+0x21c/0x270 net/socket.c:742
 ____sys_sendmsg+0x500/0x810 net/socket.c:2592
 ___sys_sendmsg+0x2a5/0x360 net/socket.c:2646
 __sys_sendmmsg+0x27c/0x4e0 net/socket.c:2735
 __do_sys_sendmmsg net/socket.c:2762 [inline]
 __se_sys_sendmmsg net/socket.c:2759 [inline]
 __x64_sys_sendmmsg+0xa0/0xc0 net/socket.c:2759
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xe2/0xf80 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f362879acb9
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f3629592028 EFLAGS: 00000246 ORIG_RAX: 0000000000000133
RAX: ffffffffffffffda RBX: 00007f3628a06090 RCX: 00007f362879acb9
RDX: 0000000000000002 RSI: 0000200000000ec0 RDI: 0000000000000006
RBP: 00007f3628808bf7 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f3628a06128 R14: 00007f3628a06090 R15: 00007ffcf5635f88
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:aa_sock security/apparmor/include/net.h:57 [inline]
RIP: 0010:apparmor_socket_sock_rcv_skb+0x3a/0x350 security/apparmor/lsm.c:1513
Code: ec 10 49 89 f7 48 89 fb 49 bd 00 00 00 00 00 fc ff df e8 99 57 6f fd 48 89 5c 24 08 48 81 c3 b0 04 00 00 48 89 d8 48 c1 e8 03 <42> 80 3c 28 00 74 08 48 89 df e8 17 54 d6 fd 44 8b 25 20 70 74 09
RSP: 0018:ffffc90004237530 EFLAGS: 00010206
RAX: 0000000000000096 RBX: 00000000000004b0 RCX: ffff88816872d7c0
RDX: 0000000000000000 RSI: ffff888110f4e600 RDI: 0000000000000000
RBP: ffffc900042376d0 R08: ffffffff82447acc R09: ffffffff8e341b20
R10: dffffc0000000000 R11: ffffed102388124c R12: ffff888110f4e67e
R13: dffffc0000000000 R14: 0000000000000000 R15: ffff888110f4e600
FS:  00007f36295926c0(0000) GS:ffff88818e40e000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f3629591ff8 CR3: 0000000113938000 CR4: 00000000000006f0
----------------
Code disassembly (best guess), 2 bytes skipped:
   0:	49 89 f7             	mov    %rsi,%r15
   3:	48 89 fb             	mov    %rdi,%rbx
   6:	49 bd 00 00 00 00 00 	movabs $0xdffffc0000000000,%r13
   d:	fc ff df
  10:	e8 99 57 6f fd       	call   0xfd6f57ae
  15:	48 89 5c 24 08       	mov    %rbx,0x8(%rsp)
  1a:	48 81 c3 b0 04 00 00 	add    $0x4b0,%rbx
  21:	48 89 d8             	mov    %rbx,%rax
  24:	48 c1 e8 03          	shr    $0x3,%rax
* 28:	42 80 3c 28 00       	cmpb   $0x0,(%rax,%r13,1) <-- trapping instruction
  2d:	74 08                	je     0x37
  2f:	48 89 df             	mov    %rbx,%rdi
  32:	e8 17 54 d6 fd       	call   0xfdd6544e
  37:	44 8b 25 20 70 74 09 	mov    0x9747020(%rip),%r12d        # 0x974705e


***

general protection fault in unix_stream_connect

tree:      net-next
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net-next.git
base:      dbf8fe85a16a33d6b6bd01f2bc606fc017771465
arch:      amd64
compiler:  Debian clang version 21.1.8 (++20251202083448+f68f64eb8130-1~exp1~20251202083504.46), Debian LLD 21.1.8
config:    https://ci.syzbot.org/builds/9f532f02-856f-4157-a897-26c37e30c537/config
C repro:   https://ci.syzbot.org/findings/b05720e7-cc44-4796-b729-e2f0c0b9e015/c_repro
syz repro: https://ci.syzbot.org/findings/b05720e7-cc44-4796-b729-e2f0c0b9e015/syz_repro

Oops: general protection fault, probably for non-canonical address 0xdffffc00000000dc: 0000 [#1] SMP KASAN PTI
KASAN: null-ptr-deref in range [0x00000000000006e0-0x00000000000006e7]
CPU: 1 UID: 0 PID: 6000 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:kasan_byte_accessible+0x12/0x30 mm/kasan/generic.c:210
Code: 79 ff ff ff 0f 1f 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 40 d6 48 c1 ef 03 48 b8 00 00 00 00 00 fc ff df <0f> b6 04 07 3c 08 0f 92 c0 e9 40 68 4e 09 cc 66 66 66 66 66 66 2e
RSP: 0018:ffffc90003f47c00 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffffffff8b7776ee RCX: 0000000080000002
RDX: 0000000000000000 RSI: ffffffff8b7776ee RDI: 00000000000000dc
RBP: ffffffff8a18a569 R08: 0000000000000001 R09: 0000000000000000
R10: dffffc0000000000 R11: ffffed1037900903 R12: 0000000000000000
R13: 00000000000006e0 R14: 00000000000006e0 R15: 0000000000000001
FS:  00007fe5e870a6c0(0000) GS:ffff8882a9a0e000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fe5e77e8400 CR3: 0000000114c7e000 CR4: 00000000000006f0
Call Trace:
 <TASK>
 __kasan_check_byte+0x12/0x40 mm/kasan/common.c:573
 kasan_check_byte include/linux/kasan.h:402 [inline]
 lock_acquire+0x84/0x330 kernel/locking/lockdep.c:5842
 __raw_spin_lock include/linux/spinlock_api_smp.h:133 [inline]
 _raw_spin_lock+0x2e/0x40 kernel/locking/spinlock.c:154
 spin_lock include/linux/spinlock.h:351 [inline]
 unix_stream_connect+0x469/0x1010 net/unix/af_unix.c:1692
 __sys_connect_file net/socket.c:2089 [inline]
 __sys_connect+0x312/0x450 net/socket.c:2108
 __do_sys_connect net/socket.c:2114 [inline]
 __se_sys_connect net/socket.c:2111 [inline]
 __x64_sys_connect+0x7a/0x90 net/socket.c:2111
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xe2/0xf80 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fe5e779acb9
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fe5e870a028 EFLAGS: 00000246 ORIG_RAX: 000000000000002a
RAX: ffffffffffffffda RBX: 00007fe5e7a05fa0 RCX: 00007fe5e779acb9
RDX: 000000000000006e RSI: 0000200000000000 RDI: 0000000000000005
RBP: 00007fe5e7808bf7 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fe5e7a06038 R14: 00007fe5e7a05fa0 R15: 00007ffcf198af58
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:kasan_byte_accessible+0x12/0x30 mm/kasan/generic.c:210
Code: 79 ff ff ff 0f 1f 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 40 d6 48 c1 ef 03 48 b8 00 00 00 00 00 fc ff df <0f> b6 04 07 3c 08 0f 92 c0 e9 40 68 4e 09 cc 66 66 66 66 66 66 2e
RSP: 0018:ffffc90003f47c00 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffffffff8b7776ee RCX: 0000000080000002
RDX: 0000000000000000 RSI: ffffffff8b7776ee RDI: 00000000000000dc
RBP: ffffffff8a18a569 R08: 0000000000000001 R09: 0000000000000000
R10: dffffc0000000000 R11: ffffed1037900903 R12: 0000000000000000
R13: 00000000000006e0 R14: 00000000000006e0 R15: 0000000000000001
FS:  00007fe5e870a6c0(0000) GS:ffff8882a9a0e000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fe5e77e8400 CR3: 0000000114c7e000 CR4: 00000000000006f0
----------------
Code disassembly (best guess), 4 bytes skipped:
   0:	0f 1f 40 00          	nopl   0x0(%rax)
   4:	90                   	nop
   5:	90                   	nop
   6:	90                   	nop
   7:	90                   	nop
   8:	90                   	nop
   9:	90                   	nop
   a:	90                   	nop
   b:	90                   	nop
   c:	90                   	nop
   d:	90                   	nop
   e:	90                   	nop
   f:	90                   	nop
  10:	90                   	nop
  11:	90                   	nop
  12:	90                   	nop
  13:	90                   	nop
  14:	0f 1f 40 d6          	nopl   -0x2a(%rax)
  18:	48 c1 ef 03          	shr    $0x3,%rdi
  1c:	48 b8 00 00 00 00 00 	movabs $0xdffffc0000000000,%rax
  23:	fc ff df
* 26:	0f b6 04 07          	movzbl (%rdi,%rax,1),%eax <-- trapping instruction
  2a:	3c 08                	cmp    $0x8,%al
  2c:	0f 92 c0             	setb   %al
  2f:	e9 40 68 4e 09       	jmp    0x94e6874
  34:	cc                   	int3
  35:	66                   	data16
  36:	66                   	data16
  37:	66                   	data16
  38:	66                   	data16
  39:	66                   	data16
  3a:	66                   	data16
  3b:	2e                   	cs


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2025-12-31 21:33 [RFC PATCH 0/1] lsm: Add hook unix_path_connect Justin Suess
  2025-12-31 21:33 ` [RFC PATCH 1/1] " Justin Suess
  2026-01-01  9:46 ` [syzbot ci] " syzbot ci
@ 2026-01-01 11:56 ` Günther Noack
  2026-01-05  7:46 ` Kuniyuki Iwashima
  2026-01-07 21:54 ` Paul Moore
  4 siblings, 0 replies; 20+ messages in thread
From: Günther Noack @ 2026-01-01 11:56 UTC (permalink / raw)
  To: Justin Suess, Paul Moore
  Cc: James Morris, Serge E . Hallyn, Kuniyuki Iwashima, Simon Horman,
	Mickaël Salaün, Günther Noack,
	linux-security-module, Tingmao Wang, netdev

Thank you for sending this, Justin!

Paul: Could you please have a look at this new LSM hook? -- it extends
the LSM interface and it is an approach that I have suggested in [1].

On Wed, Dec 31, 2025 at 04:33:13PM -0500, Justin Suess wrote:
> Hi,
> 
> This patch introduces a new LSM hook unix_path_connect.
> 
> The idea for this patch and the hook came from Günther Noack, who
> is cc'd. Much credit to him for the idea and discussion.
> 
> This patch is based on the lsm next branch.
> 
> Motivation
> ---
> 
> For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> identifying object from a policy perspective is the path passed to
> connect(2). However, this operation currently restricts LSMs that rely
> on VFS-based mediation, because the pathname resolved during connect()
> is not preserved in a form visible to existing hooks before connection
> establishment. As a result, LSMs such as Landlock cannot currently
> restrict connections to named UNIX domain sockets by their VFS path.
> 
> This gap has been discussed previously (e.g. in the context of Landlock's
> path-based access controls). [1] [2]

+1

The use case here is that Landlock should be able to restrict
connect() to named Unix sockets and control this based on the natural
identifier for named Unix sockets -- the path of the socket file.

This feature is a useful and necessary addition to Landlock.  The
discussion that Tingmao Wang linked to on her patch also shows that
users are caught by surprise when they find that connecting to UNIX
sockets is not restrictable [2].  Her patch set [3] lists some ways in
which this can be a problem.


I understand that adding LSM hooks might be controversial, but I think
that the alternatives to the new LSM hook are both worse:

* The patch set from Tingmao Wang at [3] is not restricting Unix
  sockets based on path, but on Landlock policy scope (domain).  This
  is useful, but only complementary to a path-based restriction.  If
  be build both features at some point, they'd potentially have
  surprising interactions that make the UAPI more confusing.  (I've
  written more about this at [4])

* We can not use the existing security_socket_connect() hook for this,
  because the resolved struct path has already been discarded by the
  time when security_socket_connect() is called, and looking up the
  struct path again would create a TOCTOU race condition.
    
  The hook is called from the function unix_find_bsd() in the AF_UNIX
  implementation, which looks up the struct path and keeps it
  transiently in order to find the associated listening-side struct
  sock.


Please let us know what you think!

Thanks!
–Günther


[1] https://github.com/landlock-lsm/linux/issues/36#issuecomment-3669080619
[2] https://spectrum-os.org/lists/archives/spectrum-devel/00256266-26db-40cf-8f5b-f7c7064084c2@gmail.com/
[3] https://lore.kernel.org/all/cover.1767115163.git.m@maowtm.org/
[4] https://lore.kernel.org/all/20251230.bcae69888454@gnoack.org/


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 1/1] lsm: Add hook unix_path_connect
  2025-12-31 21:33 ` [RFC PATCH 1/1] " Justin Suess
@ 2026-01-01 12:13   ` Günther Noack
  2026-01-01 19:45     ` [RFC PATCH 0/1] " Justin Suess
  0 siblings, 1 reply; 20+ messages in thread
From: Günther Noack @ 2026-01-01 12:13 UTC (permalink / raw)
  To: Justin Suess
  Cc: Paul Moore, James Morris, Serge E . Hallyn, Kuniyuki Iwashima,
	Simon Horman, Mickaël Salaün, Günther Noack,
	linux-security-module, Tingmao Wang, netdev

On Wed, Dec 31, 2025 at 04:33:14PM -0500, Justin Suess wrote:
> Adds an LSM hook unix_path_connect.
> 
> This hook is called to check the path of a named unix socket before a
> connection is initiated.
> 
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> Cc: Günther Noack <gnoack3000@gmail.com>
> ---
>  include/linux/lsm_hook_defs.h |  1 +
>  include/linux/security.h      |  6 ++++++
>  net/unix/af_unix.c            |  8 ++++++++
>  security/security.c           | 16 ++++++++++++++++
>  4 files changed, 31 insertions(+)
> 
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 8c42b4bde09c..a42d1aaf3b8a 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -318,6 +318,7 @@ LSM_HOOK(int, 0, watch_key, struct key *key)
>  #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
>  
>  #ifdef CONFIG_SECURITY_NETWORK
> +LSM_HOOK(int, 0, unix_path_connect, const struct path *path)

You are placing this guarded by CONFIG_SECURITY_NETWORK, but there is
also CONFIG_SECURITY_PATH.  Should it be guarded by both?


>  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)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 83a646d72f6f..ab66f22f7e5a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1638,6 +1638,7 @@ static inline int security_watch_key(struct key *key)
>  
>  #ifdef CONFIG_SECURITY_NETWORK
>  
> +int security_unix_path_connect(const struct path *path);
>  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);
> @@ -1699,6 +1700,11 @@ static inline int security_netlink_send(struct sock *sk, struct sk_buff *skb)
>  	return 0;
>  }
>  
> +static inline int security_unix_path_connect(const struct path *path)
> +{
> +	return 0;
> +}
> +
>  static inline int security_unix_stream_connect(struct sock *sock,
>  					       struct sock *other,
>  					       struct sock *newsk)
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 55cdebfa0da0..af1a6083a69b 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1226,6 +1226,14 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
>  	if (!S_ISSOCK(inode->i_mode))
>  		goto path_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_path_connect(&path);
> +	if (err)
> +		goto path_put;

In this place, the hook call is done also for the coredump socket.

The coredump socket is a system-wide setting, and it feels weird to me
that unprivileged processes should be able to inhibit that connection?


> +
>  	sk = unix_find_socket_byinode(inode);
>  	if (!sk)
>  		goto path_put;
> diff --git a/security/security.c b/security/security.c
> index 31a688650601..17af5d0ddf28 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4047,6 +4047,22 @@ int security_unix_stream_connect(struct sock *sock, struct sock *other,
>  }
>  EXPORT_SYMBOL(security_unix_stream_connect);
>  
> +/*
> + * security_unix_path_connect() - Check if a named AF_UNIX socket can connect
> + * @path: Path of the socket being connected to
             ^
mega-nit: lowercase for consistency

> + *
> + * This hook is called to check permissions before connecting to a named
> + * AF_UNIX socket. This is necessary because it was not possible to check the
> + * VFS inode of the target socket before the connection is made.

I'd drop the last sentence; the defense why this is necessary can go
in the commit message, and once we have a call-site for the hook,
someone browsing the kernel code can look up what it is used for.

> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_unix_path_connect(const struct path *path)
> +{
> +	return call_int_hook(unix_path_connect, path);
> +}
> +EXPORT_SYMBOL(security_unix_path_connect);
> +
>  /**
>   * security_unix_may_send() - Check if AF_UNIX socket can send datagrams
>   * @sock: originating sock
> -- 
> 2.51.0
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2026-01-01 12:13   ` Günther Noack
@ 2026-01-01 19:45     ` Justin Suess
  2026-01-01 23:11       ` Tingmao Wang
  2026-01-07 21:43       ` Paul Moore
  0 siblings, 2 replies; 20+ messages in thread
From: Justin Suess @ 2026-01-01 19:45 UTC (permalink / raw)
  To: gnoack3000
  Cc: gnoack, horms, jmorris, kuniyu, linux-security-module, m, mic,
	netdev, paul, serge, utilityemal77

On 1/1/26 07:13, Günther Noack wrote:
> On Wed, Dec 31, 2025 at 04:33:14PM -0500, Justin Suess wrote:
>> Adds an LSM hook unix_path_connect.
>>
>> This hook is called to check the path of a named unix socket before a
>> connection is initiated.
>>
>> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
>> Cc: Günther Noack <gnoack3000@gmail.com>
>> ---
>>  include/linux/lsm_hook_defs.h |  1 +
>>  include/linux/security.h      |  6 ++++++
>>  net/unix/af_unix.c            |  8 ++++++++
>>  security/security.c           | 16 ++++++++++++++++
>>  4 files changed, 31 insertions(+)
>>
>> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
>> index 8c42b4bde09c..a42d1aaf3b8a 100644
>> --- a/include/linux/lsm_hook_defs.h
>> +++ b/include/linux/lsm_hook_defs.h
>> @@ -318,6 +318,7 @@ LSM_HOOK(int, 0, watch_key, struct key *key)
>>  #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
>>  
>>  #ifdef CONFIG_SECURITY_NETWORK
>> +LSM_HOOK(int, 0, unix_path_connect, const struct path *path)
>
> You are placing this guarded by CONFIG_SECURITY_NETWORK, but there is
> also CONFIG_SECURITY_PATH.  Should it be guarded by both?

Agreed. I've moved it to a separate #if block with both
CONFIG_SECURITY_NETWORK and CONFIG_SECURITY_PATH for this and the other
places it was needed.

>
>
>
>>  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)
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 83a646d72f6f..ab66f22f7e5a 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -1638,6 +1638,7 @@ static inline int security_watch_key(struct key *key)
>>  
>>  #ifdef CONFIG_SECURITY_NETWORK
>>  
>> +int security_unix_path_connect(const struct path *path);
>>  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);
>> @@ -1699,6 +1700,11 @@ static inline int security_netlink_send(struct sock *sk, struct sk_buff *skb)
>>  	return 0;
>>  }
>>  
>> +static inline int security_unix_path_connect(const struct path *path)
>> +{
>> +	return 0;
>> +}
>> +
>>  static inline int security_unix_stream_connect(struct sock *sock,
>>  					       struct sock *other,
>>  					       struct sock *newsk)
>> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
>> index 55cdebfa0da0..af1a6083a69b 100644
>> --- a/net/unix/af_unix.c
>> +++ b/net/unix/af_unix.c
>> @@ -1226,6 +1226,14 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
>>  	if (!S_ISSOCK(inode->i_mode))
>>  		goto path_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_path_connect(&path);
>> +	if (err)
>> +		goto path_put;
>
> In this place, the hook call is done also for the coredump socket.
>
> The coredump socket is a system-wide setting, and it feels weird to me
> that unprivileged processes should be able to inhibit that connection?

No I don't think they should be able to. Does this look better?

It also fixes overwriting the the error code when the hook returns.

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 55cdebfa0da0..397687e2d87f 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1226,6 +1226,18 @@ static struct sock *unix_find_bsd(struct
sockaddr_un *sunaddr, int addr_len,
        if (!S_ISSOCK(inode->i_mode))
                goto path_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.
+        * We intentionally forgo the ability to restrict SOCK_COREDUMP.
+        */
+       if (!(flags & SOCK_COREDUMP)) {
+               err = security_unix_path_connect(&path);
+               if (err)
+                       goto path_put;
+               err = -ECONNREFUSED;
+       }
+
        sk = unix_find_socket_byinode(inode);
        if (!sk)
                goto path_put;

>
>
>> +
>>  	sk = unix_find_socket_byinode(inode);
>>  	if (!sk)
>>  		goto path_put;
>> diff --git a/security/security.c b/security/security.c
>> index 31a688650601..17af5d0ddf28 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -4047,6 +4047,22 @@ int security_unix_stream_connect(struct sock *sock, struct sock *other,
>>  }
>>  EXPORT_SYMBOL(security_unix_stream_connect);
>>  
>> +/*
>> + * security_unix_path_connect() - Check if a named AF_UNIX socket can connect
>> + * @path: Path of the socket being connected to
>              ^
> mega-nit: lowercase for consistency
Gotcha.
>
>
>> + *
>> + * This hook is called to check permissions before connecting to a named
>> + * AF_UNIX socket. This is necessary because it was not possible to check the
>> + * VFS inode of the target socket before the connection is made.
>
> I'd drop the last sentence; the defense why this is necessary can go
> in the commit message, and once we have a call-site for the hook,
> someone browsing the kernel code can look up what it is used for.
Sounds good to me.
>
>
>> + *
>> + * Return: Returns 0 if permission is granted.
>> + */
>> +int security_unix_path_connect(const struct path *path)
>> +{
>> +	return call_int_hook(unix_path_connect, path);
>> +}
>> +EXPORT_SYMBOL(security_unix_path_connect);
>> +
>>  /**
>>   * security_unix_may_send() - Check if AF_UNIX socket can send datagrams
>>   * @sock: originating sock
>> -- 
>> 2.51.0
>>


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2026-01-01 19:45     ` [RFC PATCH 0/1] " Justin Suess
@ 2026-01-01 23:11       ` Tingmao Wang
  2026-01-01 23:40         ` Justin Suess
  2026-01-07 21:43       ` Paul Moore
  1 sibling, 1 reply; 20+ messages in thread
From: Tingmao Wang @ 2026-01-01 23:11 UTC (permalink / raw)
  To: Justin Suess
  Cc: gnoack3000, gnoack, horms, jmorris, kuniyu, linux-security-module,
	mic, netdev, paul, serge

On 1/1/26 19:45, Justin Suess wrote:
> [...]
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 55cdebfa0da0..397687e2d87f 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1226,6 +1226,18 @@ static struct sock *unix_find_bsd(struct
> sockaddr_un *sunaddr, int addr_len,
>         if (!S_ISSOCK(inode->i_mode))
>                 goto path_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.
> +        * We intentionally forgo the ability to restrict SOCK_COREDUMP.
> +        */
> +       if (!(flags & SOCK_COREDUMP)) {
> +               err = security_unix_path_connect(&path);
> +               if (err)
> +                       goto path_put;
> +               err = -ECONNREFUSED;

I'm not sure if this is a good suggestion, but I think it might be cleaner
to move this `err = -ECONNREFUSED;` out of the if, and do it
unconditionally above the `sk = unix_find_socket_byinode(inode);` below?
To me that makes the intention for resetting err clear (it is to ensure
that a NULL return from unix_find_socket_byinode causes us to return
-ECONNREFUSED).


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2026-01-01 23:11       ` Tingmao Wang
@ 2026-01-01 23:40         ` Justin Suess
  0 siblings, 0 replies; 20+ messages in thread
From: Justin Suess @ 2026-01-01 23:40 UTC (permalink / raw)
  To: Tingmao Wang
  Cc: gnoack3000, gnoack, horms, jmorris, kuniyu, linux-security-module,
	mic, netdev, paul, serge

On 1/1/26 18:11, Tingmao Wang wrote:
> On 1/1/26 19:45, Justin Suess wrote:
>> [...]
>> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
>> index 55cdebfa0da0..397687e2d87f 100644
>> --- a/net/unix/af_unix.c
>> +++ b/net/unix/af_unix.c
>> @@ -1226,6 +1226,18 @@ static struct sock *unix_find_bsd(struct
>> sockaddr_un *sunaddr, int addr_len,
>>         if (!S_ISSOCK(inode->i_mode))
>>                 goto path_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.
>> +        * We intentionally forgo the ability to restrict SOCK_COREDUMP.
>> +        */
>> +       if (!(flags & SOCK_COREDUMP)) {
>> +               err = security_unix_path_connect(&path);
>> +               if (err)
>> +                       goto path_put;
>> +               err = -ECONNREFUSED;
> I'm not sure if this is a good suggestion, but I think it might be cleaner
> to move this `err = -ECONNREFUSED;` out of the if, and do it
> unconditionally above the `sk = unix_find_socket_byinode(inode);` below?
> To me that makes the intention for resetting err clear (it is to ensure
> that a NULL return from unix_find_socket_byinode causes us to return
> -ECONNREFUSED).
>
I'll do that. That does make it more clear.

I suspect resetting the error accidentally was what caused the syzbot to rightfully complain.

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 55cdebfa0da0..2e0300121ab5 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1226,6 +1226,18 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
     if (!S_ISSOCK(inode->i_mode))
         goto path_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.
+     * We intentionally forgo the ability to restrict SOCK_COREDUMP.
+     */
+    if (!(flags & SOCK_COREDUMP)) {
+        err = security_unix_path_connect(&path);
+        if (err)
+            goto path_put;
+    }
+    err = -ECONNREFUSED;
+
     sk = unix_find_socket_byinode(inode);
     if (!sk)
         goto path_put;



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2025-12-31 21:33 [RFC PATCH 0/1] lsm: Add hook unix_path_connect Justin Suess
                   ` (2 preceding siblings ...)
  2026-01-01 11:56 ` [RFC PATCH 0/1] " Günther Noack
@ 2026-01-05  7:46 ` Kuniyuki Iwashima
  2026-01-05 11:04   ` Günther Noack
  2026-01-07 21:54 ` Paul Moore
  4 siblings, 1 reply; 20+ messages in thread
From: Kuniyuki Iwashima @ 2026-01-05  7:46 UTC (permalink / raw)
  To: Justin Suess
  Cc: Paul Moore, James Morris, Serge E . Hallyn, Simon Horman,
	Mickaël Salaün, Günther Noack,
	linux-security-module, Tingmao Wang, netdev

On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
>
> Hi,
>
> This patch introduces a new LSM hook unix_path_connect.
>
> The idea for this patch and the hook came from Günther Noack, who
> is cc'd. Much credit to him for the idea and discussion.
>
> This patch is based on the lsm next branch.
>
> Motivation
> ---
>
> For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> identifying object from a policy perspective is the path passed to
> connect(2). However, this operation currently restricts LSMs that rely
> on VFS-based mediation, because the pathname resolved during connect()
> is not preserved in a form visible to existing hooks before connection
> establishment.

Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
and security_unix_may_send() ?


> As a result, LSMs such as Landlock cannot currently
> restrict connections to named UNIX domain sockets by their VFS path.
>
> This gap has been discussed previously (e.g. in the context of Landlock's
> path-based access controls). [1] [2]
>
> I've cc'd the netdev folks as well on this, as the placement of this hook is
> important and in a core unix socket function.
>
> Design Choices
> ---
>
> The hook is called in net/unix/af_unix.c in the function unix_find_bsd().
>
> The hook takes a single parameter, a const struct path* to the named unix
> socket to which the connection is being established.
>
> The hook takes place after normal permissions checks, and after the
> inode is determined to be a socket. It however, takes place before
> the socket is actually connected to.
>
> If the hook returns non-zero it will do a put on the path, and return.
>
> References
> ---
>
> [1]: https://github.com/landlock-lsm/linux/issues/36#issue-2354007438
> [2]: https://lore.kernel.org/linux-security-module/cover.1767115163.git.m@maowtm.org/
>
> Kind Regards,
> Justin Suess
>
> Justin Suess (1):
>   lsm: Add hook unix_path_connect
>
>  include/linux/lsm_hook_defs.h |  1 +
>  include/linux/security.h      |  6 ++++++
>  net/unix/af_unix.c            |  8 ++++++++
>  security/security.c           | 16 ++++++++++++++++
>  4 files changed, 31 insertions(+)
>
>
> base-commit: 1c0860d4415d52f3ad1c8e0a15c1272869278a06
> --
> 2.51.0
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2026-01-05  7:46 ` Kuniyuki Iwashima
@ 2026-01-05 11:04   ` Günther Noack
  2026-01-05 16:04     ` Justin Suess
  2026-01-07  7:33     ` Kuniyuki Iwashima
  0 siblings, 2 replies; 20+ messages in thread
From: Günther Noack @ 2026-01-05 11:04 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Justin Suess, Paul Moore, James Morris, Serge E . Hallyn,
	Simon Horman, Mickaël Salaün, linux-security-module,
	Tingmao Wang, netdev

Hello!

On Sun, Jan 04, 2026 at 11:46:46PM -0800, Kuniyuki Iwashima wrote:
> On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
> > Motivation
> > ---
> >
> > For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> > identifying object from a policy perspective is the path passed to
> > connect(2). However, this operation currently restricts LSMs that rely
> > on VFS-based mediation, because the pathname resolved during connect()
> > is not preserved in a form visible to existing hooks before connection
> > establishment.
> 
> Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
> and security_unix_may_send() ?

Thanks for bringing it up!

That path is set by the process that acts as the listening side for
the socket.  The listening and the connecting process might not live
in the same mount namespace, and in that case, it would not match the
path which is passed by the client in the struct sockaddr_un.

For more details, see
https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
and
https://github.com/landlock-lsm/linux/issues/36#issuecomment-2950632277

Justin: Maybe we could add that reasoning to the cover letter in the
next version of the patch?

–Günther

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2026-01-05 11:04   ` Günther Noack
@ 2026-01-05 16:04     ` Justin Suess
  2026-01-07  7:33     ` Kuniyuki Iwashima
  1 sibling, 0 replies; 20+ messages in thread
From: Justin Suess @ 2026-01-05 16:04 UTC (permalink / raw)
  To: Günther Noack, Kuniyuki Iwashima
  Cc: Paul Moore, James Morris, Serge E . Hallyn, Simon Horman,
	Mickaël Salaün, linux-security-module, Tingmao Wang,
	netdev

On 1/5/26 06:04, Günther Noack wrote:
> Hello!
>
> On Sun, Jan 04, 2026 at 11:46:46PM -0800, Kuniyuki Iwashima wrote:
>> On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
>>> Motivation
>>> ---
>>>
>>> For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
>>> identifying object from a policy perspective is the path passed to
>>> connect(2). However, this operation currently restricts LSMs that rely
>>> on VFS-based mediation, because the pathname resolved during connect()
>>> is not preserved in a form visible to existing hooks before connection
>>> establishment.
>> Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
>> and security_unix_may_send() ?
> Thanks for bringing it up!
>
> That path is set by the process that acts as the listening side for
> the socket.  The listening and the connecting process might not live
> in the same mount namespace, and in that case, it would not match the
> path which is passed by the client in the struct sockaddr_un.

Agreed. For the unix_sk(other)->path method you described to work, it requires the 

programs to be in the same mount namespace.


Doing it this way would make it impossible for landlock to restrict sockets mounted into a container,

and would be very confusing behavior for users to deal with, which is exactly the kind of stuff landlock avoids.


Does anyone have any thoughts on ignoring SOCK_COREDUMP? I think ignoring it for this check is correct.

>
> For more details, see
> https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
> and
> https://github.com/landlock-lsm/linux/issues/36#issuecomment-2950632277
>
> Justin: Maybe we could add that reasoning to the cover letter in the
> next version of the patch?
>
> –Günther

Will do. It's about ready to go, I may resend it today if I have time.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2026-01-05 11:04   ` Günther Noack
  2026-01-05 16:04     ` Justin Suess
@ 2026-01-07  7:33     ` Kuniyuki Iwashima
  2026-01-07 12:19       ` Justin Suess
  2026-01-07 12:49       ` Günther Noack
  1 sibling, 2 replies; 20+ messages in thread
From: Kuniyuki Iwashima @ 2026-01-07  7:33 UTC (permalink / raw)
  To: Günther Noack
  Cc: Justin Suess, Paul Moore, James Morris, Serge E . Hallyn,
	Simon Horman, Mickaël Salaün, linux-security-module,
	Tingmao Wang, netdev, Alexander Viro, Christian Brauner

+VFS maintainers

On Mon, Jan 5, 2026 at 3:04 AM Günther Noack <gnoack@google.com> wrote:
>
> Hello!
>
> On Sun, Jan 04, 2026 at 11:46:46PM -0800, Kuniyuki Iwashima wrote:
> > On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
> > > Motivation
> > > ---
> > >
> > > For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> > > identifying object from a policy perspective is the path passed to
> > > connect(2). However, this operation currently restricts LSMs that rely
> > > on VFS-based mediation, because the pathname resolved during connect()
> > > is not preserved in a form visible to existing hooks before connection
> > > establishment.
> >
> > Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
> > and security_unix_may_send() ?
>
> Thanks for bringing it up!
>
> That path is set by the process that acts as the listening side for
> the socket.  The listening and the connecting process might not live
> in the same mount namespace, and in that case, it would not match the
> path which is passed by the client in the struct sockaddr_un.

Thanks for the explanation !

So basically what you need is resolving unix_sk(sk)->addr.name
by kern_path() and comparing its d_backing_inode(path.dentry)
with d_backing_inode (unix_sk(sk)->path.dendtry).

If the new hook is only used by Landlock, I'd prefer doing that on
the existing connect() hooks.


>
> For more details, see
> https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
> and
> https://github.com/landlock-lsm/linux/issues/36#issuecomment-2950632277
>
> Justin: Maybe we could add that reasoning to the cover letter in the
> next version of the patch?
>
> –Günther

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2026-01-07  7:33     ` Kuniyuki Iwashima
@ 2026-01-07 12:19       ` Justin Suess
  2026-01-07 16:57         ` Günther Noack
  2026-01-07 12:49       ` Günther Noack
  1 sibling, 1 reply; 20+ messages in thread
From: Justin Suess @ 2026-01-07 12:19 UTC (permalink / raw)
  To: Kuniyuki Iwashima, Günther Noack
  Cc: Paul Moore, James Morris, Serge E . Hallyn, Simon Horman,
	Mickaël Salaün, linux-security-module, Tingmao Wang,
	netdev, Alexander Viro, Christian Brauner

On 1/7/26 02:33, Kuniyuki Iwashima wrote:
> +VFS maintainers
>
> On Mon, Jan 5, 2026 at 3:04 AM Günther Noack <gnoack@google.com> wrote:
>> Hello!
>>
>> On Sun, Jan 04, 2026 at 11:46:46PM -0800, Kuniyuki Iwashima wrote:
>>> On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
>>>> Motivation
>>>> ---
>>>>
>>>> For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
>>>> identifying object from a policy perspective is the path passed to
>>>> connect(2). However, this operation currently restricts LSMs that rely
>>>> on VFS-based mediation, because the pathname resolved during connect()
>>>> is not preserved in a form visible to existing hooks before connection
>>>> establishment.
>>> Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
>>> and security_unix_may_send() ?
>> Thanks for bringing it up!
>>
>> That path is set by the process that acts as the listening side for
>> the socket.  The listening and the connecting process might not live
>> in the same mount namespace, and in that case, it would not match the
>> path which is passed by the client in the struct sockaddr_un.
> Thanks for the explanation !
>
> So basically what you need is resolving unix_sk(sk)->addr.name
> by kern_path() and comparing its d_backing_inode(path.dentry)
> with d_backing_inode (unix_sk(sk)->path.dendtry).
>
> If the new hook is only used by Landlock, I'd prefer doing that on
> the existing connect() hooks.
I see. Did you have a particular hook in mind to extend?

One complication I see is whatever hook this gets added to
would also need CONFIG_SECURITY_PATH, since logically this restriction
would fall under it:

From security/Kconfig:

config SECURITY_PATH
    bool "Security hooks for pathname based access control"
    depends on SECURITY
    help
      This enables the security hooks for pathname based access control.
      If enabled, a security module can use these hooks to
      implement pathname based access controls.
      If you are unsure how to answer this question, answer N.

config SECURITY_NETWORK
    bool "Socket and Networking Security Hooks"
    depends on SECURITY
    help
      This enables the socket and networking security hooks.
      If enabled, a security module can use these hooks to
      implement socket and networking access controls.
      If you are unsure how to answer this question, answer N.

Logically, this type of access control falls under both categories, so must be
gated by both features. No existing LSM hooks are gated by both afaik, so
there is not really an existing logical place to extend an existing hook without
changing what features are required to be enabled for existing users.

I do see more uses for this hook that just landlock, bpf lsm hooks
or other non-labeling LSMs like apparmor or TOMOYO could take advantage
of this as well.

Günther did you have anything to add?

>> For more details, see
>> https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
>> and
>> https://github.com/landlock-lsm/linux/issues/36#issuecomment-2950632277
>>
>> Justin: Maybe we could add that reasoning to the cover letter in the
>> next version of the patch?
>>
>> –Günther



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2026-01-07  7:33     ` Kuniyuki Iwashima
  2026-01-07 12:19       ` Justin Suess
@ 2026-01-07 12:49       ` Günther Noack
  2026-01-08 10:17         ` Kuniyuki Iwashima
  1 sibling, 1 reply; 20+ messages in thread
From: Günther Noack @ 2026-01-07 12:49 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Justin Suess, Paul Moore, James Morris, Serge E . Hallyn,
	Simon Horman, Mickaël Salaün, linux-security-module,
	Tingmao Wang, netdev, Alexander Viro, Christian Brauner

On Tue, Jan 06, 2026 at 11:33:32PM -0800, Kuniyuki Iwashima wrote:
> +VFS maintainers
> 
> On Mon, Jan 5, 2026 at 3:04 AM Günther Noack <gnoack@google.com> wrote:
> >
> > Hello!
> >
> > On Sun, Jan 04, 2026 at 11:46:46PM -0800, Kuniyuki Iwashima wrote:
> > > On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
> > > > Motivation
> > > > ---
> > > >
> > > > For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> > > > identifying object from a policy perspective is the path passed to
> > > > connect(2). However, this operation currently restricts LSMs that rely
> > > > on VFS-based mediation, because the pathname resolved during connect()
> > > > is not preserved in a form visible to existing hooks before connection
> > > > establishment.
> > >
> > > Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
> > > and security_unix_may_send() ?
> >
> > Thanks for bringing it up!
> >
> > That path is set by the process that acts as the listening side for
> > the socket.  The listening and the connecting process might not live
> > in the same mount namespace, and in that case, it would not match the
> > path which is passed by the client in the struct sockaddr_un.
> 
> Thanks for the explanation !
> 
> So basically what you need is resolving unix_sk(sk)->addr.name
> by kern_path() and comparing its d_backing_inode(path.dentry)
> with d_backing_inode (unix_sk(sk)->path.dendtry).
> 
> If the new hook is only used by Landlock, I'd prefer doing that on
> the existing connect() hooks.

I've talked about that in the "Alternative: Use existing LSM hooks" section in
https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/

If we resolve unix_sk(sk)->addr.name ourselves in the Landlock hook
again, we would resolve the path twice: Once in unix_find_bsd() in
net/unix/af_unix.c (the Time-Of-Use), and once in the Landlock
security hook for the connect() operation (the Time-Of-Check).

If I understand you correctly, you are suggesting that we check that
the inode resolved by af_unix
(d_backing_inode(unix_sk(sk)->path.dentry)) is the same as the one
that we resolve in Landlock ourselves, and therefore we can detect the
TOCTOU race and pretend that this is equivalent to the case where
af_unix resolved to the same inode with the path that Landlock
observed?

If the walked file system hierarchy changes in between these two
accesses, Landlock enforces the policy based on path elements that
have changed in between.

* We start with a Landlock policy where Unix connect() is restricted
  by default, but is permitted on "foo/bar2" and everything underneath
  it.  The hierarchy is:

  foo/
      bar/
          baz.sock
      bar2/        <--- Landlock rule: socket connect() allowed here and below

* We connect() to the path "foo/bar/baz.sock"
* af_unix.c path lookup resolves "foo/bar/baz.sock" (TOU)
  This works because Landlock is not checked at this point yet.
* In between the two lookups:
  * the directory foo/bar gets renamed to foo/bar.old
  * foo/bar2 gets moved to foo/bar
  * baz.sock gets moved into the (new) foo/bar directory
* Landlock check: path lookup of "foo/bar/baz.sock" (TOC)
  and subsequent policy check using the resolved path.

  This succeeds because connect() is permitted on foo/bar2 and
  beneath.  We also check that the resolved inode is the same as the
  one resolved by af_unix.c.

And now the reasoning is basically that this is fine because the
(inode) result of the two lookups was the same and we pretend that the
Landlock path lookup was the one where the actual permission check was
done?

Some pieces of this which I am still unsure about:

* What we are supposed to do when the two resolved inodes are not the
  same, because we detected the race?  We can not allow the connection
  in that case, but it would be wrong to deny it as well.  I'm not
  sure whether returning one of the -ERESTART* variants is feasible in
  this place and bubbles up correctly to the system call / io_uring
  layer.

* What if other kinds of permission checks happen on a different
  lookup code path?  (If another stacked LSM had a similar
  implementation with yet another path lookup based on a different
  kind of policy, and if a race happened in between, it could at least
  be possible that for one variant of the path, it would be OK for
  Landlock but not the other LSM, and for the other variant of the
  path it would be OK for the other LSM but not Landlock, and then the
  connection could get accepted even if that would not have been
  allowed on one of the two paths alone.)  I find this a somewhat
  brittle implementation approach.

* Would have to double check the unix_dgram_connect code paths in
  af_unix to see whether this is feasible for DGRAM sockets:

  There is a way to connect() a connectionless DGRAM socket, and in
  that case, the path lookup in af_unix happens normally only during
  connect(), very far apart from the initial security_unix_may_send()
  LSM hook which is used for DGRAM sockets - It would be weird if we
  were able to connect() a DGRAM socket, thinking that now all path
  lookups are done, but then when you try to send a message through
  it, Landlock surprisingly does the path lookup again based on a very
  old and possibly outdated path.  If Landlock's path lookup fails
  (e.g. because the path has disappeared, or because the inode now
  differs), retries won't be able to recover this any more.  Normally,
  the path does not need to get resolved any more once the DGRAM
  socket is connected.

  Noteworthy: When Unix servers restart, they commonly unlink the old
  socket inode in the same place and create a new one with bind().  So
  as the time window for the race increases, it is actually a common
  scenario that a different inode with appear under the same path.

I have to digest this idea a bit.  I find it less intuitive than using
the exact same struct path with a newly introduced hook, but it does
admittedly mitigate the problem somewhat.  I'm just not feeling very
comfortable with security policy code that requires difficult
reasoning. 🤔 Or maybe I interpreted too much into your suggestion. :)
I'd be interested to hear what you think.

—Günther

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2026-01-07 12:19       ` Justin Suess
@ 2026-01-07 16:57         ` Günther Noack
  0 siblings, 0 replies; 20+ messages in thread
From: Günther Noack @ 2026-01-07 16:57 UTC (permalink / raw)
  To: Justin Suess
  Cc: Kuniyuki Iwashima, Paul Moore, James Morris, Serge E . Hallyn,
	Simon Horman, Mickaël Salaün, linux-security-module,
	Tingmao Wang, netdev, Alexander Viro, Christian Brauner

On Wed, Jan 07, 2026 at 07:19:02AM -0500, Justin Suess wrote:
> On 1/7/26 02:33, Kuniyuki Iwashima wrote:
> > +VFS maintainers
> >
> > On Mon, Jan 5, 2026 at 3:04 AM Günther Noack <gnoack@google.com> wrote:
> >> Hello!
> >>
> >> On Sun, Jan 04, 2026 at 11:46:46PM -0800, Kuniyuki Iwashima wrote:
> >>> On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
> >>>> Motivation
> >>>> ---
> >>>>
> >>>> For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> >>>> identifying object from a policy perspective is the path passed to
> >>>> connect(2). However, this operation currently restricts LSMs that rely
> >>>> on VFS-based mediation, because the pathname resolved during connect()
> >>>> is not preserved in a form visible to existing hooks before connection
> >>>> establishment.
> >>> Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
> >>> and security_unix_may_send() ?
> >> Thanks for bringing it up!
> >>
> >> That path is set by the process that acts as the listening side for
> >> the socket.  The listening and the connecting process might not live
> >> in the same mount namespace, and in that case, it would not match the
> >> path which is passed by the client in the struct sockaddr_un.
> > Thanks for the explanation !
> >
> > So basically what you need is resolving unix_sk(sk)->addr.name
> > by kern_path() and comparing its d_backing_inode(path.dentry)
> > with d_backing_inode (unix_sk(sk)->path.dendtry).
> >
> > If the new hook is only used by Landlock, I'd prefer doing that on
> > the existing connect() hooks.
> I see. Did you have a particular hook in mind to extend?
> 
> One complication I see is whatever hook this gets added to
> would also need CONFIG_SECURITY_PATH, since logically this restriction
> would fall under it:
> 
> From security/Kconfig:
> 
> config SECURITY_PATH
>     bool "Security hooks for pathname based access control"
>     depends on SECURITY
>     help
>       This enables the security hooks for pathname based access control.
>       If enabled, a security module can use these hooks to
>       implement pathname based access controls.
>       If you are unsure how to answer this question, answer N.
> 
> config SECURITY_NETWORK
>     bool "Socket and Networking Security Hooks"
>     depends on SECURITY
>     help
>       This enables the socket and networking security hooks.
>       If enabled, a security module can use these hooks to
>       implement socket and networking access controls.
>       If you are unsure how to answer this question, answer N.
> 
> Logically, this type of access control falls under both categories, so must be
> gated by both features. No existing LSM hooks are gated by both afaik, so
> there is not really an existing logical place to extend an existing hook without
> changing what features are required to be enabled for existing users.
> 
> I do see more uses for this hook that just landlock, bpf lsm hooks
> or other non-labeling LSMs like apparmor or TOMOYO could take advantage
> of this as well.

Apologies, I overlooked your reply earlier today.

The existing hooks that are called from af_unix.c are:

- security_unix_stream_connect() for SOCK_STREAM unix(7) sockets
- security_unix_may_send() for SOCK_DGRAM unix(7) sockets

Apart from that, at a higher level, there are also

- security_socket_connect()
- security_socket_sendmsg() and security_socket_recvmsg()

These are used from net/socket.c.

For the connectionless dgram Unix sockets, we would need to tell apart the cases
where sendmsg()/recvmsg() are used with and without a sockaddr.  (Dgram sockets
can be either connected with connect() and then have a fixed sockaddr, or they
can be passed a remote sockaddr with each message send and receive operation.)
This can told apart in security_socket_sendmsg() from the msg argument, but it
doesn't look like we could tell it apart from security_unix_may_send().

Landlock already depends on CONFIG_SECURITY_NETWORK and CONFIG_SECURITY_PATH,
so we would not need to have further #ifdefs to use one of these hooks.

There are other difficulties I found which worry me and which I listed in the
other mail at https://lore.kernel.org/all/aV5WTGvQB0XI8Q_N@google.com/.

—Günther

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2026-01-01 19:45     ` [RFC PATCH 0/1] " Justin Suess
  2026-01-01 23:11       ` Tingmao Wang
@ 2026-01-07 21:43       ` Paul Moore
  1 sibling, 0 replies; 20+ messages in thread
From: Paul Moore @ 2026-01-07 21:43 UTC (permalink / raw)
  To: Justin Suess
  Cc: gnoack3000, gnoack, horms, jmorris, kuniyu, linux-security-module,
	m, mic, netdev, serge

On Thu, Jan 1, 2026 at 2:45 PM Justin Suess <utilityemal77@gmail.com> wrote:
> On 1/1/26 07:13, Günther Noack wrote:
> > On Wed, Dec 31, 2025 at 04:33:14PM -0500, Justin Suess wrote:
> >> Adds an LSM hook unix_path_connect.
> >>
> >> This hook is called to check the path of a named unix socket before a
> >> connection is initiated.
> >>
> >> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> >> Cc: Günther Noack <gnoack3000@gmail.com>
> >> ---
> >>  include/linux/lsm_hook_defs.h |  1 +
> >>  include/linux/security.h      |  6 ++++++
> >>  net/unix/af_unix.c            |  8 ++++++++
> >>  security/security.c           | 16 ++++++++++++++++
> >>  4 files changed, 31 insertions(+)

...

> >> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> >> index 55cdebfa0da0..af1a6083a69b 100644
> >> --- a/net/unix/af_unix.c
> >> +++ b/net/unix/af_unix.c
> >> @@ -1226,6 +1226,14 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
> >>      if (!S_ISSOCK(inode->i_mode))
> >>              goto path_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_path_connect(&path);
> >> +    if (err)
> >> +            goto path_put;
> >
> > In this place, the hook call is done also for the coredump socket.
> >
> > The coredump socket is a system-wide setting, and it feels weird to me
> > that unprivileged processes should be able to inhibit that connection?
>
> No I don't think they should be able to. Does this look better?

Expect more comments on this patch, but this is important enough that
I wanted to reply separately.

As a reminder, we do have guidance regarding the addition of new LSM
hooks, there is a pointer to the document in MAINTAINERS, but here is
a direct link to the relevant section:

https://github.com/LinuxSecurityModule/kernel/blob/main/README.md#new-lsm-hooks

The guidance has three bullet points, the first, and perhaps most
important, states:

  "Hooks should be designed to be LSM agnostic. While it is possible
   that only one LSM might implement the hook at the time of submission,
   the hook's behavior should be generic enough that other LSMs could
   provide a meaningful implementation."

This is one of the reasons why we generally don't make the LSM hook
calls conditional on kernel state outside of the LSM, e.g.
SOCK_COREDUMP.  While Landlock may not want to implement any access
controls on a SOCK_COREDUMP socket, it's entirely possible that
another LSM which doesn't have untrusted processes defining security
policy may want to use this as a point of access control or
visibility/auditing.  Further, I think it would be a good idea to also
pass the @type and @flags parameter to the hook; at the very least
Landlock would need the flags parameter to check for SOCK_COREDUMP.

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2025-12-31 21:33 [RFC PATCH 0/1] lsm: Add hook unix_path_connect Justin Suess
                   ` (3 preceding siblings ...)
  2026-01-05  7:46 ` Kuniyuki Iwashima
@ 2026-01-07 21:54 ` Paul Moore
  4 siblings, 0 replies; 20+ messages in thread
From: Paul Moore @ 2026-01-07 21:54 UTC (permalink / raw)
  To: Justin Suess
  Cc: James Morris, Serge E . Hallyn, Kuniyuki Iwashima, Simon Horman,
	Mickaël Salaün, Günther Noack,
	linux-security-module, Tingmao Wang, netdev

On Wed, Dec 31, 2025 at 4:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
>
> Hi,
>
> This patch introduces a new LSM hook unix_path_connect.
>
> The idea for this patch and the hook came from Günther Noack, who
> is cc'd. Much credit to him for the idea and discussion.
>
> This patch is based on the lsm next branch.
>
> Motivation
> ---
>
> For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> identifying object from a policy perspective is the path passed to
> connect(2). However, this operation currently restricts LSMs that rely
> on VFS-based mediation, because the pathname resolved during connect()
> is not preserved in a form visible to existing hooks before connection
> establishment. As a result, LSMs such as Landlock cannot currently
> restrict connections to named UNIX domain sockets by their VFS path.
>
> This gap has been discussed previously (e.g. in the context of Landlock's
> path-based access controls). [1] [2]
>
> I've cc'd the netdev folks as well on this, as the placement of this hook is
> important and in a core unix socket function.
>
> Design Choices
> ---
>
> The hook is called in net/unix/af_unix.c in the function unix_find_bsd().
>
> The hook takes a single parameter, a const struct path* to the named unix
> socket to which the connection is being established.
>
> The hook takes place after normal permissions checks, and after the
> inode is determined to be a socket. It however, takes place before
> the socket is actually connected to.
>
> If the hook returns non-zero it will do a put on the path, and return.
>
> References
> ---
>
> [1]: https://github.com/landlock-lsm/linux/issues/36#issue-2354007438
> [2]: https://lore.kernel.org/linux-security-module/cover.1767115163.git.m@maowtm.org/
>
> Kind Regards,
> Justin Suess
>
> Justin Suess (1):
>   lsm: Add hook unix_path_connect
>
>  include/linux/lsm_hook_defs.h |  1 +
>  include/linux/security.h      |  6 ++++++
>  net/unix/af_unix.c            |  8 ++++++++
>  security/security.c           | 16 ++++++++++++++++
>  4 files changed, 31 insertions(+)

A couple of things related to the documentation aspects of this patch.
First, since this is just a single patch, and will need to be part of
a larger patchset to gain acceptance[1], please skip the cover letter
and ensure that the patch's description contains all the important
information.  Similarly, while it is fine to include references to
other sources of discussion in the patch's description, the links
should not replace a proper explanation of the patch.  Whenever you
are writing a patch description, imagine yourself ten years in the
future, on a plane with no/terrible network access, trying to debug an
issue and all you have for historical information is the git log.  I
promise you, it's not as outlandish as it might seem ;)

[1] See my other reply regarding new LSM hook guidance; this patch
will need to be part of a larger patchset that actually makes use of
this hook.

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2026-01-07 12:49       ` Günther Noack
@ 2026-01-08 10:17         ` Kuniyuki Iwashima
  2026-01-08 18:42           ` Mickaël Salaün
  2026-01-08 21:30           ` Günther Noack
  0 siblings, 2 replies; 20+ messages in thread
From: Kuniyuki Iwashima @ 2026-01-08 10:17 UTC (permalink / raw)
  To: Günther Noack
  Cc: Justin Suess, Paul Moore, James Morris, Serge E . Hallyn,
	Simon Horman, Mickaël Salaün, linux-security-module,
	Tingmao Wang, netdev, Alexander Viro, Christian Brauner

On Wed, Jan 7, 2026 at 4:49 AM Günther Noack <gnoack@google.com> wrote:
>
> On Tue, Jan 06, 2026 at 11:33:32PM -0800, Kuniyuki Iwashima wrote:
> > +VFS maintainers
> >
> > On Mon, Jan 5, 2026 at 3:04 AM Günther Noack <gnoack@google.com> wrote:
> > >
> > > Hello!
> > >
> > > On Sun, Jan 04, 2026 at 11:46:46PM -0800, Kuniyuki Iwashima wrote:
> > > > On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
> > > > > Motivation
> > > > > ---
> > > > >
> > > > > For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> > > > > identifying object from a policy perspective is the path passed to
> > > > > connect(2). However, this operation currently restricts LSMs that rely
> > > > > on VFS-based mediation, because the pathname resolved during connect()
> > > > > is not preserved in a form visible to existing hooks before connection
> > > > > establishment.
> > > >
> > > > Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
> > > > and security_unix_may_send() ?
> > >
> > > Thanks for bringing it up!
> > >
> > > That path is set by the process that acts as the listening side for
> > > the socket.  The listening and the connecting process might not live
> > > in the same mount namespace, and in that case, it would not match the
> > > path which is passed by the client in the struct sockaddr_un.
> >
> > Thanks for the explanation !
> >
> > So basically what you need is resolving unix_sk(sk)->addr.name
> > by kern_path() and comparing its d_backing_inode(path.dentry)
> > with d_backing_inode (unix_sk(sk)->path.dendtry).
> >
> > If the new hook is only used by Landlock, I'd prefer doing that on
> > the existing connect() hooks.
>
> I've talked about that in the "Alternative: Use existing LSM hooks" section in
> https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
>
> If we resolve unix_sk(sk)->addr.name ourselves in the Landlock hook
> again, we would resolve the path twice: Once in unix_find_bsd() in
> net/unix/af_unix.c (the Time-Of-Use), and once in the Landlock
> security hook for the connect() operation (the Time-Of-Check).
>
> If I understand you correctly, you are suggesting that we check that
> the inode resolved by af_unix
> (d_backing_inode(unix_sk(sk)->path.dentry)) is the same as the one
> that we resolve in Landlock ourselves, and therefore we can detect the
> TOCTOU race and pretend that this is equivalent to the case where
> af_unix resolved to the same inode with the path that Landlock
> observed?
>
> If the walked file system hierarchy changes in between these two
> accesses, Landlock enforces the policy based on path elements that
> have changed in between.
>
> * We start with a Landlock policy where Unix connect() is restricted
>   by default, but is permitted on "foo/bar2" and everything underneath
>   it.  The hierarchy is:
>
>   foo/
>       bar/
>           baz.sock
>       bar2/        <--- Landlock rule: socket connect() allowed here and below
>
> * We connect() to the path "foo/bar/baz.sock"
> * af_unix.c path lookup resolves "foo/bar/baz.sock" (TOU)
>   This works because Landlock is not checked at this point yet.
> * In between the two lookups:
>   * the directory foo/bar gets renamed to foo/bar.old
>   * foo/bar2 gets moved to foo/bar
>   * baz.sock gets moved into the (new) foo/bar directory
> * Landlock check: path lookup of "foo/bar/baz.sock" (TOC)
>   and subsequent policy check using the resolved path.
>
>   This succeeds because connect() is permitted on foo/bar2 and
>   beneath.  We also check that the resolved inode is the same as the
>   one resolved by af_unix.c.
>
> And now the reasoning is basically that this is fine because the
> (inode) result of the two lookups was the same and we pretend that the
> Landlock path lookup was the one where the actual permission check was
> done?

Right.  IIUC, even in your patch, the file could be renamed
while LSM is checking the path, no ?  I think holding the
path ref does not lock concurrent rename operations.

To me, it's not a small race and basically it's the same with
the ops below,

sk1.bind('test')
sk1.listen()
os.rename('test', 'test2')
sk2.connect('test2')

sk1.bind('test')
sk1.listen()
sk2.connect('test1')
os.rename('test', 'test2')

and the important part is whether the path _was_ the
allowed one when LSM checked the path.

>
> Some pieces of this which I am still unsure about:
>
> * What we are supposed to do when the two resolved inodes are not the
>   same, because we detected the race?  We can not allow the connection
>   in that case, but it would be wrong to deny it as well.  I'm not
>   sure whether returning one of the -ERESTART* variants is feasible in
>   this place and bubbles up correctly to the system call / io_uring
>   layer.

Imagine that the rename ops was done a bit earlier, which is
before the first lookup in unix_find_bsd().  Then, the socket
will not be found, and -ECONNREFUSED is returned.
LSM pcan pretend as such.


>
> * What if other kinds of permission checks happen on a different
>   lookup code path?  (If another stacked LSM had a similar
>   implementation with yet another path lookup based on a different
>   kind of policy, and if a race happened in between, it could at least
>   be possible that for one variant of the path, it would be OK for
>   Landlock but not the other LSM, and for the other variant of the
>   path it would be OK for the other LSM but not Landlock, and then the
>   connection could get accepted even if that would not have been
>   allowed on one of the two paths alone.)  I find this a somewhat
>   brittle implementation approach.

Do you mean that the evaluation of the stacked LSMs could
return 0 if one of them allows it even though other LSMs deny ?


>
> * Would have to double check the unix_dgram_connect code paths in
>   af_unix to see whether this is feasible for DGRAM sockets:
>
>   There is a way to connect() a connectionless DGRAM socket, and in
>   that case, the path lookup in af_unix happens normally only during
>   connect(),

Note that connected DGRAM socket can send() data to other sockets
by specifying the peer name in each send(), and even they can
disconnect by connect(AF_UNSPEC).


> very far apart from the initial security_unix_may_send()
>   LSM hook which is used for DGRAM sockets - It would be weird if we
>   were able to connect() a DGRAM socket, thinking that now all path
>   lookups are done, but then when you try to send a message through
>   it, Landlock surprisingly does the path lookup again based on a very
>   old and possibly outdated path.  If Landlock's path lookup fails
>   (e.g. because the path has disappeared, or because the inode now
>   differs), retries won't be able to recover this any more.  Normally,
>   the path does not need to get resolved any more once the DGRAM
>   socket is connected.
>
>   Noteworthy: When Unix servers restart, they commonly unlink the old
>   socket inode in the same place and create a new one with bind().  So
>   as the time window for the race increases, it is actually a common
>   scenario that a different inode with appear under the same path.
>
> I have to digest this idea a bit.  I find it less intuitive than using
> the exact same struct path with a newly introduced hook, but it does
> admittedly mitigate the problem somewhat.  I'm just not feeling very
> comfortable with security policy code that requires difficult
> reasoning. 🤔 Or maybe I interpreted too much into your suggestion. :)
> I'd be interested to hear what you think.
>
> —Günther

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2026-01-08 10:17         ` Kuniyuki Iwashima
@ 2026-01-08 18:42           ` Mickaël Salaün
  2026-01-08 21:30           ` Günther Noack
  1 sibling, 0 replies; 20+ messages in thread
From: Mickaël Salaün @ 2026-01-08 18:42 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Günther Noack, Justin Suess, Paul Moore, James Morris,
	Serge E . Hallyn, Simon Horman, linux-security-module,
	Tingmao Wang, netdev, Alexander Viro, Christian Brauner

On Thu, Jan 08, 2026 at 02:17:05AM -0800, Kuniyuki Iwashima wrote:
> On Wed, Jan 7, 2026 at 4:49 AM Günther Noack <gnoack@google.com> wrote:
> >
> > On Tue, Jan 06, 2026 at 11:33:32PM -0800, Kuniyuki Iwashima wrote:
> > > +VFS maintainers
> > >
> > > On Mon, Jan 5, 2026 at 3:04 AM Günther Noack <gnoack@google.com> wrote:
> > > >
> > > > Hello!
> > > >
> > > > On Sun, Jan 04, 2026 at 11:46:46PM -0800, Kuniyuki Iwashima wrote:
> > > > > On Wed, Dec 31, 2025 at 1:33 PM Justin Suess <utilityemal77@gmail.com> wrote:
> > > > > > Motivation
> > > > > > ---
> > > > > >
> > > > > > For AF_UNIX sockets bound to a filesystem path (aka named sockets), one
> > > > > > identifying object from a policy perspective is the path passed to
> > > > > > connect(2). However, this operation currently restricts LSMs that rely
> > > > > > on VFS-based mediation, because the pathname resolved during connect()
> > > > > > is not preserved in a form visible to existing hooks before connection
> > > > > > establishment.
> > > > >
> > > > > Why can't LSM use unix_sk(other)->path in security_unix_stream_connect()
> > > > > and security_unix_may_send() ?
> > > >
> > > > Thanks for bringing it up!
> > > >
> > > > That path is set by the process that acts as the listening side for
> > > > the socket.  The listening and the connecting process might not live
> > > > in the same mount namespace, and in that case, it would not match the
> > > > path which is passed by the client in the struct sockaddr_un.
> > >
> > > Thanks for the explanation !
> > >
> > > So basically what you need is resolving unix_sk(sk)->addr.name
> > > by kern_path() and comparing its d_backing_inode(path.dentry)
> > > with d_backing_inode (unix_sk(sk)->path.dendtry).

I would definitely prefer to avoid any kind of hack to try to detect
potential race conditions. :)  I think it would also be more difficult
to maintain.

A well-defined hook would avoid race conditions by design, simplify
kernel code, and document the security check.

> > >
> > > If the new hook is only used by Landlock, I'd prefer doing that on
> > > the existing connect() hooks.

I guess other security modules would like to rely on that too.

> >
> > I've talked about that in the "Alternative: Use existing LSM hooks" section in
> > https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
> >
> > If we resolve unix_sk(sk)->addr.name ourselves in the Landlock hook
> > again, we would resolve the path twice: Once in unix_find_bsd() in
> > net/unix/af_unix.c (the Time-Of-Use), and once in the Landlock
> > security hook for the connect() operation (the Time-Of-Check).
> >
> > If I understand you correctly, you are suggesting that we check that
> > the inode resolved by af_unix
> > (d_backing_inode(unix_sk(sk)->path.dentry)) is the same as the one
> > that we resolve in Landlock ourselves, and therefore we can detect the
> > TOCTOU race and pretend that this is equivalent to the case where
> > af_unix resolved to the same inode with the path that Landlock
> > observed?
> >
> > If the walked file system hierarchy changes in between these two
> > accesses, Landlock enforces the policy based on path elements that
> > have changed in between.
> >
> > * We start with a Landlock policy where Unix connect() is restricted
> >   by default, but is permitted on "foo/bar2" and everything underneath
> >   it.  The hierarchy is:
> >
> >   foo/
> >       bar/
> >           baz.sock
> >       bar2/        <--- Landlock rule: socket connect() allowed here and below
> >
> > * We connect() to the path "foo/bar/baz.sock"
> > * af_unix.c path lookup resolves "foo/bar/baz.sock" (TOU)
> >   This works because Landlock is not checked at this point yet.
> > * In between the two lookups:
> >   * the directory foo/bar gets renamed to foo/bar.old
> >   * foo/bar2 gets moved to foo/bar
> >   * baz.sock gets moved into the (new) foo/bar directory
> > * Landlock check: path lookup of "foo/bar/baz.sock" (TOC)
> >   and subsequent policy check using the resolved path.
> >
> >   This succeeds because connect() is permitted on foo/bar2 and
> >   beneath.  We also check that the resolved inode is the same as the
> >   one resolved by af_unix.c.
> >
> > And now the reasoning is basically that this is fine because the
> > (inode) result of the two lookups was the same and we pretend that the
> > Landlock path lookup was the one where the actual permission check was
> > done?
> 
> Right.  IIUC, even in your patch, the file could be renamed
> while LSM is checking the path, no ?

Yes but that should not be an issue wrt to the security policy.  The
check should atomic and consistent with the unix socket path resolution
used by the network stack.  In fact, comparing paths would potentially
forbid such rename, whereas this might be legitimate.

> I think holding the
> path ref does not lock concurrent rename operations.

We cannot hold a path ref without potential VFS issues.

> 
> To me, it's not a small race and basically it's the same with
> the ops below,
> 
> sk1.bind('test')
> sk1.listen()
> os.rename('test', 'test2')
> sk2.connect('test2')
> 
> sk1.bind('test')
> sk1.listen()
> sk2.connect('test1')
> os.rename('test', 'test2')
> 
> and the important part is whether the path _was_ the
> allowed one when LSM checked the path.

In the case of Landlock's sandboxing, we want to check the path at
connect time because that's when it makes sense for the client wrt to
its request (and its security policy).

FYI, Landlock identifies paths with a set of inodes, so if the unix
socket is explicitly allowed, then a rename may still be allowed by the
security policy.

> 
> >
> > Some pieces of this which I am still unsure about:
> >
> > * What we are supposed to do when the two resolved inodes are not the
> >   same, because we detected the race?  We can not allow the connection
> >   in that case, but it would be wrong to deny it as well.  I'm not
> >   sure whether returning one of the -ERESTART* variants is feasible in
> >   this place and bubbles up correctly to the system call / io_uring
> >   layer.
> 
> Imagine that the rename ops was done a bit earlier, which is
> before the first lookup in unix_find_bsd().  Then, the socket
> will not be found, and -ECONNREFUSED is returned.
> LSM pcan pretend as such.

Yes but this would be inconsistent with the network stack.  It would
introduce a race condition where unix socket cannot be used for
potentially no legitimate reason.

> 
> 
> >
> > * What if other kinds of permission checks happen on a different
> >   lookup code path?  (If another stacked LSM had a similar
> >   implementation with yet another path lookup based on a different
> >   kind of policy, and if a race happened in between, it could at least
> >   be possible that for one variant of the path, it would be OK for
> >   Landlock but not the other LSM, and for the other variant of the
> >   path it would be OK for the other LSM but not Landlock, and then the
> >   connection could get accepted even if that would not have been
> >   allowed on one of the two paths alone.)  I find this a somewhat
> >   brittle implementation approach.
> 
> Do you mean that the evaluation of the stacked LSMs could
> return 0 if one of them allows it even though other LSMs deny ?

If any LSM returns a non-zero value, then the call stops.

I think what Günther wanted to highlight is that a hook call may lead to
different hook implementation calls, and all these implementations should
be able to return consistent results wrt to other calls.

> 
> 
> >
> > * Would have to double check the unix_dgram_connect code paths in
> >   af_unix to see whether this is feasible for DGRAM sockets:
> >
> >   There is a way to connect() a connectionless DGRAM socket, and in
> >   that case, the path lookup in af_unix happens normally only during
> >   connect(),
> 
> Note that connected DGRAM socket can send() data to other sockets
> by specifying the peer name in each send(), and even they can
> disconnect by connect(AF_UNSPEC).

Yes, thanks for pointing this out.  It's the duty of LSMs to correctly
handle this case.  It is handled by Landlock for abstract unix sockets.

> 
> 
> > very far apart from the initial security_unix_may_send()
> >   LSM hook which is used for DGRAM sockets - It would be weird if we
> >   were able to connect() a DGRAM socket, thinking that now all path
> >   lookups are done, but then when you try to send a message through
> >   it, Landlock surprisingly does the path lookup again based on a very
> >   old and possibly outdated path.  If Landlock's path lookup fails
> >   (e.g. because the path has disappeared, or because the inode now
> >   differs), retries won't be able to recover this any more.  Normally,
> >   the path does not need to get resolved any more once the DGRAM
> >   socket is connected.
> >
> >   Noteworthy: When Unix servers restart, they commonly unlink the old
> >   socket inode in the same place and create a new one with bind().  So
> >   as the time window for the race increases, it is actually a common
> >   scenario that a different inode with appear under the same path.
> >
> > I have to digest this idea a bit.  I find it less intuitive than using
> > the exact same struct path with a newly introduced hook, but it does
> > admittedly mitigate the problem somewhat.  I'm just not feeling very
> > comfortable with security policy code that requires difficult
> > reasoning. 🤔 Or maybe I interpreted too much into your suggestion. :)
> > I'd be interested to hear what you think.
> >
> > —Günther

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect
  2026-01-08 10:17         ` Kuniyuki Iwashima
  2026-01-08 18:42           ` Mickaël Salaün
@ 2026-01-08 21:30           ` Günther Noack
  1 sibling, 0 replies; 20+ messages in thread
From: Günther Noack @ 2026-01-08 21:30 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Günther Noack, Justin Suess, Paul Moore, James Morris,
	Serge E . Hallyn, Simon Horman, Mickaël Salaün,
	linux-security-module, Tingmao Wang, netdev, Alexander Viro,
	Christian Brauner

On Thu, Jan 08, 2026 at 02:17:05AM -0800, Kuniyuki Iwashima wrote:
> On Wed, Jan 7, 2026 at 4:49 AM Günther Noack <gnoack@google.com> wrote:
> > On Tue, Jan 06, 2026 at 11:33:32PM -0800, Kuniyuki Iwashima wrote:
> > > +VFS maintainers
> > >
> > > [...]
> > >
> > > Thanks for the explanation !
> > >
> > > So basically what you need is resolving unix_sk(sk)->addr.name
> > > by kern_path() and comparing its d_backing_inode(path.dentry)
> > > with d_backing_inode (unix_sk(sk)->path.dendtry).
> > >
> > > If the new hook is only used by Landlock, I'd prefer doing that on
> > > the existing connect() hooks.
> >
> > I've talked about that in the "Alternative: Use existing LSM hooks" section in
> > https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
> >
> > If we resolve unix_sk(sk)->addr.name ourselves in the Landlock hook
> > again, we would resolve the path twice: Once in unix_find_bsd() in
> > net/unix/af_unix.c (the Time-Of-Use), and once in the Landlock
> > security hook for the connect() operation (the Time-Of-Check).
> >
> > If I understand you correctly, you are suggesting that we check that
> > the inode resolved by af_unix
> > (d_backing_inode(unix_sk(sk)->path.dentry)) is the same as the one
> > that we resolve in Landlock ourselves, and therefore we can detect the
> > TOCTOU race and pretend that this is equivalent to the case where
> > af_unix resolved to the same inode with the path that Landlock
> > observed?
> >
> > If the walked file system hierarchy changes in between these two
> > accesses, Landlock enforces the policy based on path elements that
> > have changed in between.
> >
> > * We start with a Landlock policy where Unix connect() is restricted
> >   by default, but is permitted on "foo/bar2" and everything underneath
> >   it.  The hierarchy is:
> >
> >   foo/
> >       bar/
> >           baz.sock
> >       bar2/        <--- Landlock rule: socket connect() allowed here and below
> >
> > * We connect() to the path "foo/bar/baz.sock"
> > * af_unix.c path lookup resolves "foo/bar/baz.sock" (TOU)
> >   This works because Landlock is not checked at this point yet.
> > * In between the two lookups:
> >   * the directory foo/bar gets renamed to foo/bar.old
> >   * foo/bar2 gets moved to foo/bar
> >   * baz.sock gets moved into the (new) foo/bar directory
> > * Landlock check: path lookup of "foo/bar/baz.sock" (TOC)
> >   and subsequent policy check using the resolved path.
> >
> >   This succeeds because connect() is permitted on foo/bar2 and
> >   beneath.  We also check that the resolved inode is the same as the
> >   one resolved by af_unix.c.
> >
> > And now the reasoning is basically that this is fine because the
> > (inode) result of the two lookups was the same and we pretend that the
> > Landlock path lookup was the one where the actual permission check was
> > done?
> 
> Right.  IIUC, even in your patch, the file could be renamed
> while LSM is checking the path, no ?  I think holding the
> path ref does not lock concurrent rename operations.
> 
> To me, it's not a small race and basically it's the same with
> the ops below,
> 
> sk1.bind('test')
> sk1.listen()
> os.rename('test', 'test2')
> sk2.connect('test2')
> 
> sk1.bind('test')
> sk1.listen()
> sk2.connect('test1')
> os.rename('test', 'test2')
> 
> and the important part is whether the path _was_ the
> allowed one when LSM checked the path.

I think we are in agreement here, yes.  What matters is that the LSM
does its check on the same path as connect() used for the lookup (or
that at least it behaves that way to an outside observer).

> > Some pieces of this which I am still unsure about:
> >
> > * What we are supposed to do when the two resolved inodes are not the
> >   same, because we detected the race?  We can not allow the connection
> >   in that case, but it would be wrong to deny it as well.  I'm not
> >   sure whether returning one of the -ERESTART* variants is feasible in
> >   this place and bubbles up correctly to the system call / io_uring
> >   layer.
> 
> Imagine that the rename ops was done a bit earlier, which is
> before the first lookup in unix_find_bsd().  Then, the socket
> will not be found, and -ECONNREFUSED is returned.
> LSM pcan pretend as such.

No, it can not pretend as such when the inodes differ - the reasoning
is:

The rename(2) operation can be used to put a new socket in the place
of the old one, and both sockets might be OK to use in the Landlock
policy.  If Landlock observes the race and the inodes are different,
that still does not mean that it should refuse the connection.

The trace is:

* /sock1 and /sock2 exist
* initial unix_find_bsd() lookup for /sock1
* rename(2) /sock2 to /sock1, replacing it
* LSM (Landlock) lookup for /sock1

The two lookups return different inodes, but we still should not
refuse the connection because of that.


> > * What if other kinds of permission checks happen on a different
> >   lookup code path?  (If another stacked LSM had a similar
> >   implementation with yet another path lookup based on a different
> >   kind of policy, and if a race happened in between, it could at least
> >   be possible that for one variant of the path, it would be OK for
> >   Landlock but not the other LSM, and for the other variant of the
> >   path it would be OK for the other LSM but not Landlock, and then the
> >   connection could get accepted even if that would not have been
> >   allowed on one of the two paths alone.)  I find this a somewhat
> >   brittle implementation approach.
> 
> Do you mean that the evaluation of the stacked LSMs could
> return 0 if one of them allows it even though other LSMs deny ?

Yes. If there are two LSMs employing that scheme, that would
happen.

## Example scenario 1 (two LSMs)

* LSM1 permits connections to sockets under /dir1 and denies others,
  based on the inode associated with dir1.
* LSM2 permits connections to sockets under /dir2 and denies others.
  based on the inode associated with dir2.
* The sockets /dir1/sock1 and /dir2/sock2 exist.

* initial unix_find_bsd() lookup for /dir1/sock1
* LSM1 lookup for /dir1/sock1 ==> returns 0 (accepted because /dir1 is OK in LSM1)
* Race:
  * /dir1 gets moved to /dir1.old,
  * /dir2 gets moved to /dir1 (keeping the original /dir2 inode),
  * /dir1.old/sock1 gets moved to /dir2/sock1.
* LSM2 lookup for /dir1/sock1 ==> returns 0
  (accepted because /dir1 is the old /dir2 on whose inode LSM2 accepts the permission)

The race is not detected because the inode of the resolved socket is
the same for all three lookups.

At all points during the file renaming logic, sock1 stayed under the
inode of the original /dir1 or the inode of the original /dir2.  The
connection is supposed to be denied because for both of these
directories, one of the two LSMs should deny connections to sockets
that are stored therein.


## Example scenario 2 (only one LSM)

I just realize that a similar scenario also already applies to the
simpler case where there is *only* the Landlock LSM and the af_unix.c
lookup.  Bear with me, this is a bit of a wild scenario, but it shows
that the scheme of comparing the looked-up inode does not work:

* The directories /dir1 and /dir2 exist.
  * On /dir1, the unprivileged user has Unix permissions but the LSM
    denies access to sockets underneath based on the directories'
    inode.
  * On /dir2, the unprivileged user has *no* Unix permissions, but the
    LSM accepts access to sockets underneath, based on the dir inode.
* The socket /dir1/sock is a hardlink to /dir2/sock
* Assume there is a highly privileged service that constantly invokes
  renameat2() with RENAME_EXCHANGE, exchaning the two directories back
  and forth.

Now the following operations happen:

* initial unix_find_bsd() lookup for /dir1/sock
  * We get lucky and catch the inode where we get through Unix
    permission checks
* Race:
  * /dir1 and /dir2 get exchanged atomically
* LSM lookup for /dir1/sock
  * Now /dir1 is the inode where the LSM passes the LSM check

So we end up passing both checks because a rename() happened in between.

At any given point in time, the directory through which we accessed
the socket was either /dir1 or /dir2, and we lacked either the Unix
permissions or the LSM policy should deny it. Yet, because of this
rename race, we manage to sneak through both checks.

The inode comparison for the looked up inode does not catch it,
because that is only the "sock" inode, and does not cover the other
inodes along the path.

The example is a bit artificial to make it clear, but it nevertheless
shows that in race scenarios, the behaviour can still be different
(and permit more access) than in the sceneario there is only a single
path lookup.


> > * Would have to double check the unix_dgram_connect code paths in
> >   af_unix to see whether this is feasible for DGRAM sockets:
> >
> >   There is a way to connect() a connectionless DGRAM socket, and in
> >   that case, the path lookup in af_unix happens normally only during
> >   connect(),
> 
> Note that connected DGRAM socket can send() data to other sockets
> by specifying the peer name in each send(), and even they can
> disconnect by connect(AF_UNSPEC).

Yup.  (That case would have been easier, I think, because the two
lookups would have been closer to each other.)


> > very far apart from the initial security_unix_may_send()
> >   LSM hook which is used for DGRAM sockets - It would be weird if we
> >   were able to connect() a DGRAM socket, thinking that now all path
> >   lookups are done, but then when you try to send a message through
> >   it, Landlock surprisingly does the path lookup again based on a very
> >   old and possibly outdated path.  If Landlock's path lookup fails
> >   (e.g. because the path has disappeared, or because the inode now
> >   differs), retries won't be able to recover this any more.  Normally,
> >   the path does not need to get resolved any more once the DGRAM
> >   socket is connected.
> >
> >   Noteworthy: When Unix servers restart, they commonly unlink the old
> >   socket inode in the same place and create a new one with bind().  So
> >   as the time window for the race increases, it is actually a common
> >   scenario that a different inode with appear under the same path.
> >
> > I have to digest this idea a bit.  I find it less intuitive than using
> > the exact same struct path with a newly introduced hook, but it does
> > admittedly mitigate the problem somewhat.  I'm just not feeling very
> > comfortable with security policy code that requires difficult
> > reasoning. 🤔 Or maybe I interpreted too much into your suggestion. :)
> > I'd be interested to hear what you think.
> >
> > —Günther

As the above second example showed, when we do two path lookups, one
in af_unix and one in Landlock, even if we compare the resulting
socket inode to catch the TOCTOU, there are scenarios where the
resulting observable behaviour is different (and sometimes more lax)
than if there was only a single path lookup.

Given that, I'd be leaning towards the original proposal of adding new
LSM hook, provided that Paul and the respective network maintainers
are OK with it.

Thanks for bringing it up though.  It was an interesting exploration
to think through these scenarios.

Thanks,
–Günther

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2026-01-08 21:30 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-31 21:33 [RFC PATCH 0/1] lsm: Add hook unix_path_connect Justin Suess
2025-12-31 21:33 ` [RFC PATCH 1/1] " Justin Suess
2026-01-01 12:13   ` Günther Noack
2026-01-01 19:45     ` [RFC PATCH 0/1] " Justin Suess
2026-01-01 23:11       ` Tingmao Wang
2026-01-01 23:40         ` Justin Suess
2026-01-07 21:43       ` Paul Moore
2026-01-01  9:46 ` [syzbot ci] " syzbot ci
2026-01-01 11:56 ` [RFC PATCH 0/1] " Günther Noack
2026-01-05  7:46 ` Kuniyuki Iwashima
2026-01-05 11:04   ` Günther Noack
2026-01-05 16:04     ` Justin Suess
2026-01-07  7:33     ` Kuniyuki Iwashima
2026-01-07 12:19       ` Justin Suess
2026-01-07 16:57         ` Günther Noack
2026-01-07 12:49       ` Günther Noack
2026-01-08 10:17         ` Kuniyuki Iwashima
2026-01-08 18:42           ` Mickaël Salaün
2026-01-08 21:30           ` Günther Noack
2026-01-07 21:54 ` Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox