Linux Security Modules development
 help / color / mirror / Atom feed
* Re: [PATCH v2 01/13] mm/vma: remove __private sparse decoration from vma_flags_t
From: Liam R. Howlett @ 2026-02-09 18:38 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: Andrew Morton, Jarkko Sakkinen, Dave Hansen, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, H . Peter Anvin, Arnd Bergmann,
	Greg Kroah-Hartman, Dan Williams, Vishal Verma, Dave Jiang,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Tvrtko Ursulin, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Alexander Viro, Christian Brauner, Jan Kara,
	Benjamin LaHaise, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
	Sandeep Dhavale, Hongbo Li, Chunhai Guo, Theodore Ts'o,
	Andreas Dilger, Muchun Song, Oscar Salvador, David Hildenbrand,
	Konstantin Komarov, Mike Marshall, Martin Brandenburg, Tony Luck,
	Reinette Chatre, Dave Martin, James Morse, Babu Moger,
	Carlos Maiolino, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
	Matthew Wilcox, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Hugh Dickins, Baolin Wang,
	Zi Yan, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
	Lance Yang, Jann Horn, Pedro Falcato, David Howells, Paul Moore,
	James Morris, Serge E . Hallyn, Yury Norov, Rasmus Villemoes,
	linux-sgx, linux-kernel, nvdimm, linux-cxl, dri-devel, intel-gfx,
	linux-fsdevel, linux-aio, linux-erofs, linux-ext4, linux-mm,
	ntfs3, devel, linux-xfs, keyrings, linux-security-module,
	Jason Gunthorpe
In-Reply-To: <64fa89f416f22a60ae74cfff8fd565e7677be192.1769097829.git.lorenzo.stoakes@oracle.com>

* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [260122 16:06]:
> We need to pass around these values and access them in a way that sparse
> does not allow, as __private implies noderef, i.e. disallowing dereference
> of the value, which manifests as sparse warnings even when passed around
> benignly.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  include/linux/mm.h       |  4 ++--
>  include/linux/mm_types.h | 14 ++++++++------
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index d7ca837dd8a5..776a7e03f88b 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -943,7 +943,7 @@ static inline void vm_flags_reset_once(struct vm_area_struct *vma,
>  	 * system word.
>  	 */
>  	if (NUM_VMA_FLAG_BITS > BITS_PER_LONG) {
> -		unsigned long *bitmap = ACCESS_PRIVATE(&vma->flags, __vma_flags);
> +		unsigned long *bitmap = vma->flags.__vma_flags;
>  
>  		bitmap_zero(&bitmap[1], NUM_VMA_FLAG_BITS - BITS_PER_LONG);
>  	}
> @@ -1006,7 +1006,7 @@ static inline bool __vma_flag_atomic_valid(struct vm_area_struct *vma,
>  static inline void vma_flag_set_atomic(struct vm_area_struct *vma,
>  				       vma_flag_t bit)
>  {
> -	unsigned long *bitmap = ACCESS_PRIVATE(&vma->flags, __vma_flags);
> +	unsigned long *bitmap = vma->flags.__vma_flags;
>  
>  	vma_assert_stabilised(vma);
>  	if (__vma_flag_atomic_valid(vma, bit))
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index e5ee66f84d9a..592ad065fa75 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -866,7 +866,7 @@ struct mmap_action {
>  #define NUM_VMA_FLAG_BITS BITS_PER_LONG
>  typedef struct {
>  	DECLARE_BITMAP(__vma_flags, NUM_VMA_FLAG_BITS);
> -} __private vma_flags_t;
> +} vma_flags_t;
>  
>  /*
>   * Describes a VMA that is about to be mmap()'ed. Drivers may choose to
> @@ -1056,7 +1056,7 @@ struct vm_area_struct {
>  /* Clears all bits in the VMA flags bitmap, non-atomically. */
>  static inline void vma_flags_clear_all(vma_flags_t *flags)
>  {
> -	bitmap_zero(ACCESS_PRIVATE(flags, __vma_flags), NUM_VMA_FLAG_BITS);
> +	bitmap_zero(flags->__vma_flags, NUM_VMA_FLAG_BITS);
>  }
>  
>  /*
> @@ -1067,7 +1067,9 @@ static inline void vma_flags_clear_all(vma_flags_t *flags)
>   */
>  static inline void vma_flags_overwrite_word(vma_flags_t *flags, unsigned long value)
>  {
> -	*ACCESS_PRIVATE(flags, __vma_flags) = value;
> +	unsigned long *bitmap = flags->__vma_flags;
> +
> +	bitmap[0] = value;
>  }
>  
>  /*
> @@ -1078,7 +1080,7 @@ static inline void vma_flags_overwrite_word(vma_flags_t *flags, unsigned long va
>   */
>  static inline void vma_flags_overwrite_word_once(vma_flags_t *flags, unsigned long value)
>  {
> -	unsigned long *bitmap = ACCESS_PRIVATE(flags, __vma_flags);
> +	unsigned long *bitmap = flags->__vma_flags;
>  
>  	WRITE_ONCE(*bitmap, value);
>  }
> @@ -1086,7 +1088,7 @@ static inline void vma_flags_overwrite_word_once(vma_flags_t *flags, unsigned lo
>  /* Update the first system word of VMA flags setting bits, non-atomically. */
>  static inline void vma_flags_set_word(vma_flags_t *flags, unsigned long value)
>  {
> -	unsigned long *bitmap = ACCESS_PRIVATE(flags, __vma_flags);
> +	unsigned long *bitmap = flags->__vma_flags;
>  
>  	*bitmap |= value;
>  }
> @@ -1094,7 +1096,7 @@ static inline void vma_flags_set_word(vma_flags_t *flags, unsigned long value)
>  /* Update the first system word of VMA flags clearing bits, non-atomically. */
>  static inline void vma_flags_clear_word(vma_flags_t *flags, unsigned long value)
>  {
> -	unsigned long *bitmap = ACCESS_PRIVATE(flags, __vma_flags);
> +	unsigned long *bitmap = flags->__vma_flags;
>  
>  	*bitmap &= ~value;
>  }
> -- 
> 2.52.0
> 

^ permalink raw reply

* Re: [PATCH v4 1/6] lsm: Add LSM hook security_unix_find
From: Tingmao Wang @ 2026-02-09 18:33 UTC (permalink / raw)
  To: Mickaël Salaün, Justin Suess, Günther Noack
  Cc: John Johansen, Paul Moore, James Morris, Serge E . Hallyn,
	linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
	Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
	Alyssa Ross, Jann Horn, Tahera Fahimi, Simon Horman, netdev,
	Alexander Viro, Christian Brauner
In-Reply-To: <20260209.yeeh3ieDuz9u@digikod.net>

On 2/9/26 17:51, Mickaël Salaün wrote:
> On Mon, Feb 09, 2026 at 12:10:11AM +0100, Günther Noack wrote:
>> [...]
>> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
>> index d0511225799b..db9d279b3883 100644
>> --- a/net/unix/af_unix.c
>> +++ b/net/unix/af_unix.c
>> @@ -1226,10 +1226,19 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
>>  	if (!S_ISSOCK(inode->i_mode))
>>  		goto path_put;
>>  
>> +	err = -ECONNREFUSED;
> 
> We don't see it in this patch but err is already set to -ECONNREFUSED.
> This line might be confusing, and unrelated to the goal of this patch,
> so we should remove it.

I will confess that in a side conversation with Justin previously I
suggested that for blocks like these it might be better to always assign
to err, and let the compiler optimize it away, so that when this block is
moved there is less chances of mistake.  (This was relevant in the
previous context where a move of this hook caused err to be reset,
resulting in a NULL deference from syzbot)

But of course if the convention in this file is to not do it, or if I have
missed some reason against doing this, then that's also fine (even though,
IMHO, personally I think this is better).

^ permalink raw reply

* Re: [PATCH v4 2/6] landlock: Control pathname UNIX domain socket resolution by path
From: Mickaël Salaün @ 2026-02-09 18:03 UTC (permalink / raw)
  To: Günther Noack
  Cc: Günther Noack, John Johansen, Tingmao Wang, Justin Suess,
	Jann Horn, linux-security-module, Samasth Norway Ananda,
	Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
	Demi Marie Obenour, Alyssa Ross, Tahera Fahimi
In-Reply-To: <aYm1RWtV6Af-zEHf@google.com>

On Mon, Feb 09, 2026 at 11:21:57AM +0100, Günther Noack wrote:
> On Mon, Feb 09, 2026 at 12:10:12AM +0100, Günther Noack wrote:
> > +static int hook_unix_find(const struct path *const path, struct sock *other,
> > +			  int flags)
> > +{
> > +	const struct landlock_ruleset *dom_other;
> > +	const struct landlock_cred_security *subject;
> > +	struct layer_access_masks layer_masks;
> > +	struct landlock_request request = {};
> > +	static const struct access_masks fs_resolve_unix = {
> > +		.fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> > +	};
> > +	int type = other->sk_type;
> > +
> > +	/* Lookup for the purpose of saving coredumps is OK. */
> > +	if (flags & SOCK_COREDUMP)
> > +		return 0;

We should test this case too.
tools/testing/selftests/coredump/coredump_socket_* should help.

> > +
> > +	/* Only stream, dgram and seqpacket sockets are restricted. */
> > +	if (type != SOCK_STREAM && type != SOCK_DGRAM && type != SOCK_SEQPACKET)
> > +		return 0;

^ permalink raw reply

* Re: [PATCH v4 1/6] lsm: Add LSM hook security_unix_find
From: Mickaël Salaün @ 2026-02-09 17:51 UTC (permalink / raw)
  To: Günther Noack
  Cc: John Johansen, Paul Moore, James Morris, Serge E . Hallyn,
	Tingmao Wang, Justin Suess, linux-security-module,
	Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
	konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
	Tahera Fahimi, Simon Horman, netdev, Alexander Viro,
	Christian Brauner
In-Reply-To: <20260208231017.114343-2-gnoack3000@gmail.com>

On Mon, Feb 09, 2026 at 12:10:11AM +0100, Günther Noack wrote:
> From: Justin Suess <utilityemal77@gmail.com>
> 
> Add a LSM hook security_unix_find.
> 
> This hook is called to check the path of a named unix socket before a
> connection is initiated. The peer socket may be inspected as well.
> 
> Why existing hooks are unsuitable:
> 
> Existing socket hooks, security_unix_stream_connect(),
> security_unix_may_send(), and security_socket_connect() don't provide
> TOCTOU-free / namespace independent access to the paths of sockets.
> 
> (1) We cannot resolve the path from the struct sockaddr in existing hooks.
> This requires another path lookup. A change in the path between the
> two lookups will cause a TOCTOU bug.
> 
> (2) We cannot use the struct path from the listening socket, because it
> may be bound to a path in a different namespace than the caller,
> resulting in a path that cannot be referenced at policy creation time.
> 
> Cc: Günther Noack <gnoack3000@gmail.com>
> Cc: Tingmao Wang <m@maowtm.org>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
>  include/linux/lsm_hook_defs.h |  5 +++++
>  include/linux/security.h      | 11 +++++++++++
>  net/unix/af_unix.c            |  9 +++++++++
>  security/security.c           | 20 ++++++++++++++++++++
>  4 files changed, 45 insertions(+)
> 
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 8c42b4bde09c..7a0fd3dbfa29 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -317,6 +317,11 @@ LSM_HOOK(int, 0, post_notification, const struct cred *w_cred,
>  LSM_HOOK(int, 0, watch_key, struct key *key)
>  #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
>  
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other,
> +	 int flags)
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
>  #ifdef CONFIG_SECURITY_NETWORK
>  LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
>  	 struct sock *newsk)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 83a646d72f6f..99a33d8eb28d 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1931,6 +1931,17 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
>  }
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +
> +int security_unix_find(const struct path *path, struct sock *other, int flags);
> +
> +#else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
>  #ifdef CONFIG_SECURITY_INFINIBAND
>  int security_ib_pkey_access(void *sec, u64 subnet_prefix, u16 pkey);
>  int security_ib_endport_manage_subnet(void *sec, const char *name, u8 port_num);
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index d0511225799b..db9d279b3883 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1226,10 +1226,19 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
>  	if (!S_ISSOCK(inode->i_mode))
>  		goto path_put;
>  
> +	err = -ECONNREFUSED;

We don't see it in this patch but err is already set to -ECONNREFUSED.
This line might be confusing, and unrelated to the goal of this patch,
so we should remove it.


>  	sk = unix_find_socket_byinode(inode);
>  	if (!sk)
>  		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.

This comment can be alligned with 80 columns.

> +	 */
> +	err = security_unix_find(&path, sk, flags);

This hook makes sense and is quite generic.

> +	if (err)
> +		goto sock_put;
> +
>  	err = -EPROTOTYPE;
>  	if (sk->sk_type == type)
>  		touch_atime(&path);
> diff --git a/security/security.c b/security/security.c
> index 31a688650601..9e9515955098 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4731,6 +4731,26 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
>  
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +/*

This should be a docstring like other hooks: /**

> + * security_unix_find() - Check if a named AF_UNIX socket can connect
> + * @path: path of the socket being connected to
> + * @other: peer sock
> + * @flags: flags associated with the socket
> + *
> + * This hook is called to check permissions before connecting to a named
> + * AF_UNIX socket.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_unix_find(const struct path *path, struct sock *other, int flags)
> +{
> +	return call_int_hook(unix_find, path, other, flags);
> +}
> +EXPORT_SYMBOL(security_unix_find);
> +
> +#endif	/* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
>  #ifdef CONFIG_SECURITY_INFINIBAND
>  /**
>   * security_ib_pkey_access() - Check if access to an IB pkey is allowed
> -- 
> 2.52.0
> 
> 

^ permalink raw reply

* Re: [PATCH v4 2/6] landlock: Control pathname UNIX domain socket resolution by path
From: Mickaël Salaün @ 2026-02-09 17:28 UTC (permalink / raw)
  To: Günther Noack
  Cc: Günther Noack, John Johansen, Tingmao Wang, Justin Suess,
	Jann Horn, linux-security-module, Samasth Norway Ananda,
	Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
	Demi Marie Obenour, Alyssa Ross, Tahera Fahimi
In-Reply-To: <aYm1RWtV6Af-zEHf@google.com>

On Mon, Feb 09, 2026 at 11:21:57AM +0100, Günther Noack wrote:
> On Mon, Feb 09, 2026 at 12:10:12AM +0100, Günther Noack wrote:
> > +static int hook_unix_find(const struct path *const path, struct sock *other,
> > +			  int flags)
> > +{
> > +	const struct landlock_ruleset *dom_other;
> > +	const struct landlock_cred_security *subject;
> > +	struct layer_access_masks layer_masks;
> > +	struct landlock_request request = {};
> > +	static const struct access_masks fs_resolve_unix = {
> > +		.fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> > +	};
> > +	int type = other->sk_type;
> > +
> > +	/* Lookup for the purpose of saving coredumps is OK. */
> > +	if (flags & SOCK_COREDUMP)
> > +		return 0;
> > +
> > +	/* Only stream, dgram and seqpacket sockets are restricted. */
> > +	if (type != SOCK_STREAM && type != SOCK_DGRAM && type != SOCK_SEQPACKET)
> > +		return 0;
> 
> FYI: This is a (highly speculative) safeguard, because these three
> socket types are the only ones that exist in AF_UNIX (compare unix(7),
> 2nd paragraph).
> 
> In the (highly unlikely) case that someone adds a fourth AF_UNIX
> socket type, this means that Landlock will start permitting
> connections to these sockets unconditionally.
> 
> I am unsure whether the safeguard is useful, or whether we should
> rather group that (highly unlikely) future socket type together with
> the existing ones.  *If you have opinions, I'd be interested.*
> 
> The fact that these are the only existing AF_UNIX socket types is also
> the reason why it does not matter that we are now (in v4) taking the
> type value from the server-side sk instead of the client socket.  The
> check will either way always pass as long as only these three types
> are the only ones.
> 
> For now (and probably for another few decades :)), as long as these
> are the only AF_UNIX types, it does not make a difference though
> whether the check is there or not.

You can remove these type checks.  We're building Landlock access
control wrt to the (moving) current state of Linux, and the goal is to
cover most/useful access types that currently make sense.  Once access
type is implemented, it should handle (by default) future features
related to the kernel object to make sure a sandbox is well covered.
This LANDLOCK_ACCESS_FS_RESOLVE_UNIX right is really about UNIX sockets
that can be resolved through the filesystem, so this should handle
current and potential future UNIX sockets as well.

If a new named UNIX socket type is created, Landlock should handle that
with this access right, unless there is a specific semantic (e.g.
coredump), in which case we'll update the access right, and potentially
add a new one if it makes sense.

I was thinking about calling WARN_ON_ONCE() but this is not worth it.

^ permalink raw reply

* Re: [PATCH v4 4/6] landlock/selftests: Test named UNIX domain socket restrictions
From: Mickaël Salaün @ 2026-02-09 17:29 UTC (permalink / raw)
  To: Günther Noack
  Cc: John Johansen, Justin Suess, linux-security-module, Tingmao Wang,
	Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
	konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
	Tahera Fahimi
In-Reply-To: <20260208231017.114343-5-gnoack3000@gmail.com>

On Mon, Feb 09, 2026 at 12:10:14AM +0100, Günther Noack wrote:
> * Exercise the access right for connect() and sendmsg() on named UNIX
>   domain sockets, in various combinations of Landlock domains and
>   socket types.
> * Extract common helpers from an existing IOCTL test that
>   also uses pathname unix(7) sockets.
> 
> The tested combinations are the cross product of these sets of fixture
> fields:
> 
> * {{.handled=RESOLVE_UNIX},
>    {.handled=RESOLVE_UNIX, .allowed=RESOLVE_UNIX}}
> * {{.sock_type=SOCK_STREAM},
>    {.sock_type=SOCK_DGRAM},
>    {.sock_type=SOCK_DGRAM, .use_sendto=true},
>    {.sock_type=SOCK_SEQPACKET}}
> * {{.server_in_same_domain=false},
>    {.server_in_same_domain=true}}

It would improve test clarity to follow the same approach as Tingmao to
check the scope, especially to use the scoped_base_variant.h:
https://lore.kernel.org/all/88de5bed60b06ba97088d87803f7bb3dbcc9a808.1767115163.git.m@maowtm.org/
Even if there is no more explicit scoped flag anymore, this test suite
is still relevant.

The fs_test.c part would then mostly check the
LANDLOCK_ACCESS_FS_RESOLVE_UNIX rules/exceptions.

> 
> Some additional fixtures exercise scenarios with two nested domains.
> 
> Cc: Justin Suess <utilityemal77@gmail.com>
> Cc: Mickaël Salaün <mic@digikod.net>
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
>  tools/testing/selftests/landlock/fs_test.c | 381 ++++++++++++++++++++-
>  1 file changed, 365 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index b318627e7561..9d3f5dab4567 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -4358,30 +4358,61 @@ TEST_F_FORK(layout1, named_pipe_ioctl)
>  	ASSERT_EQ(child_pid, waitpid(child_pid, NULL, 0));
>  }
>  
> +/*
> + * set_up_named_unix_server - Create a pathname unix socket
> + *
> + * If the socket type is not SOCK_DGRAM, also invoke listen(2).
> + *
> + * Return: The listening FD - it is the caller responsibility to close it.
> + */
> +static int set_up_named_unix_server(struct __test_metadata *const _metadata,
> +				    int type, const char *const path)
> +{
> +	int fd;
> +	struct sockaddr_un addr = {
> +		.sun_family = AF_UNIX,
> +	};
> +
> +	fd = socket(AF_UNIX, type, 0);
> +	ASSERT_LE(0, fd);
> +
> +	strncpy(addr.sun_path, path, sizeof(addr.sun_path));
> +	ASSERT_EQ(0, bind(fd, (struct sockaddr *)&addr, sizeof(addr)));
> +
> +	if (type != SOCK_DGRAM)
> +		ASSERT_EQ(0, listen(fd, 10 /* qlen */));
> +	return fd;
> +}
> +
> +/*
> + * test_connect_named_unix - connect to the given named UNIX socket
> + *
> + * Return: The errno from connect(), or 0
> + */
> +static int test_connect_named_unix(int fd, const char *const path)
> +{
> +	struct sockaddr_un addr = {
> +		.sun_family = AF_UNIX,
> +	};
> +	strncpy(addr.sun_path, path, sizeof(addr.sun_path));
> +
> +	if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
> +		return errno;
> +	return 0;
> +}
> +
>  /* For named UNIX domain sockets, no IOCTL restrictions apply. */
>  TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
>  {
>  	const char *const path = file1_s1d1;
>  	int srv_fd, cli_fd, ruleset_fd;
> -	struct sockaddr_un srv_un = {
> -		.sun_family = AF_UNIX,
> -	};
> -	struct sockaddr_un cli_un = {
> -		.sun_family = AF_UNIX,
> -	};
>  	const struct landlock_ruleset_attr attr = {
>  		.handled_access_fs = LANDLOCK_ACCESS_FS_IOCTL_DEV,
>  	};
>  
>  	/* Sets up a server */
>  	ASSERT_EQ(0, unlink(path));
> -	srv_fd = socket(AF_UNIX, SOCK_STREAM, 0);
> -	ASSERT_LE(0, srv_fd);
> -
> -	strncpy(srv_un.sun_path, path, sizeof(srv_un.sun_path));
> -	ASSERT_EQ(0, bind(srv_fd, (struct sockaddr *)&srv_un, sizeof(srv_un)));
> -
> -	ASSERT_EQ(0, listen(srv_fd, 10 /* qlen */));
> +	srv_fd = set_up_named_unix_server(_metadata, SOCK_STREAM, path);
>  
>  	/* Enables Landlock. */
>  	ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
> @@ -4393,9 +4424,7 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
>  	cli_fd = socket(AF_UNIX, SOCK_STREAM, 0);
>  	ASSERT_LE(0, cli_fd);
>  
> -	strncpy(cli_un.sun_path, path, sizeof(cli_un.sun_path));
> -	ASSERT_EQ(0,
> -		  connect(cli_fd, (struct sockaddr *)&cli_un, sizeof(cli_un)));
> +	ASSERT_EQ(0, test_connect_named_unix(cli_fd, path));
>  
>  	/* FIONREAD and other IOCTLs should not be forbidden. */
>  	EXPECT_EQ(0, test_fionread_ioctl(cli_fd));
> @@ -4570,6 +4599,326 @@ TEST_F_FORK(ioctl, handle_file_access_file)
>  	ASSERT_EQ(0, close(file_fd));
>  }
>  
> +/* clang-format off */
> +FIXTURE(unix_socket) {};
> +
> +FIXTURE_SETUP(unix_socket) {};
> +
> +FIXTURE_TEARDOWN(unix_socket) {};
> +/* clang-format on */
> +
> +FIXTURE_VARIANT(unix_socket)
> +{
> +	const __u64 handled;
> +	const __u64 allowed;
> +	const __u64 handled2;
> +	const __u64 allowed2;
> +	const int sock_type;
> +	const int expected;
> +	const bool use_sendto;
> +	const bool server_in_same_domain;
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, stream_handled_not_allowed)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = 0,
> +	.sock_type = SOCK_STREAM,
> +	.expected = EACCES,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, stream_handled_and_allowed)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.sock_type = SOCK_STREAM,
> +	.expected = 0,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, dgram_handled_not_allowed)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = 0,
> +	.sock_type = SOCK_DGRAM,
> +	.expected = EACCES,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, dgram_handled_and_allowed)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.sock_type = SOCK_DGRAM,
> +	.expected = 0,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, dgram_sendto_handled_not_allowed)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = 0,
> +	.sock_type = SOCK_DGRAM,
> +	.use_sendto = true,
> +	.expected = EACCES,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, dgram_sendto_handled_and_allowed)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.sock_type = SOCK_DGRAM,
> +	.use_sendto = true,
> +	.expected = 0,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, seqpacket_handled_not_allowed)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = 0,
> +	.sock_type = SOCK_SEQPACKET,
> +	.expected = EACCES,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, seqpacket_handled_and_allowed)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.sock_type = SOCK_SEQPACKET,
> +	.expected = 0,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, stream_handled_not_allowed_and_same_scope)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = 0,
> +	.sock_type = SOCK_STREAM,
> +	.expected = 0,
> +	.server_in_same_domain = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, stream_handled_and_allowed_and_same_scope)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.sock_type = SOCK_STREAM,
> +	.expected = 0,
> +	.server_in_same_domain = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, dgram_handled_not_allowed_and_same_scope)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = 0,
> +	.sock_type = SOCK_DGRAM,
> +	.expected = 0,
> +	.server_in_same_domain = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, dgram_handled_and_allowed_and_same_scope)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.sock_type = SOCK_DGRAM,
> +	.expected = 0,
> +	.server_in_same_domain = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, dgram_sendto_handled_not_allowed_and_same_scope)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = 0,
> +	.sock_type = SOCK_DGRAM,
> +	.use_sendto = true,
> +	.expected = 0,
> +	.server_in_same_domain = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, dgram_sendto_handled_and_allowed_and_same_scope)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.sock_type = SOCK_DGRAM,
> +	.use_sendto = true,
> +	.expected = 0,
> +	.server_in_same_domain = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, seqpacket_handled_not_allowed_and_same_scope)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = 0,
> +	.sock_type = SOCK_SEQPACKET,
> +	.expected = 0,
> +	.server_in_same_domain = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, seqpacket_handled_and_allowed_and_same_scope)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.sock_type = SOCK_SEQPACKET,
> +	.expected = 0,
> +	.server_in_same_domain = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, stream_nested_domains_scope_path)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = 0,
> +	.server_in_same_domain = true,
> +	.handled2 = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed2 = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.sock_type = SOCK_STREAM,
> +	.expected = 0,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, stream_nested_domains_path_scope)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.server_in_same_domain = true,
> +	.handled2 = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed2 = 0,
> +	.sock_type = SOCK_STREAM,
> +	.expected = EACCES,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, stream_nested_domains_scope_scope)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = 0,
> +	.server_in_same_domain = true,
> +	.handled2 = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed2 = 0,
> +	.sock_type = SOCK_STREAM,
> +	.expected = EACCES,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_socket, stream_nested_domains_path_path)
> +{
> +	/* clang-format on */
> +	.handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.server_in_same_domain = true,
> +	.handled2 = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.allowed2 = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	.sock_type = SOCK_STREAM,
> +	.expected = 0,
> +};
> +
> +/*
> + * test_sendto_named_unix - sendto to the given named UNIX socket
> + *
> + * sendto() is equivalent to sendmsg() in this respect.
> + *
> + * Return: The errno from sendto(), or 0
> + */
> +static int test_sendto_named_unix(int fd, const char *const path)
> +{
> +	static const char buf[] = "dummy";
> +	struct sockaddr_un addr = {
> +		.sun_family = AF_UNIX,
> +	};
> +	strncpy(addr.sun_path, path, sizeof(addr.sun_path));
> +
> +	if (sendto(fd, buf, sizeof(buf), 0, (struct sockaddr *)&addr,
> +		   sizeof(addr)) == -1)
> +		return errno;
> +	return 0;
> +}
> +
> +TEST_F_FORK(unix_socket, test)
> +{
> +	const char *const path = "sock";
> +	int cli_fd, srv_fd, ruleset_fd, res;
> +	struct rule rules[] = {
> +		{
> +			.path = ".",
> +			.access = variant->allowed,
> +		},
> +		{},
> +	};
> +
> +	/* Sets up a server (in the case where the server is in the parent domain) */
> +	if (!variant->server_in_same_domain)
> +		srv_fd = set_up_named_unix_server(_metadata, variant->sock_type,
> +						  path);
> +
> +	/* Enables Landlock. */
> +	ruleset_fd = create_ruleset(_metadata, variant->handled, rules);
> +	ASSERT_LE(0, ruleset_fd);
> +	enforce_ruleset(_metadata, ruleset_fd);
> +	ASSERT_EQ(0, close(ruleset_fd));
> +
> +	/* Sets up a server (in the case where the server is in the same domain) */
> +	if (variant->server_in_same_domain)
> +		srv_fd = set_up_named_unix_server(_metadata, variant->sock_type,
> +						  path);
> +
> +	if (variant->handled2) {
> +		/* Enables Landlock another time, if needed. */
> +		rules[0].access = variant->allowed2;
> +		ruleset_fd =
> +			create_ruleset(_metadata, variant->handled2, rules);
> +		ASSERT_LE(0, ruleset_fd);
> +		enforce_ruleset(_metadata, ruleset_fd);
> +		ASSERT_EQ(0, close(ruleset_fd));
> +	}
> +
> +	/* Sets up a client connection to it */
> +	cli_fd = socket(AF_UNIX, variant->sock_type, 0);
> +	ASSERT_LE(0, cli_fd);
> +
> +	/* Connecting or sendto to the Unix socket is denied. */
> +	if (variant->use_sendto)
> +		res = test_sendto_named_unix(cli_fd, path);
> +	else
> +		res = test_connect_named_unix(cli_fd, path);
> +	EXPECT_EQ(variant->expected, res);
> +
> +	/* Clean up. */
> +	EXPECT_EQ(0, close(cli_fd));
> +	EXPECT_EQ(0, close(srv_fd));
> +	EXPECT_EQ(0, unlink(path));
> +}
> +
>  /* clang-format off */
>  FIXTURE(layout1_bind) {};
>  /* clang-format on */
> -- 
> 2.52.0
> 
> 

