netdev - Re: [PATCH v2 net-next 7/9] af_unix: Inherit sk

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <68224e25b13ac_eb9592943d@willemb.c.googlers.com.notmuch>
Date: Mon, 12 May 2025 15:38:13 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>, 
 "David S. Miller" <davem@...emloft.net>, 
 Eric Dumazet <edumazet@...gle.com>, 
 Jakub Kicinski <kuba@...nel.org>, 
 Paolo Abeni <pabeni@...hat.com>, 
 Willem de Bruijn <willemb@...gle.com>
Cc: Simon Horman <horms@...nel.org>, 
 Christian Brauner <brauner@...nel.org>, 
 Kuniyuki Iwashima <kuniyu@...zon.com>, 
 Kuniyuki Iwashima <kuni1840@...il.com>, 
 netdev@...r.kernel.org
Subject: Re: [PATCH v2 net-next 7/9] af_unix: Inherit sk_flags at connect().

Kuniyuki Iwashima wrote:
> For SOCK_STREAM embryo sockets, the SO_PASS{CRED,PIDFD,SEC} options
> are inherited from the parent listen()ing socket.
> 
> Currently, this inheritance happens at accept(), because these
> attributes were stored in sk->sk_socket->flags and the struct socket
> is not allocated until accept().
> 
> This leads to unintentional behaviour.
> 
> When a peer sends data to an embryo socket in the accept() queue,
> unix_maybe_add_creds() embeds credentials into the skb, even if
> neither the peer nor the listener has enabled these options.
> 
> If the option is enabled, the embryo socket receives the ancillary
> data after accept().  If not, the data is silently discarded.
> 
> This conservative approach works for SO_PASS{CRED,PIDFD,SEC}, but not
> for SO_PASSRIGHTS; once an SCM_RIGHTS with a hung file descriptor is
> sent, it’s game over.

Should this be a fix to net then?

It depends on the move of this one bit from socket to sock. So is not
a stand-alone patch. But does not need all of the previous cleanup
patches if needs to be backportable.

> 
> To avoid this, we will need to preserve SOCK_PASSRIGHTS even on embryo
> sockets.
> 
> A recent change made it possible to access the parent's flags in
> sendmsg() via unix_sk(other)->listener->sk->sk_socket->flags, but
> this introduces an unnecessary condition that is irrelevant for
> most sockets, accept()ed sockets and clients.

What is this condition and how is it irrelevant? A constraint on the
kernel having the recent change? I.e., not backportable?

> Therefore, we moved SOCK_PASSXXX into struct sock.
> 
> Let’s inherit sk->sk_scm_recv_flags at connect() to avoid receiving
> SCM_RIGHTS on embryo sockets created from a parent with SO_PASSRIGHTS=0.
> 
> Now, we can remove !other->sk_socket check in unix_maybe_add_creds()
> to avoid slow SOCK_PASS{CRED,PIDFD} handling for embryo sockets
> created from a parent with SO_PASS{CRED,PIDFD}=0.
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>