netlink: Do not schedule work from sk_destruct
authorHerbert Xu <herbert@gondor.apana.org.au>
Mon, 5 Dec 2016 07:28:21 +0000 (15:28 +0800)
committerDavid S. Miller <davem@davemloft.net>
Tue, 6 Dec 2016 00:43:42 +0000 (19:43 -0500)
It is wrong to schedule a work from sk_destruct using the socket
as the memory reserve because the socket will be freed immediately
after the return from sk_destruct.

Instead we should do the deferral prior to sk_free.

This patch does just that.

Fixes: 707693c8a498 ("netlink: Call cb->done from a worker thread")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/netlink/af_netlink.c

index 602e5ebe9db39ec6c72708628bc48efad9f0e680..246f29d365c09a7761b8ae0f0a2acd6de8a6f653 100644 (file)
@@ -322,11 +322,13 @@ static void netlink_skb_set_owner_r(struct sk_buff *skb, struct sock *sk)
        sk_mem_charge(sk, skb->truesize);
 }
 
-static void __netlink_sock_destruct(struct sock *sk)
+static void netlink_sock_destruct(struct sock *sk)
 {
        struct netlink_sock *nlk = nlk_sk(sk);
 
        if (nlk->cb_running) {
+               if (nlk->cb.done)
+                       nlk->cb.done(&nlk->cb);
                module_put(nlk->cb.module);
                kfree_skb(nlk->cb.skb);
        }
@@ -348,21 +350,7 @@ static void netlink_sock_destruct_work(struct work_struct *work)
        struct netlink_sock *nlk = container_of(work, struct netlink_sock,
                                                work);
 
-       nlk->cb.done(&nlk->cb);
-       __netlink_sock_destruct(&nlk->sk);
-}
-
-static void netlink_sock_destruct(struct sock *sk)
-{
-       struct netlink_sock *nlk = nlk_sk(sk);
-
-       if (nlk->cb_running && nlk->cb.done) {
-               INIT_WORK(&nlk->work, netlink_sock_destruct_work);
-               schedule_work(&nlk->work);
-               return;
-       }
-
-       __netlink_sock_destruct(sk);
+       sk_free(&nlk->sk);
 }
 
 /* This lock without WQ_FLAG_EXCLUSIVE is good on UP and it is _very_ bad on
@@ -667,8 +655,18 @@ out_module:
 static void deferred_put_nlk_sk(struct rcu_head *head)
 {
        struct netlink_sock *nlk = container_of(head, struct netlink_sock, rcu);
+       struct sock *sk = &nlk->sk;
+
+       if (!atomic_dec_and_test(&sk->sk_refcnt))
+               return;
+
+       if (nlk->cb_running && nlk->cb.done) {
+               INIT_WORK(&nlk->work, netlink_sock_destruct_work);
+               schedule_work(&nlk->work);
+               return;
+       }
 
-       sock_put(&nlk->sk);
+       sk_free(sk);
 }
 
 static int netlink_release(struct socket *sock)