^ permalink raw reply

* Re: [PATCH] landlock: Add counted_by and fix comment in landlock_ruleset
From: Mickaël Salaün @ 2026-02-09 17:28 UTC (permalink / raw)
  To: Tingmao Wang; +Cc: Günther Noack, linux-security-module
In-Reply-To: <20260208235449.1124354-1-m@maowtm.org>

Thanks!

On Sun, Feb 08, 2026 at 11:54:48PM +0000, Tingmao Wang wrote:
> For a domain, this array stores the access masks for each layer (of
> which there are num_layers of them).  For an unmerged ruleset, we have
> one "layer", and one element in this array.  This annotation serves as
> useful documentation.
> 
> This also removes a comment saying that num_layers = 0 for unmerged
> rulesets, which is incorrect (it is 1).
> 
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
> 
> I'm not sure if I should add a Fixes tag, but if I should had, it would
> be ae271c1b14 ("landlock: Add ruleset and domain management").
> 
>  security/landlock/ruleset.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 9d6dc632684c..7005840ac641 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -168,8 +168,7 @@ struct landlock_ruleset {
>  			/**
>  			 * @num_layers: Number of layers that are used in this
>  			 * ruleset.  This enables to check that all the layers
> -			 * allow an access request.  A value of 0 identifies a
> -			 * non-merged ruleset (i.e. not a domain).
> +			 * allow an access request.
>  			 */
>  			u32 num_layers;
>  			/**
> @@ -184,7 +183,8 @@ struct landlock_ruleset {
>  			 * layers are set once and never changed for the
>  			 * lifetime of the ruleset.
>  			 */
> -			struct access_masks access_masks[];
> +			struct access_masks
> +				access_masks[] __counted_by(num_layers);
>  		};
>  	};
>  };
> 
> base-commit: f179e1859c711214412876c57f56f9b0cfb13264
> -- 
> 2.53.0
> 

^ permalink raw reply

* Re: [PATCH v3 1/5] lsm: Add hook security_unix_find
From: Paul Moore @ 2026-02-09 17:09 UTC (permalink / raw)
  To: Günther Noack
  Cc: Günther Noack, John Johansen, Tingmao Wang,
	Mickaël Salaün, James Morris, Serge E . Hallyn,
	Justin Suess, linux-security-module, Samasth Norway Ananda,
	Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
	Demi Marie Obenour, Alyssa Ross, Jann Horn, Tahera Fahimi,
	Simon Horman, netdev, Alexander Viro, Christian Brauner
In-Reply-To: <aYMenaSmBkAsFowd@google.com>

On Wed, Feb 4, 2026 at 5:25 AM Günther Noack <gnoack@google.com> wrote:
>
> Paul:
>
> You have previously said that you would like hooks to be generic and
> ideally reflect the arguments of the same function that they are
> called from [3].

To clarify, I didn't say that it is generally ideal for the LSM hook
to reflect the arguments of the calling function; while that might be
a good starting point, we have plenty of examples where that is not
desirable.  In this particular case I said it seems like it would be a
good idea to pass the "type" and "flags" parameters from the caller to
the LSM hook.

> Q: Would it be acceptable to change the hook arguments, if we can then
> avoid passing additional data between hooks through that side-storage?

If you're passing the sock, I think we can skip passing the type,
however, I could envision someone wanting the path in addition to just
the sock, but let's wait to hear back from the AppArmor folks.

-- 
paul-moore.com

^ permalink raw reply

* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Lorenzo Stoakes @ 2026-02-09 15:22 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: Alice Ryhl, Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
	Christian Brauner, Jan Kara, Paul Moore, James Morris,
	Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
	Roman Gushchin, Muchun Song, Liam R. Howlett, Vlastimil Babka,
	Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Miguel Ojeda,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross, Danilo Krummrich, kernel-team,
	linux-fsdevel, linux-kernel, linux-security-module, linux-mm,
	rust-for-linux
In-Reply-To: <ba5cb90a-cd4f-496e-a665-cc323ec598ab@kernel.org>

On Thu, Feb 05, 2026 at 01:30:16PM +0100, David Hildenbrand (Arm) wrote:
> On 2/5/26 13:24, Lorenzo Stoakes wrote:
> > On Thu, Feb 05, 2026 at 12:19:22PM +0000, Alice Ryhl wrote:
> > > On Thu, Feb 05, 2026 at 01:13:57PM +0100, David Hildenbrand (arm) wrote:
> > > >
> > > > Could Rust just use zap_vma_ptes() or does it want to zap things in VMAs
> > > > that are not VM_PFNMAP?
> > >
> > > The VMA is VM_MIXEDMAP, not VM_PFNMAP.
> >
> > OK this smells like David's cleanup could extend it to allow for
> > VM_MIXEDMAP :) then we solve the export problem.
>
> My thinking ... and while at it, gonna remove these functions to make them a
> bit more ... consistent in naming.

Sounds good :)

>
> --
> Cheers,
>
> David

