[RESEND PATCH net-next 1/6] af_unix: rework unix_maybe_add_creds() to allow sleep

Alexander Mikhalitsyn posted 6 patches 3 months, 1 week ago
There is a newer version of this series
[RESEND PATCH net-next 1/6] af_unix: rework unix_maybe_add_creds() to allow sleep
Posted by Alexander Mikhalitsyn 3 months, 1 week ago
As a preparation for the next patches we need to allow sleeping
in unix_maybe_add_creds() and also return err. Currently, we can't do
that as unix_maybe_add_creds() is being called under unix_state_lock().
There is no need for this, really. So let's move call sites of
this helper a bit and do necessary function signature changes.

Cc: linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kuniyuki Iwashima <kuniyu@google.com>
Cc: Lennart Poettering <mzxreary@0pointer.de>
Cc: Luca Boccassi <bluca@debian.org>
Cc: David Rheinsberg <david@readahead.eu>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
---
 net/unix/af_unix.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 129388c309b0..6072d89ce2e7 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1955,21 +1955,26 @@ static int unix_scm_to_skb(struct scm_cookie *scm, struct sk_buff *skb, bool sen
 	return err;
 }
 
-/*
+/* unix_maybe_add_creds() adds current task uid/gid and struct pid to skb if needed.
+ *
  * Some apps rely on write() giving SCM_CREDENTIALS
  * We include credentials if source or destination socket
  * asserted SOCK_PASSCRED.
+ *
+ * Context: May sleep.
  */
-static void unix_maybe_add_creds(struct sk_buff *skb, const struct sock *sk,
-				 const struct sock *other)
+static int unix_maybe_add_creds(struct sk_buff *skb, const struct sock *sk,
+				const struct sock *other)
 {
 	if (UNIXCB(skb).pid)
-		return;
+		return 0;
 
 	if (unix_may_passcred(sk) || unix_may_passcred(other)) {
 		UNIXCB(skb).pid = get_pid(task_tgid(current));
 		current_uid_gid(&UNIXCB(skb).uid, &UNIXCB(skb).gid);
 	}
+
+	return 0;
 }
 
 static bool unix_skb_scm_eq(struct sk_buff *skb,
@@ -2104,6 +2109,10 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 		goto out_sock_put;
 	}
 
+	err = unix_maybe_add_creds(skb, sk, other);
+	if (err)
+		goto out_sock_put;
+
 restart:
 	sk_locked = 0;
 	unix_state_lock(other);
@@ -2212,7 +2221,6 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 	if (sock_flag(other, SOCK_RCVTSTAMP))
 		__net_timestamp(skb);
 
-	unix_maybe_add_creds(skb, sk, other);
 	scm_stat_add(other, skb);
 	skb_queue_tail(&other->sk_receive_queue, skb);
 	unix_state_unlock(other);
