[PATCH v2] net: igbvf: fix double free in `igbvf_probe`

[PATCH v2] net: igbvf: fix double free in `igbvf_probe`

[Letu Ren]

In `igbvf_probe`, if register_netdev() fails, the program will go to
label err_hw_init, and then to label err_ioremap. In free_netdev() which
is just below label err_ioremap, there is `list_for_each_entry_safe` and
`netif_napi_del` which aims to delete all entries in `dev->napi_list`.
The program has added an entry `adapter->rx_ring->napi` which is added by
`netif_napi_add` in igbvf_alloc_queues(). However, adapter->rx_ring has
been freed below label err_hw_init. So this a UAF.

In terms of how to patch the problem, we can refer to igbvf_remove() and
delete the entry before `adapter->rx_ring`.

The KASAN logs are as follows:

[   35.126075] BUG: KASAN: use-after-free in free_netdev+0x1fd/0x450
[   35.127170] Read of size 8 at addr ffff88810126d990 by task modprobe/366
[   35.128360]
[   35.128643] CPU: 1 PID: 366 Comm: modprobe Not tainted 5.15.0-rc2+ #14
[   35.129789] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
[   35.131749] Call Trace:
[   35.132199]  dump_stack_lvl+0x59/0x7b
[   35.132865]  print_address_description+0x7c/0x3b0
[   35.133707]  ? free_netdev+0x1fd/0x450
[   35.134378]  __kasan_report+0x160/0x1c0
[   35.135063]  ? free_netdev+0x1fd/0x450
[   35.135738]  kasan_report+0x4b/0x70
[   35.136367]  free_netdev+0x1fd/0x450
[   35.137006]  igbvf_probe+0x121d/0x1a10 [igbvf]
[   35.137808]  ? igbvf_vlan_rx_add_vid+0x100/0x100 [igbvf]
[   35.138751]  local_pci_probe+0x13c/0x1f0
[   35.139461]  pci_device_probe+0x37e/0x6c0
[   35.165526]
[   35.165806] Allocated by task 366:
[   35.166414]  ____kasan_kmalloc+0xc4/0xf0
[   35.167117]  foo_kmem_cache_alloc_trace+0x3c/0x50 [igbvf]
[   35.168078]  igbvf_probe+0x9c5/0x1a10 [igbvf]
[   35.168866]  local_pci_probe+0x13c/0x1f0
[   35.169565]  pci_device_probe+0x37e/0x6c0
[   35.179713]
[   35.179993] Freed by task 366:
[   35.180539]  kasan_set_track+0x4c/0x80
[   35.181211]  kasan_set_free_info+0x1f/0x40
[   35.181942]  ____kasan_slab_free+0x103/0x140
[   35.182703]  kfree+0xe3/0x250
[   35.183239]  igbvf_probe+0x1173/0x1a10 [igbvf]
[   35.184040]  local_pci_probe+0x13c/0x1f0

Fixes: d4e0fe01a38a0 (igbvf: add new driver to support 82576 virtual functions)
Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Signed-off-by: Letu Ren <fantasquex@gmail.com>
---
Changes in v2:
    - Add fixes tag
---
 drivers/net/ethernet/intel/igbvf/netdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index d32e72d953c8..d051918dfdff 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -2861,6 +2861,7 @@ static int igbvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	return 0;
 
 err_hw_init:
+	netif_napi_del(&adapter->rx_ring->napi);
 	kfree(adapter->tx_ring);
 	kfree(adapter->rx_ring);
 err_sw_init:
-- 
2.33.1

RE: [Intel-wired-lan] [PATCH v2] net: igbvf: fix double free in `igbvf_probe`

