diff mbox series

[net] net: Fix memory leaks in napi_get_frags_check()

Message ID 1667213123-18922-1-git-send-email-wangyufen@huawei.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net] net: Fix memory leaks in napi_get_frags_check() | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net, async
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 4336 this patch: 4336
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 1052 this patch: 1052
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 4523 this patch: 4523
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 29 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

wangyufen Oct. 31, 2022, 10:45 a.m. UTC
kmemleak reports this issue:

unreferenced object 0xffff88819b8c97c0 (size 232):
  comm "ip", pid 2915, jiffies 4295153830 (age 408.877s)
  hex dump (first 32 bytes):
    00 80 8c 9b 81 88 ff ff e0 24 f7 12 81 88 ff ff  .........$......
    00 40 05 c0 81 88 ff ff 00 00 00 00 00 00 00 00  .@..............
  backtrace:
    [<0000000009fc179a>] napi_skb_cache_get+0xd4/0x150
    [<00000000b4383b46>] __napi_build_skb+0x15/0x50
    [<000000004902d9a0>] __napi_alloc_skb+0x26e/0x540
    [<00000000f9542b96>] napi_get_frags+0x59/0x140
    [<0000000002214936>] napi_get_frags_check+0x23/0xa0
    [<00000000c9a8df5a>] netif_napi_add_weight+0x443/0x910
    [<000000008f3a2156>] gro_cells_init+0x13f/0x230
    [<000000006f5f87fd>] vxlan_init+0x125/0x210 [vxlan]
    [<00000000f9d7479b>] register_netdevice+0x271/0xeb0
    [<000000003eb98c8e>] __vxlan_dev_create+0x2f2/0x780 [vxlan]
    [<00000000c5cdebe0>] vxlan_newlink+0xa0/0xf0 [vxlan]
    [<000000004a0b0607>] __rtnl_newlink+0xcae/0x1490
    [<0000000037188a06>] rtnl_newlink+0x5f/0x90
    [<00000000b2d4e2b2>] rtnetlink_rcv_msg+0x31a/0x980
    [<00000000613a7411>] netlink_rcv_skb+0x120/0x380
    [<00000000836c6e0e>] netlink_unicast+0x439/0x660

The root case here is: 
  napi_get_frags_check()
    napi_get_frags()
      napi_alloc_skb()
       __alloc_skb()  <-- with flag SKB_ALLOC_NAPI
        napi_skb_cache_get()
    ...
    napi_free_frags()
      kfree_skb()
        __kfree_skb()

In function napi_get_frags(), the skb is alloced by napi_skb_cache_get().
So, the skb should use __kfree_skb_defer (which called
napi_skb_cache_put()) to free.
To fix, add the function napi_free_frags_defer() to use __kfree_skb_defer
to free skb.

Fixes: cfb8ec659521 ("skbuff: allow to use NAPI cache from __napi_alloc_skb()")
Signed-off-by: Wang Yufen <wangyufen@huawei.com>
---
 include/linux/netdevice.h | 8 ++++++++
 net/core/skbuff.c         | 3 ++-
 2 files changed, 10 insertions(+), 1 deletion(-)

Comments

wangyufen Nov. 1, 2022, 4:40 a.m. UTC | #1
Ignore, this fix is wrong.

