summaryrefslogtreecommitdiffstats
path: root/net
diff options
context:
space:
mode:
authorMagnus Karlsson <magnus.karlsson@intel.com>2020-11-20 12:53:39 +0100
committerDaniel Borkmann <daniel@iogearbox.net>2020-11-20 15:52:39 +0100
commit537cf4e3cc2f6cc9088dcd6162de573f603adc29 (patch)
treea847cbcf70b85c03cbd306b5708b7e610043e86e /net
parent6200d5c3831370cd0ab4b6455933d12e82ea9956 (diff)
downloadlinux-537cf4e3cc2f6cc9088dcd6162de573f603adc29.tar.bz2
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
Diffstat (limited to 'net')
-rw-r--r--net/xdp/xdp_umem.c19
-rw-r--r--net/xdp/xdp_umem.h2
-rw-r--r--net/xdp/xsk.c2
-rw-r--r--net/xdp/xsk_buff_pool.c2
4 files changed, 19 insertions, 6 deletions
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 56d052bc65cb..56a28a686988 100644
--- 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 181fdda2f2a8..aa9fe2780410 100644
--- 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 cfbec3989a76..5a6cdf7b320d 100644
--- 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 8a3bf4e1318e..3c5a1423d922 100644
--- 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);
}