| Message ID | 1667381439-4419-1-git-send-email-wangyufen@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: tun: Fix memory leaks of napi_get_frags | expand |
Ignore, will resend later 在 2022/11/2 17:30, Wang Yufen 写道: > 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 > <-- not ask to complete the gro work, will cause memory leaks in > <-- following tun_napi_del() > ... > tun_napi_del() > netif_napi_del() > __netif_napi_del() > <-- &napi->rx_list is not empty, which caused memory leaks > > To fix, add napi_complete() after napi_gro_frags(). > > Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver") > Signed-off-by: Wang Yufen <wangyufen@huawei.com> > --- > drivers/net/tun.c | 15 +++++---------- > 1 file changed, 5 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index 27c6d23..910990d 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -1864,13 +1864,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, > > if (virtio_net_hdr_to_skb(skb, &gso, tun_is_little_endian(tun))) { > atomic_long_inc(&tun->rx_frame_errors); > - kfree_skb(skb); > - if (frags) { > - tfile->napi.skb = NULL; > - mutex_unlock(&tfile->napi_mutex); > - } > - > - return -EINVAL; > + err = -EINVAL; > + goto drop; > } > > switch (tun->flags & TUN_TYPE_MASK) { > @@ -1886,9 +1881,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, > pi.proto = htons(ETH_P_IPV6); > break; > default: > - dev_core_stats_rx_dropped_inc(tun->dev); > - kfree_skb(skb); > - return -EINVAL; > + err = -EINVAL; > + goto drop; > } > } > > @@ -1976,6 +1970,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, > > local_bh_disable(); > napi_gro_frags(&tfile->napi); > + napi_complete(&tfile->napi); > local_bh_enable(); > mutex_unlock(&tfile->napi_mutex); > } else if (tfile->napi_enabled) {
diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 27c6d23..910990d 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1864,13 +1864,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, if (virtio_net_hdr_to_skb(skb, &gso, tun_is_little_endian(tun))) { atomic_long_inc(&tun->rx_frame_errors); - kfree_skb(skb); - if (frags) { - tfile->napi.skb = NULL; - mutex_unlock(&tfile->napi_mutex); - } - - return -EINVAL; + err = -EINVAL; + goto drop; } switch (tun->flags & TUN_TYPE_MASK) { @@ -1886,9 +1881,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, pi.proto = htons(ETH_P_IPV6); break; default: - dev_core_stats_rx_dropped_inc(tun->dev); - kfree_skb(skb); - return -EINVAL; + err = -EINVAL; + goto drop; } } @@ -1976,6 +1970,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, local_bh_disable(); napi_gro_frags(&tfile->napi); + napi_complete(&tfile->napi); local_bh_enable(); mutex_unlock(&tfile->napi_mutex); } else if (tfile->napi_enabled) {
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 <-- not ask to complete the gro work, will cause memory leaks in <-- following tun_napi_del() ... tun_napi_del() netif_napi_del() __netif_napi_del() <-- &napi->rx_list is not empty, which caused memory leaks To fix, add napi_complete() after napi_gro_frags(). Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver") Signed-off-by: Wang Yufen <wangyufen@huawei.com> --- drivers/net/tun.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)