<div dir="ltr">Looks correct.<div>Could you, please, also change the order of `list_del` and `kni_dev_remove` in `kni_release`? It suffers from the same problem.</div><div><br></div><div>Igor</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 28, 2022 at 5:43 AM Min Hu (Connor) <<a href="mailto:humin29@huawei.com">humin29@huawei.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">From: Huisong Li <<a href="mailto:lihuisong@huawei.com" target="_blank">lihuisong@huawei.com</a>><br>
<br>
The "kni_dev" is the private data of the "net_device" in kni, and allocated<br>
with the "net_device" by calling "alloc_netdev()". The "net_device" is<br>
freed by calling "free_netdev()" when kni release. The freed memory<br>
includes the "kni_dev". So After "kni_dev" should not be accessed after<br>
"net_device" is released.<br>
<br>
Fixes: e77fec694936 ("kni: fix possible mbuf leaks and speed up port release")<br>
Cc: <a href="mailto:stable@dpdk.org" target="_blank">stable@dpdk.org</a><br>
<br>
KASAN trace:<br>
<br>
[   85.263717] ==========================================================<br>
[   85.264418] BUG: KASAN: use-after-free in kni_net_release_fifo_phy+<br>
                0x30/0x84 [rte_kni]<br>
[   85.265139] Read of size 8 at addr ffff000260668d60 by task kni/341<br>
[   85.265703]<br>
[   85.265857] CPU: 0 PID: 341 Comm: kni Tainted: G     U     O      <br>
                5.15.0-rc4+ #1<br>
[   85.266525] Hardware name: linux,dummy-virt (DT)<br>
[   85.266968] Call trace:<br>
[   85.267220]  dump_backtrace+0x0/0x2d0<br>
[   85.267591]  show_stack+0x24/0x30<br>
[   85.267924]  dump_stack_lvl+0x8c/0xb8<br>
[   85.268294]  print_address_description.constprop.0+0x74/0x2b8<br>
[   85.268855]  kasan_report+0x1e4/0x200<br>
[   85.269224]  __asan_load8+0x98/0xd4<br>
[   85.269577]  kni_net_release_fifo_phy+0x30/0x84 [rte_kni]<br>
[   85.270116]  kni_dev_remove.isra.0+0x50/0x64 [rte_kni]<br>
[   85.270630]  kni_ioctl_release+0x254/0x320 [rte_kni]<br>
[   85.271136]  kni_ioctl+0x64/0xb0 [rte_kni]<br>
[   85.271553]  __arm64_sys_ioctl+0xdc/0x120<br>
[   85.271955]  invoke_syscall+0x68/0x1a0<br>
[   85.272332]  el0_svc_common.constprop.0+0x90/0x200<br>
[   85.272807]  do_el0_svc+0x94/0xa4<br>
[   85.273144]  el0_svc+0x78/0x240<br>
[   85.273463]  el0t_64_sync_handler+0x1a8/0x1b0<br>
[   85.273895]  el0t_64_sync+0x1a0/0x1a4<br>
[   85.274264]<br>
[   85.274427] Allocated by task 341:<br>
[   85.274767]  kasan_save_stack+0x2c/0x60<br>
[   85.275157]  __kasan_kmalloc+0x90/0xb4<br>
[   85.275533]  __kmalloc_node+0x230/0x594<br>
[   85.275917]  kvmalloc_node+0x8c/0x190<br>
[   85.276286]  alloc_netdev_mqs+0x70/0x6b0<br>
[   85.276678]  kni_ioctl_create+0x224/0xf40 [rte_kni]<br>
[   85.277166]  kni_ioctl+0x9c/0xb0 [rte_kni]<br>
[   85.277581]  __arm64_sys_ioctl+0xdc/0x120<br>
[   85.277980]  invoke_syscall+0x68/0x1a0<br>
[   85.278357]  el0_svc_common.constprop.0+0x90/0x200<br>
[   85.278830]  do_el0_svc+0x94/0xa4<br>
[   85.279172]  el0_svc+0x78/0x240<br>
[   85.279491]  el0t_64_sync_handler+0x1a8/0x1b0<br>
[   85.279925]  el0t_64_sync+0x1a0/0x1a4<br>
[   85.280292]<br>
[   85.280454] Freed by task 341:<br>
[   85.280763]  kasan_save_stack+0x2c/0x60<br>
[   85.281147]  kasan_set_track+0x2c/0x40<br>
[   85.281522]  kasan_set_free_info+0x2c/0x50<br>
[   85.281930]  __kasan_slab_free+0xdc/0x140<br>
[   85.282331]  slab_free_freelist_hook+0x90/0x250<br>
[   85.282782]  kfree+0x128/0x580<br>
[   85.283099]  kvfree+0x48/0x60<br>
[   85.283402]  netdev_freemem+0x34/0x44<br>
[   85.283770]  netdev_release+0x50/0x64<br>
[   85.284138]  device_release+0xa0/0x120<br>
[   85.284516]  kobject_put+0xf8/0x160<br>
[   85.284867]  put_device+0x20/0x30<br>
[   85.285204]  free_netdev+0x22c/0x310<br>
[   85.285562]  kni_dev_remove.isra.0+0x48/0x64 [rte_kni]<br>
[   85.286076]  kni_ioctl_release+0x254/0x320 [rte_kni]<br>
[   85.286573]  kni_ioctl+0x64/0xb0 [rte_kni]<br>
[   85.286992]  __arm64_sys_ioctl+0xdc/0x120<br>
[   85.287392]  invoke_syscall+0x68/0x1a0<br>
[   85.287769]  el0_svc_common.constprop.0+0x90/0x200<br>
[   85.288243]  do_el0_svc+0x94/0xa4<br>
[   85.288579]  el0_svc+0x78/0x240<br>
[   85.288899]  el0t_64_sync_handler+0x1a8/0x1b0<br>
[   85.289332]  el0t_64_sync+0x1a0/0x1a4<br>
[   85.289699]<br>
[   85.289862] The buggy address belongs to the object at ffff000260668000<br>
[   85.289862]  which belongs to the cache kmalloc-cg-8k of size 8192<br>
[   85.291079] The buggy address is located 3424 bytes inside of<br>
[   85.291079]  8192-byte region [ffff000260668000, ffff00026066a000)<br>
[   85.292213] The buggy address belongs to the page:<br>
[   85.292684] page:(____ptrval____) refcount:1 mapcount:0 mapping:<br>
                0000000000000000 index:0x0 pfn:0x2a0668<br>
