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
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 8 Aug 2022 20:30:23 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Lin Ma <linma@....edu.cn>, jesse.brandeburg@...el.com,
        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
Subject: Re: [PATCH v1] idb: Add lock to avoid data race



On 8/8/2022 7:59 PM, Lin Ma wrote:
> 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, just as commit 1e53834ce541 ("ixgbe: Add locking to
> prevent panic when setting sriov_numvfs to zero") shows. The
> igb_disable_sriov function also need to watch out the requests from VF
> driver.
> 
> 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 just as
> 1d53834ce541 did.
> 
> Fixes: c23d92b80e0b ("igb: Teardown SR-IOV before unregister_netdev()")
> Signed-off-by: Lin Ma <linma@....edu.cn>

You have a typo in your subject: s/idb/igb/
-- 
Florian

Powered by blists - more mailing lists