Cheers, Lorenzo

^ permalink raw reply

* Re: [PATCH 1/5] export file_close_fd and task_work_add
From: Lorenzo Stoakes @ 2026-02-09 15:21 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
	Christian Brauner, Jan Kara, Paul Moore, James Morris,
	Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
	Roman Gushchin, Muchun Song, David Hildenbrand, Liam R. Howlett,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
	kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
	linux-mm, rust-for-linux
In-Reply-To: <aYSfBJA4hR4shPfI@google.com>

On Thu, Feb 05, 2026 at 01:45:40PM +0000, Alice Ryhl wrote:
> On Thu, Feb 05, 2026 at 11:53:19AM +0000, Lorenzo Stoakes wrote:
> > On Thu, Feb 05, 2026 at 11:42:46AM +0000, Alice Ryhl wrote:
> > > On Thu, Feb 05, 2026 at 11:20:33AM +0000, Lorenzo Stoakes wrote:
> > > > On Thu, Feb 05, 2026 at 10:51:26AM +0000, Alice Ryhl wrote:
> > > > > This exports the functionality needed by Binder to close file
> > > > > descriptors.
> > > > >
> > > > > When you send a fd over Binder, what happens is this:
> > > > >
> > > > > 1. The sending process turns the fd into a struct file and stores it in
> > > > >    the transaction object.
> > > > > 2. When the receiving process gets the message, the fd is installed as a
> > > > >    fd into the current process.
> > > > > 3. When the receiving process is done handling the message, it tells
> > > > >    Binder to clean up the transaction. As part of this, fds embedded in
> > > > >    the transaction are closed.
> > > > >
> > > > > Note that it was not always implemented like this. Previously the
> > > > > sending process would install the fd directly into the receiving proc in
> > > > > step 1, but as discussed previously [1] this is not ideal and has since
> > > > > been changed so that fd install happens during receive.
> > > > >
> > > > > The functions being exported here are for closing the fd in step 3. They
> > > > > are required because closing a fd from an ioctl is in general not safe.
> > > > > This is to meet the requirements for using fdget(), which is used by the
> > > > > ioctl framework code before calling into the driver's implementation of
> > > > > the ioctl. Binder works around this with this sequence of operations:
> > > > >
> > > > > 1. file_close_fd()
> > > > > 2. get_file()
> > > > > 3. filp_close()
> > > > > 4. task_work_add(current, TWA_RESUME)
> > > > > 5. <binder returns from ioctl>
> > > > > 6. fput()
> > > > >
> > > > > This ensures that when fput() is called in the task work, the fdget()
> > > > > that the ioctl framework code uses has already been fdput(), so if the
> > > > > fd being closed happens to be the same fd, then the fd is not closed
> > > > > in violation of the fdget() rules.
> > > >
> > > > I'm not really familiar with this mechanism but you're already talking about
> > > > this being a workaround so strikes me the correct thing to do is to find a way
> > > > to do this in the kernel sensibly rather than exporting internal implementation
> > > > details and doing it in binder.
> > >
> > > I did previously submit a patch that implemented this logic outside of
> > > Binder, but I was advised to move it into Binder.
> >
> > Right yeah that's just odd to me, we really do not want to be adding internal
> > implementation details to drivers.
> >
> > This is based on bitter experience of bugs being caused by drivers abusing every
> > interface they get, which is basically exactly what always happens, sadly.
> >
> > And out-of-tree is heavily discouraged.
> >
> > Also can we use EXPORT_SYMBOL_FOR_MODULES() for anything we do need to export to
> > make it explicitly only for binder, perhaps?
> >
> > >
> > > But I'm happy to submit a patch to extract this logic into some sort of
> > > close_fd_safe() method that can be called even if said fd is currently
> > > held using fdget().
> >
> > Yup, especially given Christian's view on the kernel task export here I think
> > that's a more sensible approach.
> >
> > But obviously I defer the sensible-ness of this to him as I am but an mm dev :)
>
> Quick sketch of how this would look:
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index adde1e40cccd..6fb7175ff69b 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -64,7 +64,6 @@
>  #include <linux/spinlock.h>
>  #include <linux/ratelimit.h>
>  #include <linux/syscalls.h>
> -#include <linux/task_work.h>
>  #include <linux/sizes.h>
>  #include <linux/ktime.h>
>
> @@ -1962,68 +1961,6 @@ static bool binder_validate_fixup(struct binder_proc *proc,
>  	return (fixup_offset >= last_min_offset);
>  }
>
> -/**
> - * struct binder_task_work_cb - for deferred close
> - *
> - * @twork:                callback_head for task work
> - * @file:                 file to close
> - *
> - * Structure to pass task work to be handled after
> - * returning from binder_ioctl() via task_work_add().
> - */
> -struct binder_task_work_cb {
> -	struct callback_head twork;
> -	struct file *file;
> -};
> -
> -/**
> - * binder_do_fd_close() - close list of file descriptors
> - * @twork:	callback head for task work
> - *
> - * It is not safe to call ksys_close() during the binder_ioctl()
> - * function if there is a chance that binder's own file descriptor
> - * might be closed. This is to meet the requirements for using
> - * fdget() (see comments for __fget_light()). Therefore use
> - * task_work_add() to schedule the close operation once we have
> - * returned from binder_ioctl(). This function is a callback
> - * for that mechanism and does the actual ksys_close() on the
> - * given file descriptor.
> - */
> -static void binder_do_fd_close(struct callback_head *twork)
> -{
> -	struct binder_task_work_cb *twcb = container_of(twork,
> -			struct binder_task_work_cb, twork);
> -
> -	fput(twcb->file);
> -	kfree(twcb);
> -}
> -
> -/**
> - * binder_deferred_fd_close() - schedule a close for the given file-descriptor
> - * @fd:		file-descriptor to close
> - *
> - * See comments in binder_do_fd_close(). This function is used to schedule
> - * a file-descriptor to be closed after returning from binder_ioctl().
> - */
> -static void binder_deferred_fd_close(int fd)
> -{
> -	struct binder_task_work_cb *twcb;
> -
> -	twcb = kzalloc(sizeof(*twcb), GFP_KERNEL);
> -	if (!twcb)
> -		return;
> -	init_task_work(&twcb->twork, binder_do_fd_close);
> -	twcb->file = file_close_fd(fd);
> -	if (twcb->file) {
> -		// pin it until binder_do_fd_close(); see comments there
> -		get_file(twcb->file);
> -		filp_close(twcb->file, current->files);
> -		task_work_add(current, &twcb->twork, TWA_RESUME);
> -	} else {
> -		kfree(twcb);
> -	}
> -}
> -
>  static void binder_transaction_buffer_release(struct binder_proc *proc,
>  					      struct binder_thread *thread,
>  					      struct binder_buffer *buffer,
> @@ -2183,7 +2120,10 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
>  						offset, sizeof(fd));
>  				WARN_ON(err);
>  				if (!err) {
> -					binder_deferred_fd_close(fd);
> +					/*
> +					 * Intentionally ignore EBADF errors here.
> +					 */
> +					close_fd_safe(fd, GFP_KERNEL | __GFP_NOFAIL);
>  					/*
>  					 * Need to make sure the thread goes
>  					 * back to userspace to complete the
> diff --git a/fs/file.c b/fs/file.c
> index 0a4f3bdb2dec..58e3825e846c 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -21,6 +21,7 @@
>  #include <linux/rcupdate.h>
>  #include <linux/close_range.h>
>  #include <linux/file_ref.h>
> +#include <linux/task_work.h>
>  #include <net/sock.h>
>  #include <linux/init_task.h>
>
> @@ -1525,3 +1526,47 @@ int iterate_fd(struct files_struct *files, unsigned n,
>  	return res;
>  }
>  EXPORT_SYMBOL(iterate_fd);
> +
> +struct close_fd_safe_task_work {
> +	struct callback_head twork;
> +	struct file *file;
> +};
> +
> +static void close_fd_safe_callback(struct callback_head *twork)
> +{
> +	struct close_fd_safe_task_work *twcb = container_of(twork,
> +			struct close_fd_safe_task_work, twork);
> +
> +	fput(twcb->file);
> +	kfree(twcb);
> +}
> +
> +/**
> + * close_fd_safe - close the given fd
> + * @fd: file descriptor to close
> + * @flags: gfp flags for allocation of task work
> + *
> + * This closes an fd. Unlike close_fd(), this may be used even if the fd is
> + * currently held with fdget().
> + *
> + * Returns: 0 or an error code
> + */
> +int close_fd_safe(unsigned int fd, gfp_t flags)
> +{
> +	struct close_fd_safe_task_work *twcb;
> +
> +	twcb = kzalloc(sizeof(*twcb), flags);
> +	if (!twcb)
> +		return -ENOMEM;
> +	init_task_work(&twcb->twork, close_fd_safe_callback);
> +	twcb->file = file_close_fd(fd);
> +	if (!twcb->file) {
> +		kfree(twcb);
> +		return -EBADF;
> +	}
> +
> +	get_file(twcb->file);
> +	filp_close(twcb->file, current->files);
> +	task_work_add(current, &twcb->twork, TWA_RESUME);
> +	return 0;
> +}

Would need an EXPORT_SYMBOL_FOR_MODULES(...) here right?

> diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
> index c45306a9f007..1c99a56c0cdf 100644
> --- a/include/linux/fdtable.h
> +++ b/include/linux/fdtable.h
> @@ -111,6 +111,7 @@ int iterate_fd(struct files_struct *, unsigned,
>  		const void *);
>
>  extern int close_fd(unsigned int fd);
> +extern int close_fd_safe(unsigned int fd, gfp_t flags);

One nit, generally well in mm anyway we avoid the 'extern' and remove them as we
go. Not sure about vfs actually though?

>  extern struct file *file_close_fd(unsigned int fd);
>
>  extern struct kmem_cache *files_cachep;

I mean this is essentially taking what's in binder and making it a general
thing, so needs Christian's input on whether this is sensible I think :)

Cheers, Lorenzo

^ permalink raw reply

* Re: [PATCH] xfrm: kill xfrm_dev_{state,policy}_flush_secctx_check()
From: Tetsuo Handa @ 2026-02-09 14:26 UTC (permalink / raw)
  To: Steffen Klassert, Paul Moore, SELinux, linux-security-module
  Cc: Herbert Xu, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Network Development
In-Reply-To: <aYnDWbxo-jAzR4ca@secunet.com>

On 2026/02/09 20:22, Steffen Klassert wrote:
> On Mon, Feb 09, 2026 at 07:02:47PM +0900, Tetsuo Handa wrote:
>> On 2026/02/09 18:25, Steffen Klassert wrote:
>>> The problem is that, with adding IPsec offloads to netdevices, security
>>> critical resources came into the netdevices. Someone who has no
>>> capabilities to delete xfrm states or xfrm policies should not be able
>>> to unregister the netdevice if xfrm states or xfrm policies are
>>> offloaded. Unfortunately, unregistering can't be canceled at this stage
>>> anymore. So I think we need some netdevice unregistration hook for
>>> the LSM subsystem so it can check for xfrm states or xfrm policies
>>> and refuse the unregistration before we actually start to remove
>>> the device.
>>
>> Unfortunately, unregistering is not always triggered by a user's request. ;-)
> 
> As far as I remember, a security context is not always tied to a
> user request. It can also be attached to system tasks or objects.

That is not what I wanted to say. There are at least three routes (listed below)
that can trigger xfrm_dev_unregister() path. You could insert LSM hooks into the
netlink_sendmsg() route and the del_device_store() route, but the cleanup_net()
route is a result of tear-down action which is too late to insert LSM hooks.

The NETDEV_UNREGISTER path can be triggered by just doing "unshare -n ip addr show"
(i.e. implicit cleanup of a network namespace due to termination of init process in
that namespace). We are not allowed to reject the cleanup_net() route.

----------
xfrm_dev_state_flush_secctx_check: LSM policy is rejecting this operation.
CPU: 0 UID: 0 PID: 16195 Comm: syz.3.3878 Tainted: G             L      syzkaller #0 PREEMPT(full) 
Tainted: [L]=SOFTLOCKUP
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/24/2026
Call Trace:
 <TASK>
 dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
 xfrm_dev_state_flush_secctx_check net/xfrm/xfrm_state.c:900 [inline]
 xfrm_dev_state_flush+0x5fa/0x740 net/xfrm/xfrm_state.c:971
 xfrm_dev_unregister net/xfrm/xfrm_device.c:549 [inline]
 xfrm_dev_event+0x1bc/0x3f0 net/xfrm/xfrm_device.c:570
 notifier_call_chain+0x1be/0x400 kernel/notifier.c:85
 call_netdevice_notifiers_extack net/core/dev.c:2281 [inline]
 call_netdevice_notifiers net/core/dev.c:2295 [inline]
 netdev_wait_allrefs_any net/core/dev.c:11589 [inline]
 netdev_run_todo+0x778/0x1130 net/core/dev.c:11710
 nsim_destroy+0x3ae/0x680 drivers/net/netdevsim/netdev.c:1190
 __nsim_dev_port_del+0x14d/0x1b0 drivers/net/netdevsim/dev.c:1529
 nsim_dev_port_del_all drivers/net/netdevsim/dev.c:1541 [inline]
 nsim_dev_reload_destroy+0x288/0x490 drivers/net/netdevsim/dev.c:1765
 nsim_dev_reload_down+0x8a/0xc0 drivers/net/netdevsim/dev.c:1039
 devlink_reload+0x1d1/0x8e0 net/devlink/dev.c:461
 devlink_nl_reload_doit+0xaaa/0xc80 net/devlink/dev.c:584
 genl_family_rcv_msg_doit+0x22a/0x330 net/netlink/genetlink.c:1115
 genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
 genl_rcv_msg+0x61c/0x7a0 net/netlink/genetlink.c:1210
 netlink_rcv_skb+0x232/0x4b0 net/netlink/af_netlink.c:2550
 genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
 netlink_unicast_kernel net/netlink/af_netlink.c:1318 [inline]
 netlink_unicast+0x80f/0x9b0 net/netlink/af_netlink.c:1344
 netlink_sendmsg+0x813/0xb40 net/netlink/af_netlink.c:1894
 sock_sendmsg_nosec+0x18f/0x1d0 net/socket.c:737
 __sock_sendmsg net/socket.c:752 [inline]
 ____sys_sendmsg+0x589/0x8c0 net/socket.c:2610
 ___sys_sendmsg+0x2a5/0x360 net/socket.c:2664
 __sys_sendmsg net/socket.c:2696 [inline]
 __do_sys_sendmsg net/socket.c:2701 [inline]
 __se_sys_sendmsg net/socket.c:2699 [inline]
 __x64_sys_sendmsg+0x1bd/0x2a0 net/socket.c:2699
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x14d/0xf80 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7feb10f9aeb9
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:00007feb11efc028 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007feb11215fa0 RCX: 00007feb10f9aeb9
RDX: 0000000006048800 RSI: 0000200000000080 RDI: 0000000000000005
RBP: 00007feb11008c1f R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007feb11216038 R14: 00007feb11215fa0 R15: 00007ffdd0b07b18
 </TASK>
----------

----------
xfrm_dev_state_flush_secctx_check: LSM policy is rejecting this operation.
CPU: 1 UID: 0 PID: 11340 Comm: kworker/u8:17 Tainted: G             L      syzkaller #0 PREEMPT(full) 
Tainted: [L]=SOFTLOCKUP
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/24/2026
Workqueue: netns cleanup_net
Call Trace:
 <TASK>
 dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
 xfrm_dev_state_flush_secctx_check net/xfrm/xfrm_state.c:900 [inline]
 xfrm_dev_state_flush+0x5fa/0x740 net/xfrm/xfrm_state.c:971
 xfrm_dev_unregister net/xfrm/xfrm_device.c:549 [inline]
 xfrm_dev_event+0x1bc/0x3f0 net/xfrm/xfrm_device.c:570
 notifier_call_chain+0x1be/0x400 kernel/notifier.c:85
 call_netdevice_notifiers_extack net/core/dev.c:2281 [inline]
 call_netdevice_notifiers net/core/dev.c:2295 [inline]
 netdev_wait_allrefs_any net/core/dev.c:11589 [inline]
 netdev_run_todo+0x778/0x1130 net/core/dev.c:11710
 nsim_destroy+0x3ae/0x680 drivers/net/netdevsim/netdev.c:1190
 __nsim_dev_port_del+0x14d/0x1b0 drivers/net/netdevsim/dev.c:1529
 nsim_dev_port_del_all drivers/net/netdevsim/dev.c:1541 [inline]
 nsim_dev_reload_destroy+0x288/0x490 drivers/net/netdevsim/dev.c:1765
 nsim_dev_reload_down+0x8a/0xc0 drivers/net/netdevsim/dev.c:1039
 devlink_reload+0x1d1/0x8e0 net/devlink/dev.c:461
 devlink_pernet_pre_exit+0x1e6/0x3f0 net/devlink/core.c:509
 ops_pre_exit_list net/core/net_namespace.c:161 [inline]
 ops_undo_list+0x187/0x940 net/core/net_namespace.c:234
 cleanup_net+0x4df/0x7b0 net/core/net_namespace.c:696
 process_one_work+0x949/0x1650 kernel/workqueue.c:3279
 process_scheduled_works kernel/workqueue.c:3362 [inline]
 worker_thread+0xb46/0x1140 kernel/workqueue.c:3443
 kthread+0x388/0x470 kernel/kthread.c:467
 ret_from_fork+0x51e/0xb90 arch/x86/kernel/process.c:158
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
 </TASK>
----------

----------
xfrm_dev_state_flush_secctx_check: LSM policy is rejecting this operation.
CPU: 0 UID: 0 PID: 18368 Comm: syz-executor Tainted: G             L      syzkaller #0 PREEMPT(full) 
Tainted: [L]=SOFTLOCKUP
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/24/2026
Call Trace:
 <TASK>
 dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
 xfrm_dev_state_flush_secctx_check net/xfrm/xfrm_state.c:900 [inline]
 xfrm_dev_state_flush+0x5fa/0x740 net/xfrm/xfrm_state.c:971
 xfrm_dev_unregister net/xfrm/xfrm_device.c:549 [inline]
 xfrm_dev_event+0x1bc/0x3f0 net/xfrm/xfrm_device.c:570
 notifier_call_chain+0x1be/0x400 kernel/notifier.c:85
 call_netdevice_notifiers_extack net/core/dev.c:2281 [inline]
 call_netdevice_notifiers net/core/dev.c:2295 [inline]
 netdev_wait_allrefs_any net/core/dev.c:11589 [inline]
 netdev_run_todo+0x778/0x1130 net/core/dev.c:11710
 nsim_destroy+0x3ae/0x680 drivers/net/netdevsim/netdev.c:1190
 __nsim_dev_port_del+0x14d/0x1b0 drivers/net/netdevsim/dev.c:1529
 nsim_dev_port_del_all drivers/net/netdevsim/dev.c:1541 [inline]
 nsim_dev_reload_destroy+0x288/0x490 drivers/net/netdevsim/dev.c:1765
 nsim_drv_remove+0x58/0x170 drivers/net/netdevsim/dev.c:1780
 device_remove drivers/base/dd.c:571 [inline]
 __device_release_driver drivers/base/dd.c:1284 [inline]
 device_release_driver_internal+0x46f/0x860 drivers/base/dd.c:1307
 bus_remove_device+0x34d/0x440 drivers/base/bus.c:616
 device_del+0x527/0x8f0 drivers/base/core.c:3878
 device_unregister+0x21/0xf0 drivers/base/core.c:3919
 nsim_bus_dev_del drivers/net/netdevsim/bus.c:491 [inline]
 del_device_store+0x2b0/0x370 drivers/net/netdevsim/bus.c:244
 kernfs_fop_write_iter+0x3af/0x540 fs/kernfs/file.c:352
 new_sync_write fs/read_write.c:595 [inline]
 vfs_write+0x61d/0xb90 fs/read_write.c:688
 ksys_write+0x150/0x270 fs/read_write.c:740
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x14d/0xf80 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fd13375b78e
Code: 08 0f 85 a5 a8 ff ff 49 89 fb 48 89 f0 48 89 d7 48 89 ce 4c 89 c2 4d 89 ca 4c 8b 44 24 08 4c 8b 4c 24 10 4c 89 5c 24 08 0f 05 <c3> 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 80 00 00 00 00 48 83 ec 08
RSP: 002b:00007ffc52b936a8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000555567157500 RCX: 00007fd13375b78e
RDX: 0000000000000001 RSI: 00007ffc52b93730 RDI: 0000000000000005
RBP: 00007fd133808a88 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007ffc52b93730 R14: 00007fd134544620 R15: 0000000000000003
 </TASK>
----------



> 
>> For example, we don't check permission for unmount when a mount is deleted
>> due to teardown of a mount namespace. I wonder why you want to check permission
>> for unregistering a net_device when triggered by a teardown path.
> 
> I just try to find out what's the right thing to do here.
> If a policy goes away, packets that match this policy will
> find another path through the network stack. As best, they
> are dropped somewhere, but they can also leave on some other
> device without encryption. A LSM that implements xfrm hooks
> must be able to check the permission to delete the xfrm policy
> or state.

Do you mean that calling xfrm_dev_down()/xfrm_dev_unregister() might
result in network traffic to be sent in cleartext ?

If yes, we need to consider updating the other patch at
https://lkml.kernel.org/r/20260202123655.GK34749@unreal to replace
the NETDEV_UNREGISTER net_device with the blackhole_netdev. (That is,
xfrm_dev_{state,policy}_flush() does not actually delete a state/policy
but instead updates that state/policy to behave as a blackhole. Then,
we won't need to call LSM hooks because we no longer delete).

Also, we need to consider changing xfrm_dev_down() to no-op, for just doing
e.g. "ip link set ens160 down; ip link set ens160 up" (which triggers
NETDEV_DOWN event and NETDEV_UP event) might result in network traffic
to be sent in cleartext because currently xfrm_dev_down() can delete a
state/policy. Such behavior might not what the administrator is expecting.



> 
>>
>>>
>>> The same happened btw. when xfrm was made per network namespace.
>>> Here we just leak the xfrm states and xfrm policies if some
>>> LSM refuses to remove them.
>>>
>>> I guess we need a solution for both cases.
>>
>> Is replacing the NETDEV_UNREGISTER net_device with the blackhole_netdev applicable
>> ( https://elixir.bootlin.com/linux/v6.19-rc5/source/net/xfrm/xfrm_policy.c#L3948 ) ?
>> If no, there is no choice but break SELinux's expectation.
> 
> That could be an option to not accidentally send out
> unencrypted packets. But finding the right place for
> these checks would be preferable IMO.

Can we have such giant lock if you found the right place for these checks
( https://lkml.kernel.org/r/f9b88268-03dc-4356-8b31-0bab73cc9b1e@I-love.SAKURA.ne.jp ) ?


^ permalink raw reply

* Re: [PATCH v2 05/13] mm: add basic VMA flag operation helper functions
From: Lorenzo Stoakes @ 2026-02-09 14:04 UTC (permalink / raw)
  To: Pedro Falcato
  Cc: Andrew Morton, Jarkko Sakkinen, Dave Hansen, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, H . Peter Anvin, Arnd Bergmann,
	Greg Kroah-Hartman, Dan Williams, Vishal Verma, Dave Jiang,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Tvrtko Ursulin, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Alexander Viro, Christian Brauner, Jan Kara,
	Benjamin LaHaise, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
	Sandeep Dhavale, Hongbo Li, Chunhai Guo, Theodore Ts'o,
	Andreas Dilger, Muchun Song, Oscar Salvador, David Hildenbrand,
	Konstantin Komarov, Mike Marshall, Martin Brandenburg, Tony Luck,
	Reinette Chatre, Dave Martin, James Morse, Babu Moger,
	Carlos Maiolino, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
	Matthew Wilcox, Liam R . Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Hugh Dickins, Baolin Wang,
	Zi Yan, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
	Lance Yang, Jann Horn, David Howells, Paul Moore, James Morris,
	Serge E . Hallyn, Yury Norov, Rasmus Villemoes, linux-sgx,
	linux-kernel, nvdimm, linux-cxl, dri-devel, intel-gfx,
	linux-fsdevel, linux-aio, linux-erofs, linux-ext4, linux-mm,
	ntfs3, devel, linux-xfs, keyrings, linux-security-module,
	Jason Gunthorpe
In-Reply-To: <vrbggto75ugvpa5wtugmayr7yops6cnvygit42f2md646y6qnx@3vzc7taleijw>

On Fri, Feb 06, 2026 at 05:35:49PM +0000, Pedro Falcato wrote:
> On Thu, Jan 22, 2026 at 04:06:14PM +0000, Lorenzo Stoakes wrote:
> > Now we have the mk_vma_flags() macro helper which permits easy
> > specification of any number of VMA flags, add helper functions which
> > operate with vma_flags_t parameters.
> >
> > This patch provides vma_flags_test[_mask](), vma_flags_set[_mask]() and
> > vma_flags_clear[_mask]() respectively testing, setting and clearing flags
> > with the _mask variants accepting vma_flag_t parameters, and the non-mask
> > variants implemented as macros which accept a list of flags.
> >
> > This allows us to trivially test/set/clear aggregate VMA flag values as
> > necessary, for instance:
> >
> > 	if (vma_flags_test(&flags, VMA_READ_BIT, VMA_WRITE_BIT))
> > 		goto readwrite;
>
> I'm not a huge fan of the _test ambiguity here, but more words makes it uglier :/
> I think I can live with it though.

Yeah, as discussed on IRC it's a bit of a trade off here unfortunately.

I don't love having the _BIT stuff there but is necessary for now I feel until
VM_xxx flags are finally fully deprecated.

>
> >
> > 	vma_flags_set(&flags, VMA_READ_BIT, VMA_WRITE_BIT);
> >
> > 	vma_flags_clear(&flags, VMA_READ_BIT, VMA_WRITE_BIT);
> >
>
> The variadic-ness here is very nice though.

Thanks!

>
> > We also add a function for testing that ALL flags are set for convenience,
> > e.g.:
> >
> > 	if (vma_flags_test_all(&flags, VMA_READ_BIT, VMA_MAYREAD_BIT)) {
> > 		/* Both READ and MAYREAD flags set */
> > 		...
> > 	}
> >
> > The compiler generates optimal assembly for each such that they behave as
> > if the caller were setting the bitmap flags manually.
> >
> > This is important for e.g. drivers which manipulate flag values rather than
> > a VMA's specific flag values.
> >
> > We also add helpers for testing, setting and clearing flags for VMA's and
> > VMA descriptors to reduce boilerplate.
> >
> > Also add the EMPTY_VMA_FLAGS define to aid initialisation of empty flags.
> >
> > Finally, update the userland VMA tests to add the helpers there so they can
> > be utilised as part of userland testing.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>

Thanks (also for other tags :P)

>
> --
> Pedro

Cheers, Lorenzo

^ permalink raw reply

* Re: [PATCH v2 03/13] mm: add mk_vma_flags() bitmap flag macro helper
From: Lorenzo Stoakes @ 2026-02-09 14:02 UTC (permalink / raw)
  To: Pedro Falcato
  Cc: Andrew Morton, Jarkko Sakkinen, Dave Hansen, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, H . Peter Anvin, Arnd Bergmann,
	Greg Kroah-Hartman, Dan Williams, Vishal Verma, Dave Jiang,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Tvrtko Ursulin, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Alexander Viro, Christian Brauner, Jan Kara,
	Benjamin LaHaise, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
	Sandeep Dhavale, Hongbo Li, Chunhai Guo, Theodore Ts'o,
	Andreas Dilger, Muchun Song, Oscar Salvador, David Hildenbrand,
	Konstantin Komarov, Mike Marshall, Martin Brandenburg, Tony Luck,
	Reinette Chatre, Dave Martin, James Morse, Babu Moger,
	Carlos Maiolino, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
	Matthew Wilcox, Liam R . Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Hugh Dickins, Baolin Wang,
	Zi Yan, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
	Lance Yang, Jann Horn, David Howells, Paul Moore, James Morris,
	Serge E . Hallyn, Yury Norov, Rasmus Villemoes, linux-sgx,
	linux-kernel, nvdimm, linux-cxl, dri-devel, intel-gfx,
	linux-fsdevel, linux-aio, linux-erofs, linux-ext4, linux-mm,
	ntfs3, devel, linux-xfs, keyrings, linux-security-module,
	Jason Gunthorpe
In-Reply-To: <mflwgdnyipdf4reufmbx7qarjcgouct5coe2bllticrabcu6rt@vf3bvmpunimw>

On Fri, Feb 06, 2026 at 05:14:10PM +0000, Pedro Falcato wrote:
> On Thu, Jan 22, 2026 at 04:06:12PM +0000, Lorenzo Stoakes wrote:
> > This patch introduces the mk_vma_flags() macro helper to allow easy
> > manipulation of VMA flags utilising the new bitmap representation
> > implemented of VMA flags defined by the vma_flags_t type.
> >
> > It is a variadic macro which provides a bitwise-or'd representation of all
> > of each individual VMA flag specified.
> >
> > Note that, while we maintain VM_xxx flags for backwards compatibility until
> > the conversion is complete, we define VMA flags of type vma_flag_t using
> > VMA_xxx_BIT to avoid confusing the two.
> >
> > This helper macro therefore can be used thusly:
> >
> > vma_flags_t flags = mk_vma_flags(VMA_READ_BIT, VMA_WRITE_BIT);
> >
> > We allow for up to 5 flags to specified at a time which should accommodate
> > all current kernel uses of combined VMA flags.
> >
>
> How do you allow up to 5 flags? I don't see any such limitation in the code?

Yeah oops :) This is from a previous implementation.

Andrew could you drop this paragraph? Thanks!

>
> > Testing has demonstrated that the compiler optimises this code such that it
> > generates the same assembly utilising this macro as it does if the flags
> > were specified manually, for instance:
> >
> > vma_flags_t get_flags(void)
> > {
> > 	return mk_vma_flags(VMA_READ_BIT, VMA_WRITE_BIT, VMA_EXEC_BIT);
> > }
> >
> > Generates the same code as:
> >
> > vma_flags_t get_flags(void)
> > {
> > 	vma_flags_t flags;
> >
> > 	vma_flags_clear_all(&flags);
> > 	vma_flag_set(&flags, VMA_READ_BIT);
> > 	vma_flag_set(&flags, VMA_WRITE_BIT);
> > 	vma_flag_set(&flags, VMA_EXEC_BIT);
> >
> > 	return flags;
> > }
> >
> > And:
> >
> > vma_flags_t get_flags(void)
> > {
> > 	vma_flags_t flags;
> > 	unsigned long *bitmap = ACCESS_PRIVATE(&flags, __vma_flags);
> >
> > 	*bitmap = 1UL << (__force int)VMA_READ_BIT;
> > 	*bitmap |= 1UL << (__force int)VMA_WRITE_BIT;
> > 	*bitmap |= 1UL << (__force int)VMA_EXEC_BIT;
> >
> > 	return flags;
> > }
> >
> > That is:
> >
> > get_flags:
> >         movl    $7, %eax
> >         ret
> >
> > Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>

Thanks!

>
> --
> Pedro

Cheers, Lorenzo

^ permalink raw reply

* Re: [RFC PATCH] fs/pidfs: Add permission check to pidfd_info()
From: Christian Brauner @ 2026-02-09 14:01 UTC (permalink / raw)
  To: danieldurning.work
  Cc: linux-fsdevel, linux-security-module, selinux, viro, jack, paul,
	stephen.smalley.work, omosnace, Oleg Nesterov
In-Reply-To: <20260206180248.12418-1-danieldurning.work@gmail.com>

On Fri, Feb 06, 2026 at 06:02:48PM +0000, danieldurning.work@gmail.com wrote:
> From: Daniel Durning <danieldurning.work@gmail.com>
> 
> Added a permission check to pidfd_info(). Originally, process info
> could be retrieved with a pidfd even if proc was mounted with hidepid
> enabled, allowing pidfds to be used to bypass those protections. We
> now call ptrace_may_access() to perform some DAC checking as well
> as call the appropriate LSM hook.
> 
> The downside to this approach is that there are now more restrictions
> on accessing this info from a pidfd than when just using proc (without
> hidepid). I am open to suggestions if anyone can think of a better way
> to handle this.

This isn't really workable since this would regress userspace quite a
bit. I think we need a different approach. I've given it some thought
and everything's kinda ugly but this might work.

In struct pid_namespace record whether anyone ever mounted a procfs
with hidepid turned on for this pidns. In pidfd_info() we check whether
hidepid was ever turned on. If it wasn't we're done and can just return
the info. This will be the common case. If hidepid was ever turned on
use kern_path("/proc") to lookup procfs. If not found check
ptrace_may_access() to decide whether to return the info or not. If
/proc is found check it's hidepid settings and make a decision based on
that.

You can probably reorder this to call ptrace_may_access() first and then
do the procfs lookup dance. Thoughts?

> I have also noticed that it is possible to use pidfds to poll on any
> process regardless of whether the process is a child of the caller,
> has a different UID, or has a different security context. Is this
> also worth addressing? If so, what exactly should the DAC checks be?

Oleg and I had discusses this and decided that such polling isn't
sensitive information so by default this should just work and it's
relied upon in Android and in a bunch of other workloads. An LSM can of
course restrict access via security_file_ioctl().

Fwiw, pidfds now support persistent trusted extended attributes so if
the LSM folks wanted we can add security.* extended attribute support
and they can mark pidfds with persistent security labels - persistent as
in for the lifetime of the task.

> Signed-off-by: Daniel Durning <danieldurning.work@gmail.com>
> ---
>  fs/pidfs.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/pidfs.c b/fs/pidfs.c
> index dba703d4ce4a..058a7d798bca 100644
> --- a/fs/pidfs.c
> +++ b/fs/pidfs.c
> @@ -365,6 +365,13 @@ static long pidfd_info(struct file *file, unsigned int cmd, unsigned long arg)
>  		goto copy_out;
>  	}
>  
> +	/*
> +	 * Do a filesystem cred ptrace check to verify access
> +	 * to the task's info.
> +	 */
> +	if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS))
> +		return -EACCES;
> +
>  	c = get_task_cred(task);
>  	if (!c)
>  		return -ESRCH;
> -- 
> 2.52.0
> 

