From 25888e30319f8896fc656fc68643e6a078263060 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 25 Nov 2010 04:11:39 +0000 Subject: af_unix: limit recursion level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Its easy to eat all kernel memory and trigger NMI watchdog, using an exploit program that queues unix sockets on top of others. lkml ref : http://lkml.org/lkml/2010/11/25/8 This mechanism is used in applications, one choice we have is to have a recursion limit. Other limits might be needed as well (if we queue other types of files), since the passfd mechanism is currently limited by socket receive queue sizes only. Add a recursion_level to unix socket, allowing up to 4 levels. Each time we send an unix socket through sendfd mechanism, we copy its recursion level (plus one) to receiver. This recursion level is cleared when socket receive queue is emptied. Reported-by: Марк Коренберг Signed-off-by: Eric Dumazet Signed-off-by: David S. Miller --- net/unix/af_unix.c | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) (limited to 'net/unix/af_unix.c') diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 3c95304a081..2268e679812 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -1343,9 +1343,25 @@ static void unix_destruct_scm(struct sk_buff *skb) sock_wfree(skb); } +#define MAX_RECURSION_LEVEL 4 + static int unix_attach_fds(struct scm_cookie *scm, struct sk_buff *skb) { int i; + unsigned char max_level = 0; + int unix_sock_count = 0; + + for (i = scm->fp->count - 1; i >= 0; i--) { + struct sock *sk = unix_get_socket(scm->fp->fp[i]); + + if (sk) { + unix_sock_count++; + max_level = max(max_level, + unix_sk(sk)->recursion_level); + } + } + if (unlikely(max_level > MAX_RECURSION_LEVEL)) + return -ETOOMANYREFS; /* * Need to duplicate file references for the sake of garbage @@ -1356,9 +1372,11 @@ static int unix_attach_fds(struct scm_cookie *scm, struct sk_buff *skb) if (!UNIXCB(skb).fp) return -ENOMEM; - for (i = scm->fp->count-1; i >= 0; i--) - unix_inflight(scm->fp->fp[i]); - return 0; + if (unix_sock_count) { + for (i = scm->fp->count - 1; i >= 0; i--) + unix_inflight(scm->fp->fp[i]); + } + return max_level; } static int unix_scm_to_skb(struct scm_cookie *scm, struct sk_buff *skb, bool send_fds) @@ -1393,6 +1411,7 @@ static int unix_dgram_sendmsg(struct kiocb *kiocb, struct socket *sock, struct sk_buff *skb; long timeo; struct scm_cookie tmp_scm; + int max_level; if (NULL == siocb->scm) siocb->scm = &tmp_scm; @@ -1431,8 +1450,9 @@ static int unix_dgram_sendmsg(struct kiocb *kiocb, struct socket *sock, goto out; err = unix_scm_to_skb(siocb->scm, skb, true); - if (err) + if (err < 0) goto out_free; + max_level = err + 1; unix_get_secdata(siocb->scm, skb); skb_reset_transport_header(skb); @@ -1514,6 +1534,8 @@ restart: if (sock_flag(other, SOCK_RCVTSTAMP)) __net_timestamp(skb); skb_queue_tail(&other->sk_receive_queue, skb); + if (max_level > unix_sk(other)->recursion_level) + unix_sk(other)->recursion_level = max_level; unix_state_unlock(other); other->sk_data_ready(other, len); sock_put(other); @@ -1544,6 +1566,7 @@ static int unix_stream_sendmsg(struct kiocb *kiocb, struct socket *sock, int sent = 0; struct scm_cookie tmp_scm; bool fds_sent = false; + int max_level; if (NULL == siocb->scm) siocb->scm = &tmp_scm; @@ -1607,10 +1630,11 @@ static int unix_stream_sendmsg(struct kiocb *kiocb, struct socket *sock, /* Only send the fds in the first buffer */ err = unix_scm_to_skb(siocb->scm, skb, !fds_sent); - if (err) { + if (err < 0) { kfree_skb(skb); goto out_err; } + max_level = err + 1; fds_sent = true; err = memcpy_fromiovec(skb_put(skb, size), msg->msg_iov, size); @@ -1626,6 +1650,8 @@ static int unix_stream_sendmsg(struct kiocb *kiocb, struct socket *sock, goto pipe_err_free; skb_queue_tail(&other->sk_receive_queue, skb); + if (max_level > unix_sk(other)->recursion_level) + unix_sk(other)->recursion_level = max_level; unix_state_unlock(other); other->sk_data_ready(other, size); sent += size; @@ -1845,6 +1871,7 @@ static int unix_stream_recvmsg(struct kiocb *iocb, struct socket *sock, unix_state_lock(sk); skb = skb_dequeue(&sk->sk_receive_queue); if (skb == NULL) { + unix_sk(sk)->recursion_level = 0; if (copied >= target) goto unlock; -- cgit v1.2.3-70-g09d2 From 3610cda53f247e176bcbb7a7cca64bc53b12acdb Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Wed, 5 Jan 2011 15:38:53 -0800 Subject: af_unix: Avoid socket->sk NULL OOPS in stream connect security hooks. unix_release() can asynchornously set socket->sk to NULL, and it does so without holding the unix_state_lock() on "other" during stream connects. However, the reverse mapping, sk->sk_socket, is only transitioned to NULL under the unix_state_lock(). Therefore make the security hooks follow the reverse mapping instead of the forward mapping. Reported-by: Jeremy Fitzhardinge Reported-by: Linus Torvalds Signed-off-by: David S. Miller --- include/linux/security.h | 15 +++++++-------- net/unix/af_unix.c | 2 +- security/capability.c | 2 +- security/security.c | 3 +-- security/selinux/hooks.c | 10 +++++----- security/smack/smack_lsm.c | 14 +++++++------- 6 files changed, 22 insertions(+), 24 deletions(-) (limited to 'net/unix/af_unix.c') diff --git a/include/linux/security.h b/include/linux/security.h index fd4d55fb884..d47a4c24b3e 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -796,8 +796,9 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts) * @unix_stream_connect: * Check permissions before establishing a Unix domain stream connection * between @sock and @other. - * @sock contains the socket structure. - * @other contains the peer socket structure. + * @sock contains the sock structure. + * @other contains the peer sock structure. + * @newsk contains the new sock structure. * Return 0 if permission is granted. * @unix_may_send: * Check permissions before connecting or sending datagrams from @sock to @@ -1568,8 +1569,7 @@ struct security_operations { int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen); #ifdef CONFIG_SECURITY_NETWORK - int (*unix_stream_connect) (struct socket *sock, - struct socket *other, struct sock *newsk); + int (*unix_stream_connect) (struct sock *sock, struct sock *other, struct sock *newsk); int (*unix_may_send) (struct socket *sock, struct socket *other); int (*socket_create) (int family, int type, int protocol, int kern); @@ -2525,8 +2525,7 @@ static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32 #ifdef CONFIG_SECURITY_NETWORK -int security_unix_stream_connect(struct socket *sock, struct socket *other, - struct sock *newsk); +int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk); int security_unix_may_send(struct socket *sock, struct socket *other); int security_socket_create(int family, int type, int protocol, int kern); int security_socket_post_create(struct socket *sock, int family, @@ -2567,8 +2566,8 @@ void security_tun_dev_post_create(struct sock *sk); int security_tun_dev_attach(struct sock *sk); #else /* CONFIG_SECURITY_NETWORK */ -static inline int security_unix_stream_connect(struct socket *sock, - struct socket *other, +static inline int security_unix_stream_connect(struct sock *sock, + struct sock *other, struct sock *newsk) { return 0; diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 417d7a6c36c..dd419d28620 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -1157,7 +1157,7 @@ restart: goto restart; } - err = security_unix_stream_connect(sock, other->sk_socket, newsk); + err = security_unix_stream_connect(sk, other, newsk); if (err) { unix_state_unlock(sk); goto out_unlock; diff --git a/security/capability.c b/security/capability.c index c773635ca3a..2a5df2b7da8 100644 --- a/security/capability.c +++ b/security/capability.c @@ -548,7 +548,7 @@ static int cap_sem_semop(struct sem_array *sma, struct sembuf *sops, } #ifdef CONFIG_SECURITY_NETWORK -static int cap_unix_stream_connect(struct socket *sock, struct socket *other, +static int cap_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk) { return 0; diff --git a/security/security.c b/security/security.c index 1b798d3df71..e5fb07a3052 100644 --- a/security/security.c +++ b/security/security.c @@ -977,8 +977,7 @@ EXPORT_SYMBOL(security_inode_getsecctx); #ifdef CONFIG_SECURITY_NETWORK -int security_unix_stream_connect(struct socket *sock, struct socket *other, - struct sock *newsk) +int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk) { return security_ops->unix_stream_connect(sock, other, newsk); } diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index c82538a4b1a..6f637d2678a 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3921,18 +3921,18 @@ static int selinux_socket_shutdown(struct socket *sock, int how) return sock_has_perm(current, sock->sk, SOCKET__SHUTDOWN); } -static int selinux_socket_unix_stream_connect(struct socket *sock, - struct socket *other, +static int selinux_socket_unix_stream_connect(struct sock *sock, + struct sock *other, struct sock *newsk) { - struct sk_security_struct *sksec_sock = sock->sk->sk_security; - struct sk_security_struct *sksec_other = other->sk->sk_security; + struct sk_security_struct *sksec_sock = sock->sk_security; + struct sk_security_struct *sksec_other = other->sk_security; struct sk_security_struct *sksec_new = newsk->sk_security; struct common_audit_data ad; int err; COMMON_AUDIT_DATA_INIT(&ad, NET); - ad.u.net.sk = other->sk; + ad.u.net.sk = other; err = avc_has_perm(sksec_sock->sid, sksec_other->sid, sksec_other->sclass, diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 489a85afa47..ccb71a044a1 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -2408,22 +2408,22 @@ static int smack_setprocattr(struct task_struct *p, char *name, /** * smack_unix_stream_connect - Smack access on UDS - * @sock: one socket - * @other: the other socket + * @sock: one sock + * @other: the other sock * @newsk: unused * * Return 0 if a subject with the smack of sock could access * an object with the smack of other, otherwise an error code */ -static int smack_unix_stream_connect(struct socket *sock, - struct socket *other, struct sock *newsk) +static int smack_unix_stream_connect(struct sock *sock, + struct sock *other, struct sock *newsk) { - struct inode *sp = SOCK_INODE(sock); - struct inode *op = SOCK_INODE(other); + struct inode *sp = SOCK_INODE(sock->sk_socket); + struct inode *op = SOCK_INODE(other->sk_socket); struct smk_audit_info ad; smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_NET); - smk_ad_setfield_u_net_sk(&ad, other->sk); + smk_ad_setfield_u_net_sk(&ad, other); return smk_access(smk_of_inode(sp), smk_of_inode(op), MAY_READWRITE, &ad); } -- cgit v1.2.3-70-g09d2