On 3/9/2025 11:22 PM, Simon Horman wrote:
On Thu, Mar 06, 2025 at 04:39:56PM -0800, Emil Tantilov wrote:
Driver calls idpf_remove() from idpf_shutdown(), which can end up
calling idpf_remove() again when disabling SRIOV.
echo 1 > /sys/class/net/<netif>/device/sriov_numvfs
reboot
BUG: kernel NULL pointer dereference, address: 0000000000000020
...
RIP: 0010:idpf_remove+0x22/0x1f0 [idpf]
...
? idpf_remove+0x22/0x1f0 [idpf]
? idpf_remove+0x1e4/0x1f0 [idpf]
pci_device_remove+0x3f/0xb0
device_release_driver_internal+0x19f/0x200
pci_stop_bus_device+0x6d/0x90
pci_stop_and_remove_bus_device+0x12/0x20
pci_iov_remove_virtfn+0xbe/0x120
sriov_disable+0x34/0xe0
idpf_sriov_configure+0x58/0x140 [idpf]
idpf_remove+0x1b9/0x1f0 [idpf]
idpf_shutdown+0x12/0x30 [idpf]
pci_device_shutdown+0x35/0x60
device_shutdown+0x156/0x200
...
Replace the direct idpf_remove() call in idpf_shutdown() with
idpf_vc_core_deinit() and idpf_deinit_dflt_mbx(), which perform
the bulk of the cleanup, such as stopping the init task, freeing IRQs,
destroying the vports and freeing the mailbox.
Hi Emil,
I think it would be worth adding some commentary on the rest of
the clean-up performed by idpf_remove() and why it is correct
The main reason behind the change is to avoid calling sriov_disable(),
which ends up calling idpf_remove() again via pci_device_remove(). The
idpf_remove() will crash in that situation as it attempts to access
adapter pointer, which was already freed.
to no longer do so directly from a call to idpf_remove() from
idpf_shutdown() (IOW, it isn't clear to me :).
I assume you are asking what portion of the idpf_remove() will not be
present in idpf_shutdown() as result? Aside from not calling
sriov_disable(), there is a small cleanup of stale netdevs and the
destruction of WQs, which did not seem like would be needed on shutdown.
Then again, I was not able to find documentation on what steps are
required for shutdown and mostly checked on how other drivers handle it
(where there is no 1:1 overlap between shutdown and remove), and applied
similar steps to idpf. Ideally I do not wish to do more than is needed
for that flow.
...
