netdev - Re: [PATCH v4 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: <68276c1118d32_2b92fe29428@willemb.c.googlers.com.notmuch>
Date: Fri, 16 May 2025 12:47: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 v4 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
> would not for SO_PASSRIGHTS; once an SCM_RIGHTS with a hung file
> descriptor was sent, it'd be game over.

Code LGTM, hence my Reviewed-by.

Just curious: could this case be handled in a way that does not
require receivers explicitly disabling a dangerous default mode?

IIUC the issue is the receiver taking a file reference using fget_raw
in scm_fp_copy from __scm_send, and if that is the last ref, it now
will hang the receiver process waiting to close this last ref?

If so, could the unwelcome ref be detected at accept, and taken from
the responsibility of this process? Worst case, assigned to some
zombie process.