[   85.293585] head:(____ptrval____) order:3 compound_mapcount:0<br>
                compound_pincount:0<br>
[   85.294305] flags: 0xbfff80000010200(slab|head|node=0|zone=2|<br>
                lastcpupid=0x7fff)<br>
[   85.295020] raw: 0bfff80000010200 0000000000000000 dead000000000122<br>
                ffff0000c000d680<br>
[   85.295767] raw: 0000000000000000 0000000080020002 00000001ffffffff<br>
                0000000000000000<br>
[   85.296512] page dumped because: kasan: bad access detected<br>
[   85.297054]<br>
[   85.297217] Memory state around the buggy address:<br>
[   85.297688]  ffff000260668c00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb<br>
                fb fb<br>
[   85.298384]  ffff000260668c80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb<br>
                fb fb<br>
[   85.299088] >ffff000260668d00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb<br>
                fb fb<br>
[   85.299781]                                                        ^<br>
[   85.300396]  ffff000260668d80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb<br>
                fb fb<br>
[   85.301092]  ffff000260668e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb<br>
                fb fb<br>
[   85.301787] ===========================================================<br>
<br>
Signed-off-by: Huisong Li <<a href="mailto:lihuisong@huawei.com" target="_blank">lihuisong@huawei.com</a>><br>
Signed-off-by: Min Hu (Connor) <<a href="mailto:humin29@huawei.com" target="_blank">humin29@huawei.com</a>><br>
---<br>
 kernel/linux/kni/kni_misc.c | 10 +++++++---<br>
 1 file changed, 7 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c<br>
index f10dcd069d..b3684c4fa6 100644<br>
--- a/kernel/linux/kni/kni_misc.c<br>
+++ b/kernel/linux/kni/kni_misc.c<br>
@@ -184,13 +184,17 @@ kni_dev_remove(struct kni_dev *dev)<br>
        if (!dev)<br>
                return -ENODEV;<br>
<br>
+       /*<br>
+        * The memory of kni device is allocated and released together<br>
+        * with net device. Release mbuf before freeing net device.<br>
+        */<br>
+       kni_net_release_fifo_phy(dev);<br>
+<br>
        if (dev->net_dev) {<br>
                unregister_netdev(dev->net_dev);<br>
                free_netdev(dev->net_dev);<br>
        }<br>
<br>
-       kni_net_release_fifo_phy(dev);<br>
-<br>
        return 0;<br>
 }<br>
<br>
@@ -470,8 +474,8 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num,<br>
                        dev->pthread = NULL;<br>
                }<br>
<br>
-               kni_dev_remove(dev);<br>
                list_del(&dev->list);<br>
+               kni_dev_remove(dev);<br>
                ret = 0;<br>
                break;<br>
        }<br>
-- <br>
2.33.0<br>
<br>
</blockquote></div>