Message-ID: <aV5WTGvQB0XI8Q_N@google.com>
Date: Wed, 7 Jan 2026 13:49:16 +0100
From: Günther Noack <gnoack@...gle.com>
To: Kuniyuki Iwashima <kuniyu@...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 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?

Some pieces of this which I am still unsure about:

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

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

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

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

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

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

—Günther

Powered by blists - more mailing lists