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

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f5c4d092-eeb7-2342-605f-7d86df9b1b10@gmail.com>
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