@@ -2256,6 +2264,10 @@ static int queue_oob(struct sock *sk, struct msghdr *msg, struct sock *other,
 	if (err < 0)
 		goto out;
 
+	err = unix_maybe_add_creds(skb, sk, other);
+	if (err)
+		goto out;
+
 	skb_put(skb, 1);
 	err = skb_copy_datagram_from_iter(skb, 0, &msg->msg_iter, 1);
 
@@ -2275,7 +2287,6 @@ static int queue_oob(struct sock *sk, struct msghdr *msg, struct sock *other,
 		goto out_unlock;
 	}
 
-	unix_maybe_add_creds(skb, sk, other);
 	scm_stat_add(other, skb);
 
 	spin_lock(&other->sk_receive_queue.lock);
@@ -2369,6 +2380,10 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 
 		fds_sent = true;
 
+		err = unix_maybe_add_creds(skb, sk, other);
+		if (err)
+			goto out_free;
+
 		if (unlikely(msg->msg_flags & MSG_SPLICE_PAGES)) {
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 			err = skb_splice_from_iter(skb, &msg->msg_iter, size,
@@ -2399,7 +2414,6 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 			goto out_free;
 		}
 
-		unix_maybe_add_creds(skb, sk, other);
 		scm_stat_add(other, skb);
 		skb_queue_tail(&other->sk_receive_queue, skb);
 		unix_state_unlock(other);
-- 
2.43.0
Re: [RESEND PATCH net-next 1/6] af_unix: rework unix_maybe_add_creds() to allow sleep
Posted by Christian Brauner 3 months, 1 week ago
On Sun, Jun 29, 2025 at 11:44:38PM +0200, Alexander Mikhalitsyn wrote:
> As a preparation for the next patches we need to allow sleeping
> in unix_maybe_add_creds() and also return err. Currently, we can't do
> that as unix_maybe_add_creds() is being called under unix_state_lock().
> There is no need for this, really. So let's move call sites of
> this helper a bit and do necessary function signature changes.
> 
> Cc: linux-kernel@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Simon Horman <horms@kernel.org>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Kuniyuki Iwashima <kuniyu@google.com>
> Cc: Lennart Poettering <mzxreary@0pointer.de>
> Cc: Luca Boccassi <bluca@debian.org>
> Cc: David Rheinsberg <david@readahead.eu>
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
> ---

This looks good to me.
Please feel free to carry my RvB post the minor fixes requested by
Kuniyuki:

Reviewed-by: Christian Brauner <brauner@kernel.org>
Re: [RESEND PATCH net-next 1/6] af_unix: rework unix_maybe_add_creds() to allow sleep
Posted by Kuniyuki Iwashima 3 months, 1 week ago
On Sun, Jun 29, 2025 at 2:45 PM Alexander Mikhalitsyn
<aleksandr.mikhalitsyn@canonical.com> wrote:
>
> As a preparation for the next patches we need to allow sleeping
> in unix_maybe_add_creds() and also return err. Currently, we can't do
> that as unix_maybe_add_creds() is being called under unix_state_lock().
> There is no need for this, really. So let's move call sites of
> this helper a bit and do necessary function signature changes.
>
> Cc: linux-kernel@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Simon Horman <horms@kernel.org>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Kuniyuki Iwashima <kuniyu@google.com>
> Cc: Lennart Poettering <mzxreary@0pointer.de>
> Cc: Luca Boccassi <bluca@debian.org>
> Cc: David Rheinsberg <david@readahead.eu>
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
> ---
>  net/unix/af_unix.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 129388c309b0..6072d89ce2e7 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1955,21 +1955,26 @@ static int unix_scm_to_skb(struct scm_cookie *scm, struct sk_buff *skb, bool sen
>         return err;
>  }
>
> -/*
> +/* unix_maybe_add_creds() adds current task uid/gid and struct pid to skb if needed.

This is not a correct kdoc format.
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation

> + *
>   * Some apps rely on write() giving SCM_CREDENTIALS
>   * We include credentials if source or destination socket
>   * asserted SOCK_PASSCRED.
> + *
> + * Context: May sleep.

This should be added later when this function starts to sleep.


>   */
> -static void unix_maybe_add_creds(struct sk_buff *skb, const struct sock *sk,
> -                                const struct sock *other)
> +static int unix_maybe_add_creds(struct sk_buff *skb, const struct sock *sk,
> +                               const struct sock *other)
>  {
>         if (UNIXCB(skb).pid)
> -               return;
> +               return 0;
>
>         if (unix_may_passcred(sk) || unix_may_passcred(other)) {
>                 UNIXCB(skb).pid = get_pid(task_tgid(current));
>                 current_uid_gid(&UNIXCB(skb).uid, &UNIXCB(skb).gid);
>         }
> +
> +       return 0;
>  }
>
>  static bool unix_skb_scm_eq(struct sk_buff *skb,
> @@ -2104,6 +2109,10 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
>                 goto out_sock_put;
>         }
>
> +       err = unix_maybe_add_creds(skb, sk, other);
> +       if (err)
> +               goto out_sock_put;
> +
>  restart:
>         sk_locked = 0;
>         unix_state_lock(other);
> @@ -2212,7 +2221,6 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
>         if (sock_flag(other, SOCK_RCVTSTAMP))
>                 __net_timestamp(skb);
>
> -       unix_maybe_add_creds(skb, sk, other);
>         scm_stat_add(other, skb);
>         skb_queue_tail(&other->sk_receive_queue, skb);
>         unix_state_unlock(other);
> @@ -2256,6 +2264,10 @@ static int queue_oob(struct sock *sk, struct msghdr *msg, struct sock *other,
>         if (err < 0)
>                 goto out;
>
> +       err = unix_maybe_add_creds(skb, sk, other);
> +       if (err)
> +               goto out;
> +
>         skb_put(skb, 1);
>         err = skb_copy_datagram_from_iter(skb, 0, &msg->msg_iter, 1);
>
> @@ -2275,7 +2287,6 @@ static int queue_oob(struct sock *sk, struct msghdr *msg, struct sock *other,
>                 goto out_unlock;
>         }
>
> -       unix_maybe_add_creds(skb, sk, other);
>         scm_stat_add(other, skb);
>
>         spin_lock(&other->sk_receive_queue.lock);
> @@ -2369,6 +2380,10 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
>
>                 fds_sent = true;
>
> +               err = unix_maybe_add_creds(skb, sk, other);
> +               if (err)
> +                       goto out_free;
> +
>                 if (unlikely(msg->msg_flags & MSG_SPLICE_PAGES)) {
>                         skb->ip_summed = CHECKSUM_UNNECESSARY;
>                         err = skb_splice_from_iter(skb, &msg->msg_iter, size,
> @@ -2399,7 +2414,6 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
>                         goto out_free;
>                 }
>
> -               unix_maybe_add_creds(skb, sk, other);
>                 scm_stat_add(other, skb);
>                 skb_queue_tail(&other->sk_receive_queue, skb);
>                 unix_state_unlock(other);
> --
> 2.43.0
>