From: Kuniyuki Iwashima <kuniyu@amazon.com>
To: "David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Willem de Bruijn <willemb@google.com>
Cc: Simon Horman <horms@kernel.org>,
Christian Brauner <brauner@kernel.org>,
Kuniyuki Iwashima <kuniyu@amazon.com>,
Kuniyuki Iwashima <kuni1840@gmail.com>, <netdev@vger.kernel.org>
Subject: [PATCH v1 net-next 5/7] af_unix: Inherit sk_flags at connect().
Date: Wed, 7 May 2025 18:29:17 -0700 [thread overview]
Message-ID: <20250508013021.79654-6-kuniyu@amazon.com> (raw)
In-Reply-To: <20250508013021.79654-1-kuniyu@amazon.com>
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,
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.
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 (i.e., accepted sockets and clients).
Therefore, we moved SOCK_PASSXXX into sk->sk_flags, which does not
depend on struct socket.
Let’s inherit sk->sk_flags at connect() to avoid receiving SCM_RIGHTS
on embryo sockets created from a parent with SO_PASSRIGHTS=0.
While at it, whitespace issues around pid assignment have been fixed.
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
net/unix/af_unix.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index e793e55f6c9b..daa7a8ead243 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1631,6 +1631,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
newsk->sk_state = TCP_ESTABLISHED;
newsk->sk_type = sk->sk_type;
init_peercred(newsk);
+ sock_copy_flags(newsk, other);
newu = unix_sk(newsk);
newu->listener = other;
RCU_INIT_POINTER(newsk->sk_wq, &newu->peer_wq);
@@ -1747,7 +1748,6 @@ static int unix_accept(struct socket *sock, struct socket *newsock,
unix_state_lock(tsk);
unix_update_edges(unix_sk(tsk));
newsock->state = SS_CONNECTED;
- sock_copy_flags(tsk, sk);
sock_graft(tsk, newsock);
unix_state_unlock(tsk);
return 0;
@@ -1856,7 +1856,7 @@ static int unix_scm_to_skb(struct scm_cookie *scm, struct sk_buff *skb, bool sen
{
int err = 0;
- UNIXCB(skb).pid = get_pid(scm->pid);
+ UNIXCB(skb).pid = get_pid(scm->pid);
UNIXCB(skb).uid = scm->creds.uid;
UNIXCB(skb).gid = scm->creds.gid;
UNIXCB(skb).fp = NULL;
@@ -1879,9 +1879,8 @@ static void maybe_add_creds(struct sk_buff *skb, const struct sock *sk,
if (UNIXCB(skb).pid)
return;
- if (unix_passcred_enabled(sk) ||
- !other->sk_socket || unix_passcred_enabled(other)) {
- UNIXCB(skb).pid = get_pid(task_tgid(current));
+ if (unix_passcred_enabled(sk) || unix_passcred_enabled(other)) {
+ UNIXCB(skb).pid = get_pid(task_tgid(current));
current_uid_gid(&UNIXCB(skb).uid, &UNIXCB(skb).gid);
}
}
--
2.49.0
next prev parent reply other threads:[~2025-05-08 1:32 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-08 1:29 [PATCH v1 net-next 0/7] af_unix: Introduce SO_PASSRIGHTS Kuniyuki Iwashima
2025-05-08 1:29 ` [PATCH v1 net-next 1/7] af_unix: Factorise test_bit() for SOCK_PASSCRED and SOCK_PASSPIDFD Kuniyuki Iwashima
2025-05-08 1:29 ` [PATCH v1 net-next 2/7] af_unix: Don't pass struct socket to maybe_add_creds() Kuniyuki Iwashima
2025-05-08 1:29 ` [PATCH v1 net-next 3/7] scm: Move scm_recv() from scm.h to scm.c Kuniyuki Iwashima
2025-05-08 1:29 ` [PATCH v1 net-next 4/7] af_unix: Move SOCK_PASS{CRED,PIDFD,SEC} to sk->sk_flags Kuniyuki Iwashima
2025-05-09 3:15 ` Jakub Kicinski
2025-05-09 4:33 ` Kuniyuki Iwashima
2025-05-08 1:29 ` Kuniyuki Iwashima [this message]
2025-05-08 1:29 ` [PATCH v1 net-next 6/7] af_unix: Introduce SO_PASSRIGHTS Kuniyuki Iwashima
2025-05-08 1:29 ` [PATCH v1 net-next 7/7] selftest: af_unix: Test SO_PASSRIGHTS Kuniyuki Iwashima
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=20250508013021.79654-6-kuniyu@amazon.com \
--to=kuniyu@amazon.com \
--cc=brauner@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=kuni1840@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=willemb@google.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).