Commit 35306eb2 authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller
Browse files

af_unix: fix races in sk_peer_pid and sk_peer_cred accesses 



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 b48596d1 "Bluetooth: L2CAP: Add get_peer_pid callback"
or implementing what was truly expected.

Fixes: 109f6e39 ("af_unix: Allow SO_PEERCRED to work across namespaces.")
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reported-by: default avatarJann 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: default avatarDavid S. Miller <davem@davemloft.net>
parent 656ed8b0

include/net/sock.h

+2 −0
Original line number Diff line number Diff line
@@ -488,8 +488,10 @@ struct sock { 
	u8			sk_prefer_busy_poll;

	u16			sk_busy_poll_budget;

#endif

	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

net/core/sock.c

+26 −6
Original line number Diff line number Diff line
@@ -1376,6 +1376,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)
@@ -1552,7 +1562,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;
@@ -1560,20 +1574,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;
@@ -1935,9 +1952,10 @@ static void __sk_destruct(struct rcu_head *head) 
		sk->sk_frag.page = NULL;

	}



	if (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);
@@ -3145,6 +3163,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;

net/unix/af_unix.c

+28 −6
Original line number Diff line number Diff line
@@ -608,20 +608,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)