Re: [PATCH v1] idb: Add lock to avoid data race

[Florian Fainelli]

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@xxxxxxxxxx>

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