Message-ID: <CAAVpQUAd==+Pw02+E6UC-qwaDNm7aFg+Q9YDbWzyniShAkAhFQ@mail.gmail.com>
Date: Thu, 8 Jan 2026 02:17:05 -0800
From: Kuniyuki Iwashima <kuniyu@...gle.com>
To: Günther Noack <gnoack@...gle.com>
Cc: Justin Suess <utilityemal77@...il.com>, Paul Moore <paul@...l-moore.com>, 
	James Morris <jmorris@...ei.org>, "Serge E . Hallyn" <serge@...lyn.com>, Simon Horman <horms@...nel.org>, 
	Mickaël Salaün <mic@...ikod.net>, 
	linux-security-module@...r.kernel.org, Tingmao Wang <m@...wtm.org>, 
	netdev@...r.kernel.org, Alexander Viro <viro@...iv.linux.org.uk>, 
	Christian Brauner <brauner@...nel.org>
Subject: Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect

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

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

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

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

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

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

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

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


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

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


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

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


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

Powered by blists - more mailing lists