[PATCH net v2] tun: bound the NAPI receive queue

[PATCH net v2] tun: bound the NAPI receive queue

[Antonio Quartulli]

With IFF_NAPI, tun_get_user() queues every written packet for NAPI on
an unbounded list. When the poll runs asynchronously (threaded NAPI)
and is slower than the writer, the queue grows until the system runs
out of memory. An unprivileged user can do this from its own user and
network namespaces, and the queued skbs are not charged to its memcg:
in a test VM, a uid 1000 process limited to 128 MB caused a global OOM
in ~6 seconds.

Drop packets written while netdev_max_backlog of them are queued,
before allocating them, and count them as rx_dropped. The sendmsg()
path, whose callers pass zerocopy buffers, is left alone.

Still open: the limit counts packets rather than bytes and applies per
queue, and the queued skbs are still not charged to the writer's memcg.

Found, fixed and tested with the help of an AI coding assistant
(Claude Code).

Fixes: 943170998b20 ("tun: enable NAPI for TUN/TAP driver")
Signed-off-by: Antonio Quartulli <antonio@mandelbit.com>
---

v2:
- Cc'ong netdev and maintainers

Notes:
- Affected since 943170998b20 (v4.15). Reproduced on mainline
  7be4bc52fd9e; the code is unchanged in net/main 94cea342e891, which
  this patch is based on and was build-tested (W=1) against. The
  before/after runtime tests were done on 7be4bc52fd9e.
- Conditions: unprivileged user namespaces allowed, /dev/net/tun
  accessible (0666 with the usual udev rule), threaded NAPI.
- Mitigation: disallow unprivileged user namespaces or restrict
  access to /dev/net/tun.
- A reproducer is available on request.

 drivers/net/tun.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 770b8fe085c3..1ace0c1f22f2 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -60,6 +60,7 @@
 #include <linux/rcupdate.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
+#include <net/hotdata.h>
 #include <net/rtnetlink.h>
 #include <net/sock.h>
 #include <net/xdp.h>
@@ -1856,6 +1857,17 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 			zerocopy = true;
 	}
 
+	/* In NAPI mode packets are queued here and processed by the NAPI
+	 * poll, possibly in a kthread or ksoftirqd. Bound that queue like
+	 * the per-CPU backlog, and drop before allocating and copying.
+	 */
+	if (tfile->napi_enabled && !frags && !msg_control &&
+	    unlikely(skb_queue_len_lockless(&tfile->sk.sk_write_queue) >=
+		     READ_ONCE(net_hotdata.max_backlog))) {
+		dev_core_stats_rx_dropped_inc(tun->dev);
+		return total_len;
+	}
+
 	if (!frags && tun_can_build_skb(tun, tfile, len, noblock, zerocopy)) {
 		/* For the packet that is not easy to be processed
 		 * (e.g gso or jumbo packet), we will do it at after
-- 
2.56.0

Re: [PATCH net v2] tun: bound the NAPI receive queue

[Willem de Bruijn]

Antonio Quartulli wrote:
> With IFF_NAPI, tun_get_user() queues every written packet for NAPI on
> an unbounded list. When the poll runs asynchronously (threaded NAPI)
> and is slower than the writer, the queue grows until the system runs
> out of memory. An unprivileged user can do this from its own user and
> network namespaces, and the queued skbs are not charged to its memcg:
> in a test VM, a uid 1000 process limited to 128 MB caused a global OOM
> in ~6 seconds.
> 
> Drop packets written while netdev_max_backlog of them are queued,
> before allocating them, and count them as rx_dropped. The sendmsg()
> path, whose callers pass zerocopy buffers, is left alone.
> 
> Still open: the limit counts packets rather than bytes and applies per
> queue, and the queued skbs are still not charged to the writer's memcg.
> 
> Found, fixed and tested with the help of an AI coding assistant
> (Claude Code).
> 
> Fixes: 943170998b20 ("tun: enable NAPI for TUN/TAP driver")
> Signed-off-by: Antonio Quartulli <antonio@mandelbit.com>
> ---
> 
> v2:
> - Cc'ong netdev and maintainers
> 
> Notes:
> - Affected since 943170998b20 (v4.15). Reproduced on mainline
>   7be4bc52fd9e; the code is unchanged in net/main 94cea342e891, which
>   this patch is based on and was build-tested (W=1) against. The
>   before/after runtime tests were done on 7be4bc52fd9e.
> - Conditions: unprivileged user namespaces allowed, /dev/net/tun
>   accessible (0666 with the usual udev rule), threaded NAPI.
> - Mitigation: disallow unprivileged user namespaces or restrict
>   access to /dev/net/tun.
> - A reproducer is available on request.
> 
>  drivers/net/tun.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 770b8fe085c3..1ace0c1f22f2 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -60,6 +60,7 @@
>  #include <linux/rcupdate.h>
>  #include <net/net_namespace.h>
>  #include <net/netns/generic.h>
> +#include <net/hotdata.h>
>  #include <net/rtnetlink.h>
>  #include <net/sock.h>
>  #include <net/xdp.h>

Keep alphebetical order


> @@ -1856,6 +1857,17 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  			zerocopy = true;
>  	}
>  
> +	/* In NAPI mode packets are queued here and processed by the NAPI
> +	 * poll, possibly in a kthread or ksoftirqd. Bound that queue like
> +	 * the per-CPU backlog, and drop before allocating and copying.
> +	 */
> +	if (tfile->napi_enabled && !frags && !msg_control &&

Why disable if frags? That is still using NAPI scheduling.

Same for msg_control. tun_chr_write_iter calls with NULL msg_control.

> +	    unlikely(skb_queue_len_lockless(&tfile->sk.sk_write_queue) >=
> +		     READ_ONCE(net_hotdata.max_backlog))) {

Nit: consider > instead of >=, same as enqueue_to_backlog

> +		dev_core_stats_rx_dropped_inc(tun->dev);
> +		return total_len;
> +	}

also best effort: more robust would require retesting once holding
the lock.

> +
>  	if (!frags && tun_can_build_skb(tun, tfile, len, noblock, zerocopy)) {
>  		/* For the packet that is not easy to be processed
>  		 * (e.g gso or jumbo packet), we will do it at after
> -- 
> 2.56.0
>