lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 12 Jul 2022 08:56:37 +0100 From: Martin Habets <habetsm.xilinx@...il.com> To: Íñigo Huguet <ihuguet@...hat.com> Cc: ecree.xilinx@...il.com, sshah@...arflare.com, davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org, Yanghang Liu <yanghliu@...hat.com> Subject: Re: [PATCH v2 net] sfc: fix use after free when disabling sriov On Tue, Jul 12, 2022 at 08:26:42AM +0200, Íñigo Huguet wrote: > Use after free is detected by kfence when disabling sriov. What was read > after being freed was vf->pci_dev: it was freed from pci_disable_sriov > and later read in efx_ef10_sriov_free_vf_vports, called from > efx_ef10_sriov_free_vf_vswitching. > > Set the pointer to NULL at release time to not trying to read it later. This solution just bypasses the check we have in efx_ef10_sriov_free_vf_vports(): /* If VF is assigned, do not free the vport */ if (vf->pci_dev && pci_is_dev_assigned(vf->pci_dev)) continue; If we don't want to detect this any more we should remove this check in stead of this patch. There is another issue here, in efx_ef10_sriov_free_vf_vswitching() we do free the memory even if a VF was still assigned. This leads me to think that removing the check above is the better thing to do. Martin > Reproducer and dmesg log (note that kfence doesn't detect it every time): > $ echo 1 > /sys/class/net/enp65s0f0np0/device/sriov_numvfs > $ echo 0 > /sys/class/net/enp65s0f0np0/device/sriov_numvfs > > BUG: KFENCE: use-after-free read in efx_ef10_sriov_free_vf_vswitching+0x82/0x170 [sfc] > > Use-after-free read at 0x00000000ff3c1ba5 (in kfence-#224): > efx_ef10_sriov_free_vf_vswitching+0x82/0x170 [sfc] > efx_ef10_pci_sriov_disable+0x38/0x70 [sfc] > efx_pci_sriov_configure+0x24/0x40 [sfc] > sriov_numvfs_store+0xfe/0x140 > kernfs_fop_write_iter+0x11c/0x1b0 > new_sync_write+0x11f/0x1b0 > vfs_write+0x1eb/0x280 > ksys_write+0x5f/0xe0 > do_syscall_64+0x5c/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xae > > kfence-#224: 0x00000000edb8ef95-0x00000000671f5ce1, size=2792, cache=kmalloc-4k > > allocated by task 6771 on cpu 10 at 3137.860196s: > pci_alloc_dev+0x21/0x60 > pci_iov_add_virtfn+0x2a2/0x320 > sriov_enable+0x212/0x3e0 > efx_ef10_sriov_configure+0x67/0x80 [sfc] > efx_pci_sriov_configure+0x24/0x40 [sfc] > sriov_numvfs_store+0xba/0x140 > kernfs_fop_write_iter+0x11c/0x1b0 > new_sync_write+0x11f/0x1b0 > vfs_write+0x1eb/0x280 > ksys_write+0x5f/0xe0 > do_syscall_64+0x5c/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xae > > freed by task 6771 on cpu 12 at 3170.991309s: > device_release+0x34/0x90 > kobject_cleanup+0x3a/0x130 > pci_iov_remove_virtfn+0xd9/0x120 > sriov_disable+0x30/0xe0 > efx_ef10_pci_sriov_disable+0x57/0x70 [sfc] > efx_pci_sriov_configure+0x24/0x40 [sfc] > sriov_numvfs_store+0xfe/0x140 > kernfs_fop_write_iter+0x11c/0x1b0 > new_sync_write+0x11f/0x1b0 > vfs_write+0x1eb/0x280 > ksys_write+0x5f/0xe0 > do_syscall_64+0x5c/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xae > > Fixes: 3c5eb87605e85 ("sfc: create vports for VFs and assign random MAC addresses") > Reported-by: Yanghang Liu <yanghliu@...hat.com> > Signed-off-by: Íñigo Huguet <ihuguet@...hat.com> > --- > v2: add missing Fixes tag > > drivers/net/ethernet/sfc/ef10_sriov.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/sfc/ef10_sriov.c b/drivers/net/ethernet/sfc/ef10_sriov.c > index 7f5aa4a8c451..92550c7e85ce 100644 > --- a/drivers/net/ethernet/sfc/ef10_sriov.c > +++ b/drivers/net/ethernet/sfc/ef10_sriov.c > @@ -408,8 +408,9 @@ static int efx_ef10_pci_sriov_enable(struct efx_nic *efx, int num_vfs) > static int efx_ef10_pci_sriov_disable(struct efx_nic *efx, bool force) > { > struct pci_dev *dev = efx->pci_dev; > + struct efx_ef10_nic_data *nic_data = efx->nic_data; > unsigned int vfs_assigned = pci_vfs_assigned(dev); > - int rc = 0; > + int i, rc = 0; > > if (vfs_assigned && !force) { > netif_info(efx, drv, efx->net_dev, "VFs are assigned to guests; " > @@ -417,10 +418,13 @@ static int efx_ef10_pci_sriov_disable(struct efx_nic *efx, bool force) > return -EBUSY; > } > > - if (!vfs_assigned) > + if (!vfs_assigned) { > + for (i = 0; i < efx->vf_count; i++) > + nic_data->vf[i].pci_dev = NULL; > pci_disable_sriov(dev); > - else > + } else { > rc = -EBUSY; > + } > > efx_ef10_sriov_free_vf_vswitching(efx); > efx->vf_count = 0; > -- > 2.34.1
Powered by blists - more mailing lists