Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ebc26df487674f25803d59a39ceb7018ab5df4fc.camel@redhat.com>
Subject: Re: [PATCH] Fix a use-after-free bug
From:   Paolo Abeni <pabeni@redhat.com>
To:     Wentao_Liang <Wentao_Liang_g@163.com>, jdmason@kudzu.us,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Date:   Tue, 14 Jun 2022 13:21:02 +0200
In-Reply-To: <20220614012853.10560-1-Wentao_Liang_g@163.com>
References: <20220614012853.10560-1-Wentao_Liang_g@163.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.4 (3.42.4-2.fc35) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Tue, 2022-06-14 at 09:28 +0800, Wentao_Liang wrote:
> The pointer vdev points to a memory region adjacent to a net_device
> structure ndev, which is a field of hldev. At line 4740, the invocation
> to vxge_device_unregister unregisters device hldev, and it also releases
> the memory region pointed by vdev->bar0. At line 4743, the freed memory
> region is referenced (i.e., iounmap(vdev->bar0)), resulting in a
> use-after-free vulnerability. We can fix the bug by calling iounmap
> before vxge_device_unregister.
> 
> 4721.      static void vxge_remove(struct pci_dev *pdev)
> 4722.      {
> 4723.             struct __vxge_hw_device *hldev;
> 4724.             struct vxgedev *vdev;
> …
> 4731.             vdev = netdev_priv(hldev->ndev);
> …
> 4740.             vxge_device_unregister(hldev);
> 4741.             /* Do not call pci_disable_sriov here, as it
> 						will break child devices */
> 4742.             vxge_hw_device_terminate(hldev);
> 4743.             iounmap(vdev->bar0);
> …
> 4749              vxge_debug_init(vdev->level_trace, "%s:%d
> 								Device unregistered",
> 4750                            __func__, __LINE__);
> 4751              vxge_debug_entryexit(vdev->level_trace, "%s:%d
> 								Exiting...", __func__,
> 4752                          __LINE__);
> 4753.      }
> 
> This is the screenshot when the vulnerability is triggered by using
> KASAN. We can see that there is a use-after-free reported by KASAN.
> 
> /***********************report begin***************************/
> 
> root@kernel:~# echo 1 > /sys/bus/pci/devices/0000:00:03.0/remove
> [  178.296316] vxge_remove
> [  182.057081]
>  ==================================================================
> [  182.057548] BUG: KASAN: use-after-free in vxge_remove+0xe0/0x15c
> [  182.057760] Read of size 8 at addr ffff888006c76598 by task bash/119
> [  182.057983]
> [  182.058747] CPU: 0 PID: 119 Comm: bash Not tainted 5.18.0 #5
> [  182.058919] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
> BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
> [  182.059463] Call Trace:
> [  182.059726]  <TASK>
> [  182.060017]  dump_stack_lvl+0x34/0x44
> [  182.060316]  print_report.cold+0xb2/0x6b7
> [  182.060401]  ? kfree+0x89/0x290
> [  182.060478]  ? vxge_remove+0xe0/0x15c
> [  182.060545]  kasan_report+0xa9/0x120
> ...
> [  182.070606]
>  ==================================================================
> [  182.071374] Disabling lock debugging due to kernel taint
> 
> /************************report end***************************/

It's better to include a complete backtrace

> 
> After fixing the bug as done in the patch, we can find KASAN do not report
>  the bug and the device(00:03.0) has been successfully removed.
> 
> /************************report begin*************************/
> 
> root@kernel:~# echo 1 > /sys/bus/pci/devices/0000:00:03.0/remove
> root@kernel:~#
> 
> /************************report end***************************/
> 
> Signed-off-by: Wentao_Liang <Wentao_Liang_g@163.com>

Please include a 'Fixes' tag pointing to the commit introducing the
bug, and please specify the relevant target tree and driver in the
patch subj. It should be something alike:

[PATCH net v2] vxge: fix a use-after-free bug

Thanks,

Paolo

> ---
>  drivers/net/ethernet/neterion/vxge/vxge-main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/neterion/vxge/vxge-main.c b/drivers/net/ethernet/neterion/vxge/vxge-main.c
> index fa5d4ddf429b..092fd0ae5831 100644
> --- a/drivers/net/ethernet/neterion/vxge/vxge-main.c
> +++ b/drivers/net/ethernet/neterion/vxge/vxge-main.c
> @@ -4736,10 +4736,10 @@ static void vxge_remove(struct pci_dev *pdev)
>  	for (i = 0; i < vdev->no_of_vpath; i++)
>  		vxge_free_mac_add_list(&vdev->vpaths[i]);
>  
> +	iounmap(vdev->bar0);
>  	vxge_device_unregister(hldev);
>  	/* Do not call pci_disable_sriov here, as it will break child devices */
>  	vxge_hw_device_terminate(hldev);
> -	iounmap(vdev->bar0);
>  	pci_release_region(pdev, 0);
>  	pci_disable_device(pdev);
>  	driver_config->config_dev_cnt--;