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, 16 Aug 2022 10:35:05 +0000 From: "Jankowski, Konrad0" <konrad0.jankowski@...el.com> To: Lin Ma <linma@....edu.cn>, "Brandeburg, Jesse" <jesse.brandeburg@...el.com>, "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>, "ast@...nel.org" <ast@...nel.org>, "daniel@...earbox.net" <daniel@...earbox.net>, "hawk@...nel.org" <hawk@...nel.org>, "john.fastabend@...il.com" <john.fastabend@...il.com>, "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "bpf@...r.kernel.org" <bpf@...r.kernel.org> Subject: RE: [Intel-wired-lan] [PATCH v3] igb: Add lock to avoid data race > -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf Of > Lin Ma > Sent: Tuesday, August 9, 2022 9:36 AM > To: Brandeburg, Jesse <jesse.brandeburg@...el.com>; Nguyen, Anthony L > <anthony.l.nguyen@...el.com>; davem@...emloft.net; > edumazet@...gle.com; kuba@...nel.org; pabeni@...hat.com; > ast@...nel.org; daniel@...earbox.net; hawk@...nel.org; > john.fastabend@...il.com; intel-wired-lan@...ts.osuosl.org; > netdev@...r.kernel.org; linux-kernel@...r.kernel.org; > bpf@...r.kernel.org > Cc: Lin Ma <linma@....edu.cn> > Subject: [Intel-wired-lan] [PATCH v3] igb: Add lock to avoid data race > > The commit c23d92b80e0b ("igb: Teardown SR-IOV before > unregister_netdev()") places the unregister_netdev() call after the > igb_disable_sriov() call to avoid functionality issue. > > However, it introduces several race conditions when detaching a device. > For example, when .remove() is called, the below interleaving leads to use- > after-free. > > (FREE from device detaching) | (USE from netdev core) > igb_remove | igb_ndo_get_vf_config > igb_disable_sriov | vf >= adapter->vfs_allocated_count? > kfree(adapter->vf_data) | > adapter->vfs_allocated_count = 0 | > | memcpy(... adapter->vf_data[vf] > > Moreover, the igb_disable_sriov() also suffers from data race with the > requests from VF driver. > > (FREE from device detaching) | (USE from requests) > igb_remove | igb_msix_other > igb_disable_sriov | igb_msg_task > kfree(adapter->vf_data) | vf < adapter->vfs_allocated_count > adapter->vfs_allocated_count = 0 | > > To this end, this commit first eliminates the data races from netdev core by > using rtnl_lock (similar to commit 719479230893 ("dpaa2-eth: add MAC/PHY > support through phylink")). And then adds a spinlock to eliminate races from > driver requests. (similar to commit 1e53834ce541 > ("ixgbe: Add locking to prevent panic when setting sriov_numvfs to zero") > > > Fixes: c23d92b80e0b ("igb: Teardown SR-IOV before unregister_netdev()") > Signed-off-by: Lin Ma <linma@....edu.cn> > --- > V2 -> V3: make the commit message much clear > V1 -> V2: fix typo in title idb -> igb > V0 -> V1: change title from "Add rtnl_lock" to "Add lock" > add additional spinlock as suggested by Jakub, according to > 1e53834ce541 ("ixgbe: Add locking to prevent panic when setting > sriov_numvfs to zero") > > drivers/net/ethernet/intel/igb/igb.h | 2 ++ > drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++++++- > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/igb/igb.h > b/drivers/net/ethernet/intel/igb/igb.h > index 2d3daf022651..015b78144114 100644 > --- a/drivers/net/ethernet/intel/igb/igb.h > +++ b/drivers/net/ethernet/intel/igb/igb.h > @@ -664,6 +664,8 @@ struct igb_adapter { Tested-by: Konrad Jankowski <konrad0.jankowski@...el.com>
Powered by blists - more mailing lists