Re: [PATCH v4 2/7] landlock: Add UDP connect() access control

["Mickaël Salaün" <mic@digikod.net>]

On Sat, May 02, 2026 at 02:43:01PM +0200, Matthieu Buffet wrote:
> Add support for a second fine-grained UDP access right.
> This first half of LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP controls the
> ability to set the remote port of a socket (via connect()). It will be
> useful for applications that send datagrams, and for some servers too
> (those creating per-client sockets, which want to receive traffic only
> from a specific address).
> 
> Similarly as for bind(), this access control is performed when
> configuring sockets, not in hot code paths.
> 
> Include detection of when autobind is about to be required, and check if
> the process would be allowed to call bind(0) explicitly. Autobind can
> only be performed when sending a first datagram, when connect()ing, and
> in some splice() EOF edge case which, afaiu, can only happen after a
> remote peer has been set (which is already covered).
> 
> Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
> ---
>  include/uapi/linux/landlock.h               | 19 +++++
>  security/landlock/audit.c                   |  2 +
>  security/landlock/limits.h                  |  2 +-
>  security/landlock/net.c                     | 79 +++++++++++++++++----
>  tools/testing/selftests/landlock/net_test.c |  5 +-
>  5 files changed, 92 insertions(+), 15 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 045b251ff1b4..22c8cc63f30e 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -378,11 +378,30 @@ struct landlock_net_port_attr {
>   *
>   * - %LANDLOCK_ACCESS_NET_BIND_UDP: Bind UDP sockets to the given local
>   *   port. Support added in Landlock ABI version 10.
> + * - %LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP: Set the remote port of UDP
> + *   sockets to the given port, or send datagrams to the given remote port
> + *   ignoring any destination pre-set on a socket. Support added in
> + *   Landlock ABI version 10.
> + *
> + * .. note:: Setting a remote address or sending a first datagram
> + *   auto-binds UDP sockets to an ephemeral local source port if not
> + *   already bound. To allow this if both %LANDLOCK_ACCESS_NET_BIND_UDP
> + *   and %LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP are handled, you need to
> + *   either:
> + *
> + *   - use a socket already bound to a port before the ruleset started
> + *     being enforced;
> + *   - or grant %LANDLOCK_ACCESS_NET_BIND_UDP on port 0, meaning "any
> + *     port in the ephemeral port range";
> + *   - or grant %LANDLOCK_ACCESS_NET_BIND_UDP on a specific port, and
> + *     call :manpage:`bind(2)` on that port before trying to
> + *     :manpage:`connect(2)` or send datagrams.
>   */
>  /* clang-format off */
>  #define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
>  #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
>  #define LANDLOCK_ACCESS_NET_BIND_UDP			(1ULL << 2)
> +#define LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP		(1ULL << 3)
>  /* clang-format on */
>  
>  /**
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index e676ebffeebe..851647197a01 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -46,6 +46,8 @@ static const char *const net_access_strings[] = {
>  	[BIT_INDEX(LANDLOCK_ACCESS_NET_BIND_TCP)] = "net.bind_tcp",
>  	[BIT_INDEX(LANDLOCK_ACCESS_NET_CONNECT_TCP)] = "net.connect_tcp",
>  	[BIT_INDEX(LANDLOCK_ACCESS_NET_BIND_UDP)] = "net.bind_udp",
> +	[BIT_INDEX(LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP)] =
> +		"net.connect_send_udp",
>  };
>  
>  static_assert(ARRAY_SIZE(net_access_strings) == LANDLOCK_NUM_ACCESS_NET);
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index c0f30a4591b8..a4d908b240a2 100644
> --- a/security/landlock/limits.h
> +++ b/security/landlock/limits.h
> @@ -23,7 +23,7 @@
>  #define LANDLOCK_MASK_ACCESS_FS		((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
>  #define LANDLOCK_NUM_ACCESS_FS		__const_hweight64(LANDLOCK_MASK_ACCESS_FS)
>  
> -#define LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_BIND_UDP
> +#define LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP
>  #define LANDLOCK_MASK_ACCESS_NET	((LANDLOCK_LAST_ACCESS_NET << 1) - 1)
>  #define LANDLOCK_NUM_ACCESS_NET		__const_hweight64(LANDLOCK_MASK_ACCESS_NET)
>  
> diff --git a/security/landlock/net.c b/security/landlock/net.c
> index f9ccb52e7d45..045881f81295 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -68,16 +68,17 @@ static int current_check_access_socket(struct socket *const sock,
>  
>  	switch (address->sa_family) {
>  	case AF_UNSPEC:
> -		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP) {
> +		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP ||
> +		    access_request == LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP) {
>  			/*
>  			 * Connecting to an address with AF_UNSPEC dissolves
> -			 * the TCP association, which have the same effect as
> -			 * closing the connection while retaining the socket
> -			 * object (i.e., the file descriptor).  As for dropping
> -			 * privileges, closing connections is always allowed.
> -			 *
> -			 * For a TCP access control system, this request is
> -			 * legitimate. Let the network stack handle potential
> +			 * the remote association while retaining the socket
> +			 * object (i.e., the file descriptor). For TCP, it has
> +			 * the same effect as closing the connection. For UDP,
> +			 * it removes any preset remote address. As for
> +			 * dropping privileges, these actions are always
> +			 * allowed.
> +			 * Let the network stack handle potential
>  			 * inconsistencies and return -EINVAL if needed.
>  			 */
>  			return 0;
> @@ -134,7 +135,8 @@ static int current_check_access_socket(struct socket *const sock,
>  		addr4 = (struct sockaddr_in *)address;
>  		port = addr4->sin_port;
>  
> -		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP) {
> +		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP ||
> +		    access_request == LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP) {
>  			audit_net.dport = port;
>  			audit_net.v4info.daddr = addr4->sin_addr.s_addr;
>  		} else if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP ||
> @@ -157,7 +159,8 @@ static int current_check_access_socket(struct socket *const sock,
>  		addr6 = (struct sockaddr_in6 *)address;
>  		port = addr6->sin6_port;
>  
> -		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP) {
> +		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP ||
> +		    access_request == LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP) {
>  			audit_net.dport = port;
>  			audit_net.v6info.daddr = addr6->sin6_addr;
>  		} else if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP ||
> @@ -213,6 +216,50 @@ static int current_check_access_socket(struct socket *const sock,
>  	return -EACCES;
>  }
>  
> +static int current_check_autobind_udp_socket(struct socket *const sock)
> +{
> +	struct sockaddr_storage port0 = { 0 };

unsigned short num;
bool slow;

> +
> +	/*
> +	 * On UDP sockets, if a local port has not already been bound,
> +	 * calling connect() or sending a first datagram has the side
> +	 * effect of autobinding an ephemeral port: we also have to check
> +	 * that the process would have had the right to bind(0) explicitly.


> +	 * Note: socket is not locked, so another thread could do an
> +	 * explicit bind(!=0) on this socket, changing inet_num to non-zero
> +	 * after we read it, but this would only have us enforce an
> +	 * additional bind(0) access check and would not bypass policy.
> +	 */
> +	if (inet_sk(sock->sk)->inet_num != 0)

There is still a race condition, this should fix it:

 *
 * Hold the socket lock around the inet_num read to exclude
 * udp_lib_get_port()'s transient inet_num = snum write that is reverted
 * to 0 on a failing reuseport bind.
 */
if (inet_sk(sock->sk)->inet_num != 0)
slow = lock_sock_fast(sock->sk);
num = inet_sk(sock->sk)->inet_num;
unlock_sock_fast(sock->sk, slow);
if (num != 0)
	return 0;


> +		return 0;
> +
> +	/*
> +	 * Construct a struct sockaddr* with port 0 to pretend the
> +	 * process tried to bind() on that address.
> +	 */
> +	port0.ss_family = sock->sk->__sk_common.skc_family;

Looking at net/, __sk_common.skc_family (and other __sk_common.* fields)
should be replaced by sk_family (see #define in include/net/sock.h).
Same for all other __sk_common.

In fact, it should be READ_ONCE(sock->sk->sk_family) for consistency
with the net/ code and because the socket are not locked when the LSM
hooks are called.  I realized that the existing Landlock code have the
same issue...  Could you please add a new patch (to be backported) to
always use this pattern?

> +	switch (port0.ss_family) {
> +	case AF_INET: {
> +		((struct sockaddr_in *)&port0)->sin_port = 0;
> +		break;
> +	}
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> +	case AF_INET6: {
> +		((struct sockaddr_in6 *)&port0)->sin6_port = 0;
> +		break;
> +	}
> +#endif /* IS_ENABLED(CONFIG_IPV6) */
> +
> +	default:
> +		return 0;
> +	}
> +
> +	return current_check_access_socket(sock, (struct sockaddr *)&port0,
> +					   sizeof(port0),
> +					   LANDLOCK_ACCESS_NET_BIND_UDP);
> +}
> +
>  static int hook_socket_bind(struct socket *const sock,
>  			    struct sockaddr *const address, const int addrlen)
>  {
> @@ -234,14 +281,22 @@ static int hook_socket_connect(struct socket *const sock,
>  			       const int addrlen)
>  {
>  	access_mask_t access_request;
> +	int ret = 0;
>  
>  	if (sk_is_tcp(sock->sk))
>  		access_request = LANDLOCK_ACCESS_NET_CONNECT_TCP;
> +	else if (sk_is_udp(sock->sk))
> +		access_request = LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP;
>  	else
>  		return 0;
>  
> -	return current_check_access_socket(sock, address, addrlen,
> -					   access_request);
> +	ret = current_check_access_socket(sock, address, addrlen,
> +					  access_request);
> +
> +	if (ret == 0 && sk_is_udp(sock->sk))
> +		ret = current_check_autobind_udp_socket(sock);
> +
> +	return ret;
>  }
>  
>  static struct security_hook_list landlock_hooks[] __ro_after_init = {
> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
> index ec392d971ea3..016c7277e370 100644
> --- a/tools/testing/selftests/landlock/net_test.c
> +++ b/tools/testing/selftests/landlock/net_test.c
> @@ -1326,12 +1326,13 @@ FIXTURE_TEARDOWN(mini)
>  
>  /* clang-format off */
>  
> -#define ACCESS_LAST LANDLOCK_ACCESS_NET_BIND_UDP
> +#define ACCESS_LAST LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP
>  
>  #define ACCESS_ALL ( \
>  	LANDLOCK_ACCESS_NET_BIND_TCP | \
>  	LANDLOCK_ACCESS_NET_CONNECT_TCP | \
> -	LANDLOCK_ACCESS_NET_BIND_UDP)
> +	LANDLOCK_ACCESS_NET_BIND_UDP | \
> +	LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP)
>  
>  /* clang-format on */
>  
> -- 
> 2.39.5
>