Re: [PATCH 3/6] landlock: Add MPTCP bind and connect access rights

[Geliang Tang <geliang@kernel.org>]

Hi Günther,

On Wed, 2026-09-23 at 16:27 +0200, Günther Noack wrote:
> On Wed, Sep 23, 2026 at 01:04:40PM +0800, Geliang Tang wrote:
> > On Mon, 2026-08-31 at 12:09 +0800, Geliang Tang wrote:
> > > On Sun, 2026-08-30 at 22:16 +0200, Günther Noack wrote:
> > > > MPTCP sockets have equivalent bind(2) and connect(2) operations
> > > > as
> > > > TCP
> > > > sockets, but can not currently be restricted with Landlock
> > > > without
> > > > explicit MPTCP access rights.  As MPTCP operates on the same
> > > > TCP
> > > > port
> > > > number space as TCP, this is a gap in Landlock's policies.
> > > > 
> > > > Add access rights for MPTCP bind(2) and connect(2) operations
> > > > and document them in the header.
> > > > 
> > > > Treat TCP Fast Open the same as done for plain TCP in
> > > > commit 33cb713db016 ("landlock: Fix TCP Fast Open connection
> > > > bypass")
> > > > 
> > > > The port numbers used in MPTCP subflows are negotiated by the
> > > > kernel
> > > > and therefore not subject to these access rights.
> > > > 
> > > > Bump the Landlock ABI version to 12.
> > > > 
> > > > Closes: https://github.com/landlock-lsm/linux/issues/54
> > > > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > > > ---
> > > >  include/linux/landlock.h                     |  5 +-
> > > >  include/uapi/linux/landlock.h                | 24 +++++++
> > > >  security/landlock/limits.h                   |  2 +-
> > > >  security/landlock/net.c                      | 68
> > > > ++++++++++++++--
> > > > --
> > > > --
> > > >  security/landlock/syscalls.c                 |  2 +-
> > > >  tools/testing/selftests/landlock/base_test.c |  2 +-
> > > >  6 files changed, 79 insertions(+), 24 deletions(-)
> > > > 
> > > > diff --git a/include/linux/landlock.h
> > > > b/include/linux/landlock.h
> > > > index 004cbd0b9298..b04ffc7caa21 100644
> > > > --- a/include/linux/landlock.h
> > > > +++ b/include/linux/landlock.h
> > > > @@ -46,7 +46,10 @@
> > > >  	_LANDLOCK_NAME_ENTRY(LANDLOCK_ACCESS_NET_CONNECT_TCP,
> > > > "connect_tcp"), \
> > > >  	_LANDLOCK_NAME_ENTRY(LANDLOCK_ACCESS_NET_BIND_UDP,
> > > > "bind_udp"), \
> > > >  	_LANDLOCK_NAME_ENTRY(LANDLOCK_ACCESS_NET_CONNECT_SEND_
> > > > UDP,
> > > > \
> > > > -			     "connect_send_udp")
> > > > +			     "connect_send_udp"), \
> > > > +	_LANDLOCK_NAME_ENTRY(LANDLOCK_ACCESS_NET_BIND_MPTCP,
> > > > "bind_mptcp"), \
> > > > +	_LANDLOCK_NAME_ENTRY(LANDLOCK_ACCESS_NET_CONNECT_MPTCP
> > > > , \
> > > > +			     "connect_mptcp")
> > > >  
> > > >  #define _LANDLOCK_SCOPE_NAMES \
> > > >  	_LANDLOCK_NAME_ENTRY(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCK
> > > > ET,
> > > > \
> > > > diff --git a/include/uapi/linux/landlock.h
> > > > b/include/uapi/linux/landlock.h
> > > > index cceda3b3b961..2a953ba7ce25 100644
> > > > --- a/include/uapi/linux/landlock.h
> > > > +++ b/include/uapi/linux/landlock.h
> > > > @@ -448,6 +448,9 @@ struct landlock_net_port_attr {
> > > >   * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect TCP sockets to
> > > > the
> > > > given
> > > >   *   remote port. Support added in Landlock ABI version 4.
> > > >   *
> > > > + * .. note:: These rights do not apply to MPTCP sockets, which
> > > > have
> > > > their own
> > > > + *   access rights (see below).
> > > > + *
> > > >   * And similarly for UDP port numbers:
> > > >   *
> > > >   * - %LANDLOCK_ACCESS_NET_BIND_UDP: Bind UDP sockets to the
> > > > given
> > > > local
> > > > @@ -474,12 +477,33 @@ struct landlock_net_port_attr {
> > > >   * .. note:: Sending datagrams to an ``AF_UNSPEC`` destination
> > > > address
> > > >   *   family is not supported for IPv6 UDP sockets: you will
> > > > need
> > > > to
> > > > use a
> > > >   *   ``NULL`` address instead.
> > > > + *
> > > > + * MPTCP sockets (created with ``IPPROTO_MPTCP``) use TCP port
> > > > numbers, but
> > > > + * they are controlled by their own access rights:
> > > > + *
> > > > + * - %LANDLOCK_ACCESS_NET_BIND_MPTCP: Bind MPTCP sockets to
> > > > the
> > > > given local
> > > > + *   port. Support added in Landlock ABI version 12.
> > > > + * - %LANDLOCK_ACCESS_NET_CONNECT_MPTCP: Connect MPTCP sockets
> > > > to
> > > > the given
> > > > + *   remote port. Support added in Landlock ABI version 12.
> > > > + *
> > > > + * .. note:: The TCP and the MPTCP access rights are
> > > > independent,
> > > > even though
> > > > + *   they refer to the same port number space. Handling only
> > > > + *   %LANDLOCK_ACCESS_NET_BIND_TCP and
> > > > %LANDLOCK_ACCESS_NET_CONNECT_TCP leaves
> > > > + *   MPTCP sockets unrestricted, and vice versa. A sandbox
> > > > that
> > > > wants to
> > > > + *   control all TCP-based traffic needs to handle both sets.
> > > > + *
> > > > + * .. note:: These MPTCP access rights restrict the ports
> > > > passed
> > > > to
> > > > + *   :manpage:`bind(2)` and :manpage:`connect(2)`. The ports
> > > > used
> > > > in
> > > > MPTCP
> > > > + *   subflows are negotiated in the MPTCP protocol by the
> > > > kernel
> > > > and
> > > > are not
> > > > + *   subject to these restrictions.
> > > >   */
> > > >  /* 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)
> > > > +#define LANDLOCK_ACCESS_NET_BIND_MPTCP			(1ULL
> > > > <<
> > > > 4)
> > > > +#define LANDLOCK_ACCESS_NET_CONNECT_MPTCP		(1ULL
> > > > <<
> > > > 5)
> > > >  /* clang-format on */
> > > >  
> > > >  /**
> > > > diff --git a/security/landlock/limits.h
> > > > b/security/landlock/limits.h
> > > > index 1a7c5fb8f6fd..d25e056b7ca2 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_MAS
> > > > K_AC
> > > > CESS_FS)
> > > >  
> > > > -#define
> > > > LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_CONNECT_SE
> > > > ND_U
> > > > DP
> > > > +#define
> > > > LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_CONNECT_MP
> > > > TCP
> > > >  #define
> > > > LANDLOCK_MASK_ACCESS_NET	((LANDLOCK_LAST_ACCESS_NET
> > > > << 1) - 1)
> > > >  #define
> > > > LANDLOCK_NUM_ACCESS_NET		__const_hweight64(LANDLOCK_MAS
> > > > K_AC
> > > > CESS_NET)
> > > >  
> > > > diff --git a/security/landlock/net.c b/security/landlock/net.c
> > > > index 8f2aaac54b33..8541b0c07d64 100644
> > > > --- a/security/landlock/net.c
> > > > +++ b/security/landlock/net.c
> > > > @@ -11,6 +11,7 @@
> > > >  #include <linux/net.h>
> > > >  #include <linux/socket.h>
> > > >  #include <net/ipv6.h>
> > > > +#include <net/mptcp.h>
> > > >  
> > > >  #include "common.h"
> > > >  #include "cred.h"
> > > > @@ -53,6 +54,26 @@ int landlock_append_net_rule(struct
> > > > landlock_ruleset *const ruleset,
> > > >  	return err;
> > > >  }
> > > >  
> > > > +static bool sk_is_mptcp_socket(const struct sock *sk)
> > > > +{
> > > > +	return sk_is_inet(sk) && sk->sk_type == SOCK_STREAM &&
> > > > +	       sk->sk_protocol == IPPROTO_MPTCP;
> > > > +}
> > > 
> > > This helper should be placed in include/net/mptcp.h. I had
> > > already
> > > implemented one in [1], called sk_is_msk(), to differentiate it
> > > from
> > > sk_is_mptcp(). If you have no concerns with my implementation,
> > > please
> > > feel free to pick it up and use it in your series.
> > 
> > Sorry, let me correct myself. I thought about this more carefully,
> > and
> > the ideal place to define sk_is_msk() is in include/net/sock.h,
> > between
> > sk_is_tcp() and sk_is_udp():
> > 
> > static inline bool sk_is_tcp(const struct sock *sk) 
> > {
> >         return sk_is_inet(sk) &&
> >                sk->sk_type == SOCK_STREAM &&
> >                sk->sk_protocol == IPPROTO_TCP;
> > }
> > 
> > static inline bool sk_is_msk(const struct sock *sk) 
> > {
> >         return sk_is_inet(sk) &&
> >                sk->sk_type == SOCK_STREAM &&
> >                sk->sk_protocol == IPPROTO_MPTCP;
> > }
> > 
> > static inline bool sk_is_udp(const struct sock *sk) 
> > {
> >         return sk_is_inet(sk) &&
> >                sk->sk_type == SOCK_DGRAM &&
> >                sk->sk_protocol == IPPROTO_UDP;
> > }
> > 
> > If possible, please fold this code into this patch.
> 
> Thank you for the review, Geliang!
> 
> I'll include something like this in the next review round.
> (We are a bit bottlenecked on doing Landlock code reviews at the
> moment,
> so some delays are unfortunately expected.)
> 
> Small question about the function name sk_is_msk(): Is "msk" the
> right
> name for this?  It seems to be used as abbreviation for "MPTCP" in
> net/mptcp, but it's also used in net/mctp as an abbreviation, and I

Thanks for the heads-up! I hadn't considered that "msk" is also used as
an abbreviation in net/mctp.

> could not find a function name in include/net/mptcp.h which used
> "msk"
> to mean "MPTCP".  Should that say sk_is_mptcp() instead?

However, sk_is_mptcp() is already taken - it's defined in
include/net/mptcp.h and used to check whether a socket is an MPTCP
subflow (tcp_sk(sk)->is_mptcp).


So I'd like to propose the following rename scheme:

- Rename the existing sk_is_mptcp() (subflow check) to ssk_is_mptcp(),
where "ssk" stands for "subflow socket".

- Rename sk_is_msk() (the one I introduced here) to sk_is_mptcp(),
which checks whether the socket is the MPTCP socket itself.

This would give a consistent naming scheme:

  sk_is_mptcp()    - is this the MPTCP socket?
  ssk_is_mptcp()   - is this an MPTCP subflow?
  rsk_is_mptcp()   - is this an MPTCP request subflow?

We'll need to discuss this with the MPTCP maintainers before
proceeding.

Thanks,
-Geliang

> 
> Thanks,
> —Günther