On Tue, Nov 1, 2022 at 8:34 PM Wang Yufen <wangyufen@xxxxxxxxxx> wrote:
>
> kmemleak reports after running test_progs:
>
> unreferenced object 0xffff8881b1672dc0 (size 232):
> comm "test_progs", pid 394388, jiffies 4354712116 (age 841.975s)
> hex dump (first 32 bytes):
> e0 84 d7 a8 81 88 ff ff 80 2c 67 b1 81 88 ff ff .........,g.....
> 00 40 c5 9b 81 88 ff ff 00 00 00 00 00 00 00 00 .@..............
> backtrace:
> [<00000000c8f01748>] napi_skb_cache_get+0xd4/0x150
> [<0000000041c7fc09>] __napi_build_skb+0x15/0x50
> [<00000000431c7079>] __napi_alloc_skb+0x26e/0x540
> [<000000003ecfa30e>] napi_get_frags+0x59/0x140
> [<0000000099b2199e>] tun_get_user+0x183d/0x3bb0 [tun]
> [<000000008a5adef0>] tun_chr_write_iter+0xc0/0x1b1 [tun]
> [<0000000049993ff4>] do_iter_readv_writev+0x19f/0x320
> [<000000008f338ea2>] do_iter_write+0x135/0x630
> [<000000008a3377a4>] vfs_writev+0x12e/0x440
> [<00000000a6b5639a>] do_writev+0x104/0x280
> [<00000000ccf065d8>] do_syscall_64+0x3b/0x90
> [<00000000d776e329>] entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> The issue occurs in the following scenarios:
> tun_get_user()
> napi_gro_frags()
> napi_frags_finish()
> case GRO_NORMAL:
> gro_normal_one()
> list_add_tail(&skb->list, &napi->rx_list);
> <-- While napi->rx_count < READ_ONCE(gro_normal_batch),
> <-- gro_normal_list() is not called, napi->rx_list is not empty
> ...
> netif_napi_del()
> __netif_napi_del()
> <-- &napi->rx_list is not empty, which caused memory leaks
>
> To fix, add flush_rx_list() to free skbs in napi->rx_list.
>
> Fixes: 323ebb61e32b ("net: use listified RX for handling GRO_NORMAL skbs")
I do not think the bug is there.
Most likely tun driver is buggy.
It does not follow the correct napi protocol.
It feeds packets to GRO, but does not ever ask to complete the work.
More sanity work is needed in tun, not in GRO layer.
> Signed-off-by: Wang Yufen <wangyufen@xxxxxxxxxx>
> ---
> net/core/dev.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 3be2560..de3bc9c 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6461,6 +6461,16 @@ static void flush_gro_hash(struct napi_struct *napi)
> }
> }
>
> +static void flush_rx_list(struct napi_struct *napi)
> +{
> + struct sk_buff *skb, *next;
> +
> + list_for_each_entry_safe(skb, next, &napi->rx_list, list) {
> + skb_list_del_init(skb);
> + kfree_skb(skb);
> + }
> +}
> +
> /* Must be called in process context */
> void __netif_napi_del(struct napi_struct *napi)
> {
> @@ -6471,6 +6481,7 @@ void __netif_napi_del(struct napi_struct *napi)
> list_del_rcu(&napi->dev_list);
> napi_free_frags(napi);
>
> + flush_rx_list(napi);
> flush_gro_hash(napi);
> napi->gro_bitmask = 0;
>
> --
> 1.8.3.1
>
