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

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

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
>