^ permalink raw reply

* Re: [PATCH net-next v2 0/4] net: uapi: Provide an UAPI definition of 'struct sockaddr'
From: Thomas Weißschuh @ 2026-02-09 13:34 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
	David S. Miller, Simon Horman, Shuah Khan, Matthieu Baerts,
	Mat Martineau, Geliang Tang, Mickaël Salaün,
	Günther Noack, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Hao Luo, Jiri Olsa, netdev, linux-kernel,
	linux-api, Arnd Bergmann, linux-kselftest, mptcp,
	linux-security-module, bpf, libc-alpha, Carlos O'Donell,
	Adhemerval Zanella, Rich Felker, klibc, Florian Weimer
In-Reply-To: <20260204175516.4fb11966@kernel.org>

On Wed, Feb 04, 2026 at 05:55:16PM -0800, Jakub Kicinski wrote:
> On Wed, 4 Feb 2026 06:51:46 +0100 Thomas Weißschuh wrote:
> > > make -j16 O="$kobj" INSTALL_HDR_PATH="${kobj}/hdr" headers_install
> > > popd
> > > 
> > > pushd uapi
> > > find . -type f -name '*.h' -exec cp -v "${kobj}/hdr/include/{}" {} \;  
> > 
> > Here only those headers which already exist in ethtool's uapi/ directory
> > are copied. As linux/typelimits.h is new, it is now missing.
> > Honestly, if a user fiddles with the internals of the UAPI headers like
> > this, it is on them to update their code if the internal structure
> > changes. In your case a simple 'touch uapi/linux/typelimits.h'
> > before running the script will be enough. Also internal.h now requires
> > an explicit inclusion of <limits.h>, as that is not satisfied by the
> > UAPI anymore.
> 
> Hopefully you understand that while due to uapi header copy this is not
> a huge issue for ethtool itself, but it is a proof that your changes
> can break normal user space applications which do not vendor in uapi.