[Jankowski, Konrad0]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Letu Ren
> Sent: sobota, 13 listopada 2021 04:43
> To: Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; davem@davemloft.net; kuba@kernel.org
> Cc: netdev@vger.kernel.org; Letu Ren <fantasquex@gmail.com>; intel-
> wired-lan@lists.osuosl.org; linux-kernel@vger.kernel.org; Zheyu Ma
> <zheyuma97@gmail.com>
> Subject: [Intel-wired-lan] [PATCH v2] net: igbvf: fix double free in
> `igbvf_probe`
> 
> In `igbvf_probe`, if register_netdev() fails, the program will go to label
> err_hw_init, and then to label err_ioremap. In free_netdev() which is just
> below label err_ioremap, there is `list_for_each_entry_safe` and
> `netif_napi_del` which aims to delete all entries in `dev->napi_list`.
> The program has added an entry `adapter->rx_ring->napi` which is added by
> `netif_napi_add` in igbvf_alloc_queues(). However, adapter->rx_ring has
> been freed below label err_hw_init. So this a UAF.
> 
> In terms of how to patch the problem, we can refer to igbvf_remove() and
> delete the entry before `adapter->rx_ring`.
> 
> The KASAN logs are as follows:
> 
> [   35.126075] BUG: KASAN: use-after-free in free_netdev+0x1fd/0x450
> [   35.127170] Read of size 8 at addr ffff88810126d990 by task modprobe/366
> [   35.128360]
> [   35.128643] CPU: 1 PID: 366 Comm: modprobe Not tainted 5.15.0-rc2+ #14
> [   35.129789] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
> rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
> [   35.131749] Call Trace:
> [   35.132199]  dump_stack_lvl+0x59/0x7b
> [   35.132865]  print_address_description+0x7c/0x3b0
> [   35.133707]  ? free_netdev+0x1fd/0x450
> [   35.134378]  __kasan_report+0x160/0x1c0
> [   35.135063]  ? free_netdev+0x1fd/0x450
> [   35.135738]  kasan_report+0x4b/0x70
> [   35.136367]  free_netdev+0x1fd/0x450
> [   35.137006]  igbvf_probe+0x121d/0x1a10 [igbvf]
> [   35.137808]  ? igbvf_vlan_rx_add_vid+0x100/0x100 [igbvf]
> [   35.138751]  local_pci_probe+0x13c/0x1f0
> [   35.139461]  pci_device_probe+0x37e/0x6c0
> [   35.165526]
> [   35.165806] Allocated by task 366:
> [   35.166414]  ____kasan_kmalloc+0xc4/0xf0
> [   35.167117]  foo_kmem_cache_alloc_trace+0x3c/0x50 [igbvf]
> [   35.168078]  igbvf_probe+0x9c5/0x1a10 [igbvf]
> [   35.168866]  local_pci_probe+0x13c/0x1f0
> [   35.169565]  pci_device_probe+0x37e/0x6c0
> [   35.179713]
> [   35.179993] Freed by task 366:
> [   35.180539]  kasan_set_track+0x4c/0x80
> [   35.181211]  kasan_set_free_info+0x1f/0x40
> [   35.181942]  ____kasan_slab_free+0x103/0x140
> [   35.182703]  kfree+0xe3/0x250
> [   35.183239]  igbvf_probe+0x1173/0x1a10 [igbvf]
> [   35.184040]  local_pci_probe+0x13c/0x1f0
> 
> Fixes: d4e0fe01a38a0 (igbvf: add new driver to support 82576 virtual
> functions)
> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> Signed-off-by: Letu Ren <fantasquex@gmail.com>
> ---
> Changes in v2:
>     - Add fixes tag
> ---
>  drivers/net/ethernet/intel/igbvf/netdev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c
> b/drivers/net/ethernet/intel/igbvf/netdev.c
> index d32e72d953c8..d051918dfdff 100644
> --- a/drivers/net/ethernet/intel/igbvf/netdev.c
> +++ b/drivers/net/ethernet/intel/igbvf/netdev.c
> @@ -2861,6 +2861,7 @@ static int igbvf_probe(struct pci_dev *pdev, const

Tested-by: Konrad Jankowski <konrad0.jankowski@intel.com>