[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1448548293.879369.450777185.798D081C@webmail.messagingengine.com>
Date: Thu, 26 Nov 2015 15:31:33 +0100
From: Hannes Frederic Sowa <hannes@...essinduktion.org>
To: Eric Dumazet <eric.dumazet@...il.com>
Cc: Rainer Weikusat <rweikusat@...ileactivedefense.com>,
Eric Dumazet <edumazet@...gle.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
Benjamin LaHaise <bcrl@...ck.org>,
"David S. Miller" <davem@...emloft.net>,
Al Viro <viro@...iv.linux.org.uk>,
David Howells <dhowells@...hat.com>,
Ying Xue <ying.xue@...driver.com>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
netdev <netdev@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
syzkaller <syzkaller@...glegroups.com>,
Kostya Serebryany <kcc@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
Sasha Levin <sasha.levin@...cle.com>
Subject: Re: use-after-free in sock_wake_async
On Thu, Nov 26, 2015, at 14:32, Hannes Frederic Sowa wrote:
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 7f89e4b..ae34da1 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1674,7 +1674,7 @@ static inline void sock_orphan(struct sock *sk)
> static inline void sock_graft(struct sock *sk, struct socket *parent)
> {
> write_lock_bh(&sk->sk_callback_lock);
> - sk->sk_wq = parent->wq;
> + sk->sk_wq = &parent->wq;
RCU_INIT_POINTER(sk->sk_wq, &parent->wq);
> parent->sk = sk;
> sk_set_socket(sk, parent);
> security_sock_graft(sk, parent);
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index 630c197..c125881 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -657,7 +657,7 @@ static void rcu_preempt_do_callbacks(void)
> /*
> * Queue a preemptible-RCU callback for invocation after a grace period.
> */
> -void call_rcu(struct rcu_head *head, rcu_callback_t func)
> +static void call_rcu(struct rcu_head *head, rcu_callback_t func)
> {
> __call_rcu(head, func, rcu_state_p, -1, 0);
> }
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 1e4dd54..314ab6a 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2383,7 +2383,7 @@ void sock_init_data(struct socket *sock, struct
> sock *sk)
>
> if (sock) {
> sk->sk_type = sock->type;
> - sk->sk_wq = sock->wq;
> + sk->sk_wq = &sock->wq;
RCU_INIT_POINTER()
> sock->sk = sk;
> } else
> sk->sk_wq = NULL;
> diff --git a/net/socket.c b/net/socket.c
> index dd2c247..495485e 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -245,19 +245,12 @@ static struct kmem_cache *sock_inode_cachep
> __read_mostly;
> static struct inode *sock_alloc_inode(struct super_block *sb)
> {
> struct socket_alloc *ei;
> - struct socket_wq *wq;
>
> ei = kmem_cache_alloc(sock_inode_cachep, GFP_KERNEL);
> if (!ei)
> return NULL;
> - wq = kmalloc(sizeof(*wq), GFP_KERNEL);
> - if (!wq) {
> - kmem_cache_free(sock_inode_cachep, ei);
> - return NULL;
> - }
> - init_waitqueue_head(&wq->wait);
> - wq->fasync_list = NULL;
> - RCU_INIT_POINTER(ei->socket.wq, wq);
> + init_waitqueue_head(&ei->socket.wq.wait);
> + ei->socket.wq.fasync_list = NULL;
>
> ei->socket.state = SS_UNCONNECTED;
> ei->socket.flags = 0;
> @@ -268,17 +261,18 @@ static struct inode *sock_alloc_inode(struct
> super_block *sb)
> return &ei->vfs_inode;
> }
>
> -static void sock_destroy_inode(struct inode *inode)
> +static void sock_cache_free_rcu(struct rcu_head *rcu)
> {
> - struct socket_alloc *ei;
> - struct socket_wq *wq;
> -
> - ei = container_of(inode, struct socket_alloc, vfs_inode);
> - wq = rcu_dereference_protected(ei->socket.wq, 1);
> - kfree_rcu(wq, rcu);
> + struct socket_alloc *ei =
> + container_of(rcu, struct socket_alloc, vfs_inode.i_rcu);
> kmem_cache_free(sock_inode_cachep, ei);
> }
>
> +static void sock_destroy_inode(struct inode *inode)
> +{
> + call_rcu(&inode->i_rcu, sock_cache_free_rcu);
> +}
> +
> static void init_once(void *foo)
> {
> struct socket_alloc *ei = (struct socket_alloc *)foo;
> @@ -573,7 +567,7 @@ void sock_release(struct socket *sock)
> module_put(owner);
> }
>
> - if (rcu_dereference_protected(sock->wq, 1)->fasync_list)
> + if (sock->wq.fasync_list)
> pr_err("%s: fasync list not empty!\n", __func__);
>
> this_cpu_sub(sockets_in_use, 1);
> @@ -1044,7 +1038,7 @@ static int sock_fasync(int fd, struct file *filp,
> int on)
> return -EINVAL;
>
> lock_sock(sk);
> - wq = rcu_dereference_protected(sock->wq, sock_owned_by_user(sk));
> + wq = &sock->wq;
> fasync_helper(fd, filp, on, &wq->fasync_list);
>
> if (!wq->fasync_list)
> @@ -1065,7 +1059,7 @@ int sock_wake_async(struct socket *sock, int how,
> int band)
> if (!sock)
> return -1;
> rcu_read_lock();
> - wq = rcu_dereference(sock->wq);
> + wq = &sock->wq;
> if (!wq || !wq->fasync_list) {
> rcu_read_unlock();
> return -1;
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists