| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAAVpQUC1Z=eUcX9RqE7PLRvUPVHeuc8X7dnk1Vr_6w0_t+V84A@mail.gmail.com>
Date: Mon, 30 Jun 2025 11:57:52 -0700
From: Kuniyuki Iwashima <kuniyu@...gle.com>
To: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
Cc: linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
Leon Romanovsky <leon@...nel.org>, Arnd Bergmann <arnd@...db.de>, Christian Brauner <brauner@...nel.org>,
Lennart Poettering <mzxreary@...inter.de>, Luca Boccassi <bluca@...ian.org>,
David Rheinsberg <david@...dahead.eu>, Kuniyuki Iwashima <kuniyu@...zon.com>
Subject: Re: [RESEND PATCH net-next 1/6] af_unix: rework unix_maybe_add_creds()
to allow sleep
On Sun, Jun 29, 2025 at 2:45 PM Alexander Mikhalitsyn
<aleksandr.mikhalitsyn@...onical.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@...r.kernel.org
> Cc: netdev@...r.kernel.org
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: Paolo Abeni <pabeni@...hat.com>
> Cc: Simon Horman <horms@...nel.org>
> Cc: Leon Romanovsky <leon@...nel.org>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Christian Brauner <brauner@...nel.org>
> Cc: Kuniyuki Iwashima <kuniyu@...gle.com>
> Cc: Lennart Poettering <mzxreary@...inter.de>
> Cc: Luca Boccassi <bluca@...ian.org>
> Cc: David Rheinsberg <david@...dahead.eu>
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.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
>
Powered by blists - more mailing lists