在 2022/10/31 18:45, Wang Yufen 写道:
> kmemleak reports this issue:
>
> unreferenced object 0xffff88819b8c97c0 (size 232):
>    comm "ip", pid 2915, jiffies 4295153830 (age 408.877s)
>    hex dump (first 32 bytes):
>      00 80 8c 9b 81 88 ff ff e0 24 f7 12 81 88 ff ff  .........$......
>      00 40 05 c0 81 88 ff ff 00 00 00 00 00 00 00 00  .@..............
>    backtrace:
>      [<0000000009fc179a>] napi_skb_cache_get+0xd4/0x150
>      [<00000000b4383b46>] __napi_build_skb+0x15/0x50
>      [<000000004902d9a0>] __napi_alloc_skb+0x26e/0x540
>      [<00000000f9542b96>] napi_get_frags+0x59/0x140
>      [<0000000002214936>] napi_get_frags_check+0x23/0xa0
>      [<00000000c9a8df5a>] netif_napi_add_weight+0x443/0x910
>      [<000000008f3a2156>] gro_cells_init+0x13f/0x230
>      [<000000006f5f87fd>] vxlan_init+0x125/0x210 [vxlan]
>      [<00000000f9d7479b>] register_netdevice+0x271/0xeb0
>      [<000000003eb98c8e>] __vxlan_dev_create+0x2f2/0x780 [vxlan]
>      [<00000000c5cdebe0>] vxlan_newlink+0xa0/0xf0 [vxlan]
>      [<000000004a0b0607>] __rtnl_newlink+0xcae/0x1490
>      [<0000000037188a06>] rtnl_newlink+0x5f/0x90
>      [<00000000b2d4e2b2>] rtnetlink_rcv_msg+0x31a/0x980
>      [<00000000613a7411>] netlink_rcv_skb+0x120/0x380
>      [<00000000836c6e0e>] netlink_unicast+0x439/0x660
>
> The root case here is:
>    napi_get_frags_check()
>      napi_get_frags()
>        napi_alloc_skb()
>         __alloc_skb()  <-- with flag SKB_ALLOC_NAPI
>          napi_skb_cache_get()
>      ...
>      napi_free_frags()
>        kfree_skb()
>          __kfree_skb()
>
> In function napi_get_frags(), the skb is alloced by napi_skb_cache_get().
> So, the skb should use __kfree_skb_defer (which called
> napi_skb_cache_put()) to free.
> To fix, add the function napi_free_frags_defer() to use __kfree_skb_defer
> to free skb.
>
> Fixes: cfb8ec659521 ("skbuff: allow to use NAPI cache from __napi_alloc_skb()")
> Signed-off-by: Wang Yufen <wangyufen@huawei.com>
> ---
>   include/linux/netdevice.h | 8 ++++++++
>   net/core/skbuff.c         | 3 ++-
>   2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index eddf8ee..0811663 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3833,6 +3833,14 @@ static inline void napi_free_frags(struct napi_struct *napi)
>   	napi->skb = NULL;
>   }
>   
> +static inline void napi_free_frags_defer(struct napi_struct *napi)
> +{
> +	if (!skb_unref(napi->skb))
> +		return;
> +	__kfree_skb_defer(napi->skb);
> +	napi->skb = NULL;
> +}
> +
>   bool netdev_is_rx_handler_busy(struct net_device *dev);
>   int netdev_rx_handler_register(struct net_device *dev,
>   			       rx_handler_func_t *rx_handler,
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index d1a3fa6..1db3634 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -214,7 +214,7 @@ void napi_get_frags_check(struct napi_struct *napi)
>   	local_bh_disable();
>   	skb = napi_get_frags(napi);
>   	WARN_ON_ONCE(!NAPI_HAS_SMALL_PAGE_FRAG && skb && skb->head_frag);
> -	napi_free_frags(napi);
> +	napi_free_frags_defer(napi);
>   	local_bh_enable();
>   }
>   
> @@ -1073,6 +1073,7 @@ void __kfree_skb_defer(struct sk_buff *skb)
>   	skb_release_all(skb);
>   	napi_skb_cache_put(skb);
>   }
> +EXPORT_SYMBOL(__kfree_skb_defer);
>   
>   void napi_skb_free_stolen_head(struct sk_buff *skb)
>   {
diff mbox series

Patch

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index eddf8ee..0811663 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3833,6 +3833,14 @@  static inline void napi_free_frags(struct napi_struct *napi)
 	napi->skb = NULL;
 }
 
+static inline void napi_free_frags_defer(struct napi_struct *napi)
+{
+	if (!skb_unref(napi->skb))
+		return;
+	__kfree_skb_defer(napi->skb);
+	napi->skb = NULL;
+}
+
 bool netdev_is_rx_handler_busy(struct net_device *dev);
 int netdev_rx_handler_register(struct net_device *dev,
 			       rx_handler_func_t *rx_handler,
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d1a3fa6..1db3634 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -214,7 +214,7 @@  void napi_get_frags_check(struct napi_struct *napi)
 	local_bh_disable();
 	skb = napi_get_frags(napi);
 	WARN_ON_ONCE(!NAPI_HAS_SMALL_PAGE_FRAG && skb && skb->head_frag);
-	napi_free_frags(napi);
+	napi_free_frags_defer(napi);
 	local_bh_enable();
 }
 
@@ -1073,6 +1073,7 @@  void __kfree_skb_defer(struct sk_buff *skb)
 	skb_release_all(skb);
 	napi_skb_cache_put(skb);
 }
+EXPORT_SYMBOL(__kfree_skb_defer);
 
 void napi_skb_free_stolen_head(struct sk_buff *skb)
 {