A regular application which uses a full UAPI headers tree will get
access to the new header automatically.
If the application uses symbols from <limits.h> without including <limits.h>
on its own, it is broken.


Thomas

^ permalink raw reply

* Re: [PATCH v4 2/6] landlock: Control pathname UNIX domain socket resolution by path
From: Justin Suess @ 2026-02-09 13:11 UTC (permalink / raw)
  To: Günther Noack, Günther Noack
  Cc: Mickaël Salaün, John Johansen, Tingmao Wang, Jann Horn,
	linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
	Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
	Alyssa Ross, Tahera Fahimi
In-Reply-To: <aYm1RWtV6Af-zEHf@google.com>

On 2/9/26 05:21, Günther Noack wrote:
> On Mon, Feb 09, 2026 at 12:10:12AM +0100, Günther Noack wrote:
>> +static int hook_unix_find(const struct path *const path, struct sock *other,
>> +			  int flags)
>> +{
>> +	const struct landlock_ruleset *dom_other;
>> +	const struct landlock_cred_security *subject;
>> +	struct layer_access_masks layer_masks;
>> +	struct landlock_request request = {};
>> +	static const struct access_masks fs_resolve_unix = {
>> +		.fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
>> +	};
>> +	int type = other->sk_type;
>> +
>> +	/* Lookup for the purpose of saving coredumps is OK. */
>> +	if (flags & SOCK_COREDUMP)
>> +		return 0;   
if (unlikely(flags & SOCK_COREDUMP))
    return 0;
>> +
>> +	/* Only stream, dgram and seqpacket sockets are restricted. */
>> +	if (type != SOCK_STREAM && type != SOCK_DGRAM && type != SOCK_SEQPACKET)
>> +		return 0;
if (unlikely(type != SOCK_STREAM && type != SOCK_DGRAM && type != SOCK_SEQPACKET))
    return 0;
> FYI: This is a (highly speculative) safeguard, because these three
> socket types are the only ones that exist in AF_UNIX (compare unix(7),
> 2nd paragraph).
>
> In the (highly unlikely) case that someone adds a fourth AF_UNIX
> socket type, this means that Landlock will start permitting
> connections to these sockets unconditionally.
>
> I am unsure whether the safeguard is useful, or whether we should
> rather group that (highly unlikely) future socket type together with
> the existing ones.  *If you have opinions, I'd be interested.*
In that case, a new access right could be added for that new socket type.

So we should only handle the ones we expect for now, and reserve the option for future rights
to expand the capability.
>
> The fact that these are the only existing AF_UNIX socket types is also
> the reason why it does not matter that we are now (in v4) taking the
> type value from the server-side sk instead of the client socket.  The
> check will either way always pass as long as only these three types
> are the only ones.
Last time a new socket type for UDS (seqpacket) was added was 2.6.4.
UDS are also part of POSIX, so they're pretty standardized and not exactly likely to change.

>
> For now (and probably for another few decades :)), as long as these
> are the only AF_UNIX types, it does not make a difference though
> whether the check is there or not.
I think this is the correct choice.
>
> —Günther


