All of lore.kernel.org
 help / color / mirror / Atom feed
From: Justin Suess <utilityemal77@gmail.com>
To: mic@digikod.net
Cc: brauner@kernel.org, demiobenour@gmail.com,
	fahimitahera@gmail.com, gnoack3000@gmail.com, hi@alyssa.is,
	horms@kernel.org, ivanov.mikhail1@huawei-partners.com,
	jannh@google.com, jmorris@namei.org, john.johansen@canonical.com,
	konstantin.meskhidze@huawei.com,
	linux-security-module@vger.kernel.org, m@maowtm.org,
	matthieu@buffet.re, netdev@vger.kernel.org, paul@paul-moore.com,
	samasth.norway.ananda@oracle.com, serge@hallyn.com,
	utilityemal77@gmail.com, viro@zeniv.linux.org.uk
Subject: Re: [PATCH v4 1/6] lsm: Add LSM hook security_unix_find
Date: Tue, 10 Feb 2026 08:02:28 -0500	[thread overview]
Message-ID: <20260210130232.212260-1-utilityemal77@gmail.com> (raw)
In-Reply-To: <20260209.yeeh3ieDuz9u@digikod.net>

> 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.

Done. I debated keeping it, but it seems more appropriate to follow the
convention. Thanks for the catch.

> 
> 
> >  	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.

Done.

> > +	 */
> > +	err = security_unix_find(&path, sk, flags);
> 
> This hook makes sense and is quite generic.

Indeed, I suspect it will be useful for other path-based LSM.

> > +	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: /**

Done.

> 
> > + * 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
> > 
> > 
>

Below follows revised lsm hook patch based on feedback.

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..369812b79dd8 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1230,6 +1230,14 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
 	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.
+	 */
+	err = security_unix_find(&path, sk, flags);
+	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..eaf8f8fdf0c2 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)
+/**
+ * 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

  parent reply	other threads:[~2026-02-10 13:02 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-08 23:10 [PATCH v4 0/6] landlock: UNIX connect() control by pathname and scope Günther Noack
2026-02-08 23:10 ` [PATCH v4 1/6] lsm: Add LSM hook security_unix_find Günther Noack
2026-02-09 17:51   ` Mickaël Salaün
2026-02-09 18:33     ` Tingmao Wang
2026-02-09 19:53       ` Tingmao Wang
2026-02-10 13:02     ` Justin Suess [this message]
2026-02-08 23:10 ` [PATCH v4 2/6] landlock: Control pathname UNIX domain socket resolution by path Günther Noack
2026-02-09 10:21   ` Günther Noack
2026-02-09 13:11     ` Justin Suess
2026-02-10 23:04       ` Günther Noack
2026-02-09 17:28     ` Mickaël Salaün
2026-02-10 23:09       ` Günther Noack
2026-02-09 18:03     ` Mickaël Salaün
2026-02-08 23:10 ` [PATCH v4 3/6] samples/landlock: Add support for named UNIX domain socket restrictions Günther Noack
2026-02-08 23:10 ` [PATCH v4 4/6] landlock/selftests: Test " Günther Noack
2026-02-09 17:29   ` Mickaël Salaün
2026-02-15  3:01     ` Tingmao Wang
2026-02-08 23:10 ` [PATCH v4 5/6] landlock: Document FS access right for pathname UNIX sockets Günther Noack
2026-02-08 23:10 ` [PATCH v4 6/6] landlock: Document design rationale for scoped access rights Günther Noack

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260210130232.212260-1-utilityemal77@gmail.com \
    --to=utilityemal77@gmail.com \
    --cc=brauner@kernel.org \
    --cc=demiobenour@gmail.com \
    --cc=fahimitahera@gmail.com \
    --cc=gnoack3000@gmail.com \
    --cc=hi@alyssa.is \
    --cc=horms@kernel.org \
    --cc=ivanov.mikhail1@huawei-partners.com \
    --cc=jannh@google.com \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=konstantin.meskhidze@huawei.com \
    --cc=linux-security-module@vger.kernel.org \
    --cc=m@maowtm.org \
    --cc=matthieu@buffet.re \
    --cc=mic@digikod.net \
    --cc=netdev@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=samasth.norway.ananda@oracle.com \
    --cc=serge@hallyn.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.