xsk: Fix umem cleanup bug at socket destruct

Fix a bug that is triggered when a partially setup socket is
destroyed. For a fully setup socket, a socket that has been bound to a
device, the cleanup of the umem is performed at the end of the buffer
pool's cleanup work queue item. This has to be performed in a work
queue, and not in RCU cleanup, as it is doing a vunmap that cannot
execute in interrupt context. However, when a socket has only been
partially set up so that a umem has been created but the buffer pool
has not, the code erroneously directly calls the umem cleanup function
instead of using a work queue, and this leads to a BUG_ON() in
vunmap().

As there in this case is no buffer pool, we cannot use its work queue,
so we need to introduce a work queue for the umem and schedule this for
the cleanup. So in the case there is no pool, we are going to use the
umem's own work queue to schedule the cleanup. But if there is a
pool, the cleanup of the umem is still being performed by the pool's
work queue, as it is important that the umem is cleaned up after the
pool.

Fixes: e5e1a4bc916d ("xsk: Fix possible memory leak at socket close")
Reported-by: Marek Majtyka <marekx.majtyka@intel.com>
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Tested-by: Marek Majtyka <marekx.majtyka@intel.com>
Link: https://lore.kernel.org/bpf/1605873219-21629-1-git-send-email-magnus.karlsson@gmail.com

diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index 1a9559c0cbdda363fcc5489ddec51df963720174..4f4e93bf814c3e66b392f4bca8dd6e565685f0bc 100644 (file)
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -31,6 +31,7 @@ struct xdp_umem {
        struct page **pgs;
        int id;
        struct list_head xsk_dma_list;
+       struct work_struct work;
 };
 
 struct xsk_map {

diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 56d052bc65cb5bbe30eabcb532f9325c76b44872..56a28a686988d5590b45aa8e6fa967c2184472c3 100644 (file)
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -66,18 +66,31 @@ static void xdp_umem_release(struct xdp_umem *umem)
        kfree(umem);
 }
 
+static void xdp_umem_release_deferred(struct work_struct *work)
+{
+       struct xdp_umem *umem = container_of(work, struct xdp_umem, work);
+
+       xdp_umem_release(umem);
+}
+
 void xdp_get_umem(struct xdp_umem *umem)
 {
        refcount_inc(&umem->users);
 }
 
-void xdp_put_umem(struct xdp_umem *umem)
+void xdp_put_umem(struct xdp_umem *umem, bool defer_cleanup)
 {
        if (!umem)
                return;
 
-       if (refcount_dec_and_test(&umem->users))
-               xdp_umem_release(umem);
+       if (refcount_dec_and_test(&umem->users)) {
+               if (defer_cleanup) {
+                       INIT_WORK(&umem->work, xdp_umem_release_deferred);
+                       schedule_work(&umem->work);
+               } else {
+                       xdp_umem_release(umem);
+               }
+       }
 }
 
 static int xdp_umem_pin_pages(struct xdp_umem *umem, unsigned long address)

diff --git a/net/xdp/xdp_umem.h b/net/xdp/xdp_umem.h
index 181fdda2f2a84c9f213a62615115afeaa219405c..aa9fe2780410e77aaf45bba9781a619a195e9c91 100644 (file)
--- a/net/xdp/xdp_umem.h
+++ b/net/xdp/xdp_umem.h
@@ -9,7 +9,7 @@
 #include <net/xdp_sock_drv.h>
 
 void xdp_get_umem(struct xdp_umem *umem);
-void xdp_put_umem(struct xdp_umem *umem);
+void xdp_put_umem(struct xdp_umem *umem, bool defer_cleanup);
 struct xdp_umem *xdp_umem_create(struct xdp_umem_reg *mr);
 
 #endif /* XDP_UMEM_H_ */

diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index cfbec3989a769ebfb29140742c8e14dcd5712acb..5a6cdf7b320dc80a21436a046acafcb828f7aab7 100644 (file)
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -1147,7 +1147,7 @@ static void xsk_destruct(struct sock *sk)
                return;
 
        if (!xp_put_pool(xs->pool))
-               xdp_put_umem(xs->umem);
+               xdp_put_umem(xs->umem, !xs->pool);
 
        sk_refcnt_debug_dec(sk);
 }

diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 8a3bf4e1318e1d6e8e7c6cedbed878f8b0aa3811..3c5a1423d9227709f55ea7e9f1bff92b66ac85ad 100644 (file)
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -242,7 +242,7 @@ static void xp_release_deferred(struct work_struct *work)
                pool->cq = NULL;
        }
 
-       xdp_put_umem(pool->umem);
+       xdp_put_umem(pool->umem, false);
        xp_destroy(pool);
 }