^ permalink raw reply

* Re: [PATCH] xfrm: kill xfrm_dev_{state,policy}_flush_secctx_check()
From: Steffen Klassert @ 2026-02-09 11:22 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: Paul Moore, SELinux, linux-security-module, Herbert Xu,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Network Development
In-Reply-To: <85546d35-c7bd-49bf-b0c3-9677bde25859@I-love.SAKURA.ne.jp>

On Mon, Feb 09, 2026 at 07:02:47PM +0900, Tetsuo Handa wrote:
> On 2026/02/09 18:25, Steffen Klassert wrote:
> > The problem is that, with adding IPsec offloads to netdevices, security
> > critical resources came into the netdevices. Someone who has no
> > capabilities to delete xfrm states or xfrm policies should not be able
> > to unregister the netdevice if xfrm states or xfrm policies are
> > offloaded. Unfortunately, unregistering can't be canceled at this stage
> > anymore. So I think we need some netdevice unregistration hook for
> > the LSM subsystem so it can check for xfrm states or xfrm policies
> > and refuse the unregistration before we actually start to remove
> > the device.
> 
> Unfortunately, unregistering is not always triggered by a user's request. ;-)

As far as I remember, a security context is not always tied to a
user request. It can also be attached to system tasks or objects.

> For example, we don't check permission for unmount when a mount is deleted
> due to teardown of a mount namespace. I wonder why you want to check permission
> for unregistering a net_device when triggered by a teardown path.

I just try to find out what's the right thing to do here.
If a policy goes away, packets that match this policy will
find another path through the network stack. As best, they
are dropped somewhere, but they can also leave on some other
device without encryption. A LSM that implements xfrm hooks
must be able to check the permission to delete the xfrm policy
or state.

