netdev - Re: [PATCH net] net: Fix memory leaks of napi->rx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89i+cNjUH8pR0-QTdWM09G8ZfX_gzDqOY6ecyY4igDwrYaQ@mail.gmail.com>
Date:   Tue, 1 Nov 2022 21:27:42 -0700
From:   Eric Dumazet <edumazet@...gle.com>
To:     Wang Yufen <wangyufen@...wei.com>
Cc:     netdev@...r.kernel.org, bpf@...r.kernel.org, davem@...emloft.net,
        kuba@...nel.org, pabeni@...hat.com, ecree@...arflare.com
Subject: Re: [PATCH net] net: Fix memory leaks of napi->rx_list

On Tue, Nov 1, 2022 at 8:34 PM Wang Yufen <wangyufen@...wei.com> 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@...wei.com>
> ---
>  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
>