Re: use-after-free in sock_wake_async

[Hannes Frederic Sowa <hannes@stressinduktion.org>]

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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html