| From ba9a57e349fcebc897540d7d6c7f311c4a1d1c5b Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Wed, 29 Sep 2021 15:57:50 -0700 |
| Subject: af_unix: fix races in sk_peer_pid and sk_peer_cred accesses |
| |
| From: Eric Dumazet <edumazet@google.com> |
| |
| [ Upstream commit 35306eb23814444bd4021f8a1c3047d3cb0c8b2b ] |
| |
| Jann Horn reported that SO_PEERCRED and SO_PEERGROUPS implementations |
| are racy, as af_unix can concurrently change sk_peer_pid and sk_peer_cred. |
| |
| In order to fix this issue, this patch adds a new spinlock that needs |
| to be used whenever these fields are read or written. |
| |
| Jann also pointed out that l2cap_sock_get_peer_pid_cb() is currently |
| reading sk->sk_peer_pid which makes no sense, as this field |
| is only possibly set by AF_UNIX sockets. |
| We will have to clean this in a separate patch. |
| This could be done by reverting b48596d1dc25 "Bluetooth: L2CAP: Add get_peer_pid callback" |
| or implementing what was truly expected. |
| |
| Fixes: 109f6e39fa07 ("af_unix: Allow SO_PEERCRED to work across namespaces.") |
| Signed-off-by: Eric Dumazet <edumazet@google.com> |
| Reported-by: Jann Horn <jannh@google.com> |
| Cc: Eric W. Biederman <ebiederm@xmission.com> |
| Cc: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> |
| Cc: Marcel Holtmann <marcel@holtmann.org> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| include/net/sock.h | 2 ++ |
| net/core/sock.c | 32 ++++++++++++++++++++++++++------ |
| net/unix/af_unix.c | 34 ++++++++++++++++++++++++++++------ |
| 3 files changed, 56 insertions(+), 12 deletions(-) |
| |
| diff --git a/include/net/sock.h b/include/net/sock.h |
| index 3c7addf95150..cdca984f3630 100644 |
| --- a/include/net/sock.h |
| +++ b/include/net/sock.h |
| @@ -479,8 +479,10 @@ struct sock { |
| u32 sk_ack_backlog; |
| u32 sk_max_ack_backlog; |
| kuid_t sk_uid; |
| + spinlock_t sk_peer_lock; |
| struct pid *sk_peer_pid; |
| const struct cred *sk_peer_cred; |
| + |
| long sk_rcvtimeo; |
| ktime_t sk_stamp; |
| #if BITS_PER_LONG==32 |
| diff --git a/net/core/sock.c b/net/core/sock.c |
| index d638c5361ed2..f9c835167391 100644 |
| --- a/net/core/sock.c |
| +++ b/net/core/sock.c |
| @@ -1255,6 +1255,16 @@ int sock_setsockopt(struct socket *sock, int level, int optname, |
| } |
| EXPORT_SYMBOL(sock_setsockopt); |
| |
| +static const struct cred *sk_get_peer_cred(struct sock *sk) |
| +{ |
| + const struct cred *cred; |
| + |
| + spin_lock(&sk->sk_peer_lock); |
| + cred = get_cred(sk->sk_peer_cred); |
| + spin_unlock(&sk->sk_peer_lock); |
| + |
| + return cred; |
| +} |
| |
| static void cred_to_ucred(struct pid *pid, const struct cred *cred, |
| struct ucred *ucred) |
| @@ -1428,7 +1438,11 @@ int sock_getsockopt(struct socket *sock, int level, int optname, |
| struct ucred peercred; |
| if (len > sizeof(peercred)) |
| len = sizeof(peercred); |
| + |
| + spin_lock(&sk->sk_peer_lock); |
| cred_to_ucred(sk->sk_peer_pid, sk->sk_peer_cred, &peercred); |
| + spin_unlock(&sk->sk_peer_lock); |
| + |
| if (copy_to_user(optval, &peercred, len)) |
| return -EFAULT; |
| goto lenout; |
| @@ -1436,20 +1450,23 @@ int sock_getsockopt(struct socket *sock, int level, int optname, |
| |
| case SO_PEERGROUPS: |
| { |
| + const struct cred *cred; |
| int ret, n; |
| |
| - if (!sk->sk_peer_cred) |
| + cred = sk_get_peer_cred(sk); |
| + if (!cred) |
| return -ENODATA; |
| |
| - n = sk->sk_peer_cred->group_info->ngroups; |
| + n = cred->group_info->ngroups; |
| if (len < n * sizeof(gid_t)) { |
| len = n * sizeof(gid_t); |
| + put_cred(cred); |
| return put_user(len, optlen) ? -EFAULT : -ERANGE; |
| } |
| len = n * sizeof(gid_t); |
| |
| - ret = groups_to_user((gid_t __user *)optval, |
| - sk->sk_peer_cred->group_info); |
| + ret = groups_to_user((gid_t __user *)optval, cred->group_info); |
| + put_cred(cred); |
| if (ret) |
| return ret; |
| goto lenout; |
| @@ -1788,9 +1805,10 @@ static void __sk_destruct(struct rcu_head *head) |
| sk->sk_frag.page = NULL; |
| } |
| |
| - if (sk->sk_peer_cred) |
| - put_cred(sk->sk_peer_cred); |
| + /* We do not need to acquire sk->sk_peer_lock, we are the last user. */ |
| + put_cred(sk->sk_peer_cred); |
| put_pid(sk->sk_peer_pid); |
| + |
| if (likely(sk->sk_net_refcnt)) |
| put_net(sock_net(sk)); |
| sk_prot_free(sk->sk_prot_creator, sk); |
| @@ -3000,6 +3018,8 @@ void sock_init_data(struct socket *sock, struct sock *sk) |
| |
| sk->sk_peer_pid = NULL; |
| sk->sk_peer_cred = NULL; |
| + spin_lock_init(&sk->sk_peer_lock); |
| + |
| sk->sk_write_pending = 0; |
| sk->sk_rcvlowat = 1; |
| sk->sk_rcvtimeo = MAX_SCHEDULE_TIMEOUT; |
| diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c |
| index d5c0ae34b1e4..b7edca89e0ba 100644 |
| --- a/net/unix/af_unix.c |
| +++ b/net/unix/af_unix.c |
| @@ -593,20 +593,42 @@ static void unix_release_sock(struct sock *sk, int embrion) |
| |
| static void init_peercred(struct sock *sk) |
| { |
| - put_pid(sk->sk_peer_pid); |
| - if (sk->sk_peer_cred) |
| - put_cred(sk->sk_peer_cred); |
| + const struct cred *old_cred; |
| + struct pid *old_pid; |
| + |
| + spin_lock(&sk->sk_peer_lock); |
| + old_pid = sk->sk_peer_pid; |
| + old_cred = sk->sk_peer_cred; |
| sk->sk_peer_pid = get_pid(task_tgid(current)); |
| sk->sk_peer_cred = get_current_cred(); |
| + spin_unlock(&sk->sk_peer_lock); |
| + |
| + put_pid(old_pid); |
| + put_cred(old_cred); |
| } |
| |
| static void copy_peercred(struct sock *sk, struct sock *peersk) |
| { |
| - put_pid(sk->sk_peer_pid); |
| - if (sk->sk_peer_cred) |
| - put_cred(sk->sk_peer_cred); |
| + const struct cred *old_cred; |
| + struct pid *old_pid; |
| + |
| + if (sk < peersk) { |
| + spin_lock(&sk->sk_peer_lock); |
| + spin_lock_nested(&peersk->sk_peer_lock, SINGLE_DEPTH_NESTING); |
| + } else { |
| + spin_lock(&peersk->sk_peer_lock); |
| + spin_lock_nested(&sk->sk_peer_lock, SINGLE_DEPTH_NESTING); |
| + } |
| + old_pid = sk->sk_peer_pid; |
| + old_cred = sk->sk_peer_cred; |
| sk->sk_peer_pid = get_pid(peersk->sk_peer_pid); |
| sk->sk_peer_cred = get_cred(peersk->sk_peer_cred); |
| + |
| + spin_unlock(&sk->sk_peer_lock); |
| + spin_unlock(&peersk->sk_peer_lock); |
| + |
| + put_pid(old_pid); |
| + put_cred(old_cred); |
| } |
| |
| static int unix_listen(struct socket *sock, int backlog) |
| -- |
| 2.33.0 |
| |