> 
> > 
> > The same happened btw. when xfrm was made per network namespace.
> > Here we just leak the xfrm states and xfrm policies if some
> > LSM refuses to remove them.
> > 
> > I guess we need a solution for both cases.
> 
> Is replacing the NETDEV_UNREGISTER net_device with the blackhole_netdev applicable
> ( https://elixir.bootlin.com/linux/v6.19-rc5/source/net/xfrm/xfrm_policy.c#L3948 ) ?
> If no, there is no choice but break SELinux's expectation.

That could be an option to not accidentally send out
unencrypted packets. But finding the right place for
these checks would be preferable IMO.

^ permalink raw reply

* Re: [PATCH v4 2/6] landlock: Control pathname UNIX domain socket resolution by path
From: Günther Noack @ 2026-02-09 10:21 UTC (permalink / raw)
  To: Günther Noack
  Cc: Mickaël Salaün, John Johansen, Tingmao Wang,
	Justin Suess, Jann Horn, linux-security-module,
	Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
	konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross,
	Tahera Fahimi
In-Reply-To: <20260208231017.114343-3-gnoack3000@gmail.com>

On Mon, Feb 09, 2026 at 12:10:12AM +0100, Günther Noack wrote:
> +static int hook_unix_find(const struct path *const path, struct sock *other,
> +			  int flags)
> +{
> +	const struct landlock_ruleset *dom_other;
> +	const struct landlock_cred_security *subject;
> +	struct layer_access_masks layer_masks;
> +	struct landlock_request request = {};
> +	static const struct access_masks fs_resolve_unix = {
> +		.fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> +	};
> +	int type = other->sk_type;
> +
> +	/* Lookup for the purpose of saving coredumps is OK. */
> +	if (flags & SOCK_COREDUMP)
> +		return 0;
> +
> +	/* Only stream, dgram and seqpacket sockets are restricted. */
> +	if (type != SOCK_STREAM && type != SOCK_DGRAM && type != SOCK_SEQPACKET)
> +		return 0;

FYI: This is a (highly speculative) safeguard, because these three
socket types are the only ones that exist in AF_UNIX (compare unix(7),
2nd paragraph).

In the (highly unlikely) case that someone adds a fourth AF_UNIX
socket type, this means that Landlock will start permitting
connections to these sockets unconditionally.

I am unsure whether the safeguard is useful, or whether we should
rather group that (highly unlikely) future socket type together with
the existing ones.  *If you have opinions, I'd be interested.*

The fact that these are the only existing AF_UNIX socket types is also
the reason why it does not matter that we are now (in v4) taking the
type value from the server-side sk instead of the client socket.  The
check will either way always pass as long as only these three types
are the only ones.

For now (and probably for another few decades :)), as long as these
are the only AF_UNIX types, it does not make a difference though
whether the check is there or not.

—Günther

^ permalink raw reply

* Re: [PATCH] xfrm: kill xfrm_dev_{state,policy}_flush_secctx_check()
From: Tetsuo Handa @ 2026-02-09 10:02 UTC (permalink / raw)
  To: Steffen Klassert, Paul Moore, SELinux, linux-security-module
  Cc: Herbert Xu, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Network Development
In-Reply-To: <aYmoDwO-YXrc4W1c@secunet.com>

On 2026/02/09 18:25, Steffen Klassert wrote:
> The problem is that, with adding IPsec offloads to netdevices, security
> critical resources came into the netdevices. Someone who has no
> capabilities to delete xfrm states or xfrm policies should not be able
> to unregister the netdevice if xfrm states or xfrm policies are
> offloaded. Unfortunately, unregistering can't be canceled at this stage
> anymore. So I think we need some netdevice unregistration hook for
> the LSM subsystem so it can check for xfrm states or xfrm policies
> and refuse the unregistration before we actually start to remove
> the device.

Unfortunately, unregistering is not always triggered by a user's request. ;-)

For example, we don't check permission for unmount when a mount is deleted
due to teardown of a mount namespace. I wonder why you want to check permission
for unregistering a net_device when triggered by a teardown path.

> 
> The same happened btw. when xfrm was made per network namespace.
> Here we just leak the xfrm states and xfrm policies if some
> LSM refuses to remove them.
> 
> I guess we need a solution for both cases.

Is replacing the NETDEV_UNREGISTER net_device with the blackhole_netdev applicable
( https://elixir.bootlin.com/linux/v6.19-rc5/source/net/xfrm/xfrm_policy.c#L3948 ) ?
If no, there is no choice but break SELinux's expectation.


^ permalink raw reply

* Re: [PATCH] landlock: Add counted_by and fix comment in landlock_ruleset
From: Günther Noack @ 2026-02-09  9:47 UTC (permalink / raw)
  To: Tingmao Wang; +Cc: Mickaël Salaün, linux-security-module
In-Reply-To: <20260208235449.1124354-1-m@maowtm.org>

On Sun, Feb 08, 2026 at 11:54:48PM +0000, Tingmao Wang wrote:
> For a domain, this array stores the access masks for each layer (of
> which there are num_layers of them).  For an unmerged ruleset, we have
> one "layer", and one element in this array.  This annotation serves as
> useful documentation.
> 
> This also removes a comment saying that num_layers = 0 for unmerged
> rulesets, which is incorrect (it is 1).
> 
> Signed-off-by: Tingmao Wang <m@maowtm.org>

Reviewed-by: Günther Noack <gnoack@google.com>

Thank you for fixing this! We should use these bounds check macros
wherever we can, IMHO.  Removing the remark about num_layers=0 is also
correct.

—Günther

^ permalink raw reply

* [bug report] apparmor: add support loading per permission tagging
From: Dan Carpenter @ 2026-02-09  9:44 UTC (permalink / raw)
  To: John Johansen; +Cc: apparmor, linux-security-module, linux-kernel
In-Reply-To: <caa37f28-a2e8-4e0a-a9ce-a365ce805e4b@stanley.mountain>

[ Smatch checking is paused while we raise funding. #SadFace
  https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]

Hello John Johansen,

Commit 3d28e2397af7 ("apparmor: add support loading per permission
tagging") from Apr 1, 2025 (linux-next), leads to the following
Smatch static checker warning:

	security/apparmor/policy_unpack.c:966 unpack_pdb()
	warn: unsigned 'unpack_tags(e, &pdb->tags, info)' is never less than zero.

security/apparmor/policy_unpack.c
    951 static int unpack_pdb(struct aa_ext *e, struct aa_policydb **policy,
    952                       bool required_dfa, bool required_trans,
    953                       const char **info)
    954 {
    955         struct aa_policydb *pdb;
    956         void *pos = e->pos;
    957         int i, flags, error = -EPROTO;
    958         ssize_t size;
    959         u32 version = 0;
    960 
    961         pdb = aa_alloc_pdb(GFP_KERNEL);
    962         if (!pdb)
    963                 return -ENOMEM;
    964 
    965         AA_DEBUG(DEBUG_UNPACK, "unpacking tags");
--> 966         if (unpack_tags(e, &pdb->tags, info) < 0)
                    ^^^^^^^^^^^
Signedness bug.  unpack_tags() is unsigned.

    967                 goto fail;
    968         AA_DEBUG(DEBUG_UNPACK, "done unpacking tags");
    969 
    970         size = unpack_perms_table(e, &pdb->perms);
    971         if (size < 0) {
    972                 error = size;
    973                 pdb->perms = NULL;

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH] xfrm: kill xfrm_dev_{state,policy}_flush_secctx_check()
From: Steffen Klassert @ 2026-02-09  9:25 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: Paul Moore, SELinux, linux-security-module, Herbert Xu,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Network Development
In-Reply-To: <7ef21dab-3805-4eae-80d7-9779aeff3f58@I-love.SAKURA.ne.jp>

On Wed, Feb 04, 2026 at 10:57:30PM +0900, Tetsuo Handa wrote:
> On 2026/02/04 19:15, Tetsuo Handa wrote:
> > On 2026/02/04 7:40, Paul Moore wrote:
> >>         This is not an unusual request for such a proposed change, and
> >> is something that I would expect a LSM maintainer to do without much
> >> hesitation.  If you are unwilling to investigate this, can you explain
> >> why?
> > 
> > Because I'm not familiar with how XFRM works; I'm not a user of LSM XFRM hooks.
> > 
> > I can't judge whether the current code is COMPREHENSIVELY GATING;
> > I can't imagine what the state you call COMPREHENSIVELY GATING is.
> 
> Steffen Klassert worried that killing xfrm_dev_state_flush_secctx_check() and
> xfrm_dev_policy_flush_secctx_check() might violate a LSM policy and you agreed
> ( https://lkml.kernel.org/r/CAHC9VhQ54LRD7k_x6tUju2kPVBEHcdgBh46_hBN8btG0vhfy_w@mail.gmail.com ),
> but the reality is that nobody in the world has enforced an LSM policy for almost 9 years
> that makes xfrm_dev_{state,policy}_flush() no-op. That is, xfrm_dev_state_flush_secctx_check()
> and xfrm_dev_policy_flush_secctx_check() had been effectively unused.
> 
> Killing xfrm_dev_state_flush_secctx_check() and xfrm_dev_policy_flush_secctx_check()
> increases "system's stability" without sacrificing "authorization".
> 
> It is up to SELinux developers to discuss what actions to take as a compensation for
> killing xfrm_dev_state_flush_secctx_check() and xfrm_dev_policy_flush_secctx_check().
> The compensation might be to add LSM hooks to immediately before the point of no return.

The problem is that, with adding IPsec offloads to netdevices, security
critical resources came into the netdevices. Someone who has no
capabilities to delete xfrm states or xfrm policies should not be able
to unregister the netdevice if xfrm states or xfrm policies are
offloaded. Unfortunately, unregistering can't be canceled at this stage
anymore. So I think we need some netdevice unregistration hook for
the LSM subsystem so it can check for xfrm states or xfrm policies
and refuse the unregistration before we actually start to remove
the device.

The same happened btw. when xfrm was made per network namespace.
Here we just leak the xfrm states and xfrm policies if some
LSM refuses to remove them.

I guess we need a solution for both cases.

^ permalink raw reply

* [PATCH] landlock: Add counted_by and fix comment in landlock_ruleset
From: Tingmao Wang @ 2026-02-08 23:54 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Tingmao Wang, Günther Noack, linux-security-module

For a domain, this array stores the access masks for each layer (of
which there are num_layers of them).  For an unmerged ruleset, we have
one "layer", and one element in this array.  This annotation serves as
useful documentation.

This also removes a comment saying that num_layers = 0 for unmerged
rulesets, which is incorrect (it is 1).

Signed-off-by: Tingmao Wang <m@maowtm.org>
---

I'm not sure if I should add a Fixes tag, but if I should had, it would
be ae271c1b14 ("landlock: Add ruleset and domain management").

 security/landlock/ruleset.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 9d6dc632684c..7005840ac641 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -168,8 +168,7 @@ struct landlock_ruleset {
 			/**
 			 * @num_layers: Number of layers that are used in this
 			 * ruleset.  This enables to check that all the layers
-			 * allow an access request.  A value of 0 identifies a
-			 * non-merged ruleset (i.e. not a domain).
+			 * allow an access request.
 			 */
 			u32 num_layers;
 			/**
@@ -184,7 +183,8 @@ struct landlock_ruleset {
 			 * layers are set once and never changed for the
 			 * lifetime of the ruleset.
 			 */
-			struct access_masks access_masks[];
+			struct access_masks
+				access_masks[] __counted_by(num_layers);
 		};
 	};
 };

base-commit: f179e1859c711214412876c57f56f9b0cfb13264
-- 
2.53.0

^ permalink raw reply related

* Re: [PATCH v2 0/6] Landlock: Implement scope control for pathname Unix sockets
From: Günther Noack @ 2026-02-08 23:21 UTC (permalink / raw)
  To: Tingmao Wang
  Cc: Mickaël Salaün, Günther Noack, Justin Suess,
	Paul Moore, John Johansen, Demi Marie Obenour, Alyssa Ross,
	Jann Horn, Tahera Fahimi, Matthieu Buffet, linux-security-module
In-Reply-To: <c33926c5-3c7a-40d8-b910-ac99b43576ff@maowtm.org>

On Sun, Feb 08, 2026 at 08:48:22PM +0000, Tingmao Wang wrote:
> On 2/8/26 20:37, Günther Noack wrote:
> > Thank you, Tingmao!
> >
> > So far, the selftests that I already had in fs_test.c were
> > straightforward to extend so that they cover the new cases.  I had a
> > look at your patch set, but found the scoping tests difficult to port
> > to fs_test.c
> 
> I was thinking that the tests in scoped_abstract_unix_test.c could be
> extended to test scoping of pathname UNIX sockets as well (otherwise
> wouldn't you have to write another instance of the scoped_domains test
> based on scoped_base_variants.h, whether you put it in fs_test.c or
> somewhere else?)
> 
> And if you think that is sensible, then I'm hoping that patch 4,5,6 of the
> series would be mostly useful.  But it's up to you :)

I maybe have not wrapped my head around the scoped_test enough; for
now I sent a tentative V4 patch set to the list, so that we can
discuss something concrete.

If you spot things that are missing, or you feel inspired to port your
tests on top, I am still happy to accept that. (But for today it is
too late in the evening here %-))

–Günther

^ permalink raw reply

* [PATCH v4 6/6] landlock: Document design rationale for scoped access rights
From: Günther Noack @ 2026-02-08 23:10 UTC (permalink / raw)
  To: Mickaël Salaün, John Johansen
  Cc: Günther Noack, linux-security-module, Tingmao Wang,
	Justin Suess, Samasth Norway Ananda, Matthieu Buffet,
	Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
	Alyssa Ross, Jann Horn, Tahera Fahimi
In-Reply-To: <20260208231017.114343-1-gnoack3000@gmail.com>

Document the (possible future) interaction between scoped flags and
other access rights in struct landlock_ruleset_attr, and summarize the
rationale, as discussed in code review leading up to [1].

Link[1]: https://lore.kernel.org/all/20260205.8531e4005118@gnoack.org/
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
 Documentation/security/landlock.rst | 38 +++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/Documentation/security/landlock.rst b/Documentation/security/landlock.rst
index 3e4d4d04cfae..49ef02d5e272 100644
--- a/Documentation/security/landlock.rst
+++ b/Documentation/security/landlock.rst
@@ -89,6 +89,44 @@ this is required to keep access controls consistent over the whole system, and
 this avoids unattended bypasses through file descriptor passing (i.e. confused
 deputy attack).
 
+Interaction between scoped flags and other access rights
+--------------------------------------------------------
+
+The ``scoped`` flags in ``struct landlock_ruleset_attr`` restrict the
+use of *outgoing* IPC from the created Landlock domain, while they
+permit reaching out to IPC endpoints *within* the created Landlock
+domain.
+
+In the future, scoped flags *may* interact with other access rights,
+e.g. so that abstract UNIX sockets can be allow-listed by name, or so
+that signals can be allow-listed by signal number or target process.
+
+When introducing ``LANDLOCK_ACCESS_FS_RESOLVE_UNIX``, we defined it to
+implicitly have the same scoping semantics as a
+``LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET`` flag would have: connecting to
+UNIX sockets within the same domain (where
+``LANDLOCK_ACCESS_FS_RESOLVE_UNIX`` is used) is unconditionally
+allowed.
+
+The reasoning is:
+
+* Like other IPC mechanisms, connecting to named UNIX sockets in the
+  same domain should be expected and harmless.  (If needed, users can
+  further refine their Landlock policies with nested domains or by
+  restricting ``LANDLOCK_ACCESS_FS_MAKE_SOCK``.)
+* We reserve the option to still introduce
+  ``LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET`` in the future.  (This would
+  be useful if we wanted to have a Landlock rule to permit IPC access
+  to other Landlock domains.)
+* But we can postpone the point in time when users have to deal with
+  two interacting flags visible in the userspace API.  (In particular,
+  it is possible that it won't be needed in practice, in which case we
+  can avoid the second flag altogether.)
+* If we *do* introduce ``LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET`` in the
+  future, setting this scoped flag in a ruleset does *not reduce* the
+  restrictions, because access within the same scope is already
+  allowed based on ``LANDLOCK_ACCESS_FS_RESOLVE_UNIX``.
+
 Tests
 =====
 
-- 
2.52.0


^ permalink raw reply related


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