The probe() id argument may be NULL in 2 scenarios:
1. brcmf_pcie_pm_leave_D3() calling brcmf_pcie_probe() to reprobe the device.
2. If a user tries to manually bind the driver from sysfs then the sdio / pcie / usb probe() function gets called with NULL as id argument.
1. Is being hit by users causing the following oops on resume and causing wifi to stop working:
BUG: kernel NULL pointer dereference, address: 0000000000000018 <snip> Hardware name: Dell Inc. XPS 13 9350/0PWNCR, BIDS 1.13.0 02/10/2020 Workgueue: events_unbound async_run_entry_fn RIP: 0010:brcmf_pcie_probe+Ox16b/0x7a0 [brcmfmac] <snip> Call Trace: <TASK> brcmf_pcie_pm_leave_D3+0xc5/8x1a0 [brcmfmac be3b4cefca451e190fa35be8f00db1bbec293887] ? pci_pm_resume+0x5b/0xf0 ? pci_legacy_resume+0x80/0x80 dpm_run_callback+0x47/0x150 device_resume+0xa2/0x1f0 async_resume+0x1d/0x30 <snip>
Fix this by checking for id being NULL.
In the case of brcmf_pcie_probe() also add a manual lookup of the id when it is NULL so that fwvid will still get set correctly on reprobe on resume.
Fixes: da6d9c8ecd00 ("wifi: brcmfmac: add firmware vendor info in driver info") Reported-by: Felix nimrod4garoa@gmail.com Link: https://lore.kernel.org/regressions/4ef3f252ff530cbfa336f5a0d80710020fc5cb1e... Cc: Arend van Spriel arend.vanspriel@broadcom.com Cc: stable@vger.kernel.org Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 2 +- drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 8 +++++++- drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c index 65d4799a5658..833524bc9b50 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c @@ -1067,7 +1067,7 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func, sdiodev->bus_if = bus_if; bus_if->bus_priv.sdio = sdiodev; bus_if->proto_type = BRCMF_PROTO_BCDC; - bus_if->fwvid = id->driver_data; + bus_if->fwvid = id ? id->driver_data : BRCMF_FWVENDOR_INVALID; dev_set_drvdata(&func->dev, bus_if); dev_set_drvdata(&sdiodev->func1->dev, bus_if); sdiodev->dev = &sdiodev->func1->dev; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index a9b9b2dc62d4..b29ab19d695d 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -2339,6 +2339,9 @@ static void brcmf_pcie_debugfs_create(struct device *dev) } #endif
+/* Forward declaration for pci_match_id() call */ +static const struct pci_device_id brcmf_pcie_devid_table[]; + static int brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id) { @@ -2351,6 +2354,9 @@ brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
brcmf_dbg(PCIE, "Enter %x:%x\n", pdev->vendor, pdev->device);
+ if (!id) + id = pci_match_id(brcmf_pcie_devid_table, pdev); + ret = -ENOMEM; devinfo = kzalloc(sizeof(*devinfo), GFP_KERNEL); if (devinfo == NULL) @@ -2406,7 +2412,7 @@ brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id) bus->bus_priv.pcie = pcie_bus_dev; bus->ops = &brcmf_pcie_bus_ops; bus->proto_type = BRCMF_PROTO_MSGBUF; - bus->fwvid = id->driver_data; + bus->fwvid = id ? id->driver_data : BRCMF_FWVENDOR_INVALID; bus->chip = devinfo->coreid; bus->wowl_supported = pci_pme_capable(pdev, PCI_D3hot); dev_set_drvdata(&pdev->dev, bus); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c index 246843aeb696..2541a47b1f8f 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c @@ -1425,7 +1425,7 @@ brcmf_usb_probe(struct usb_interface *intf, const struct usb_device_id *id) else brcmf_dbg(USB, "Broadcom full speed USB WLAN interface detected\n");
- ret = brcmf_usb_probe_cb(devinfo, id->driver_info); + ret = brcmf_usb_probe_cb(devinfo, id ? id->driver_info : BRCMF_FWVENDOR_INVALID); if (ret) goto fail;
On 5/10/2023 12:00 PM, Hans de Goede wrote:
The probe() id argument may be NULL in 2 scenarios:
brcmf_pcie_pm_leave_D3() calling brcmf_pcie_probe() to reprobe the device.
If a user tries to manually bind the driver from sysfs then the sdio / pcie / usb probe() function gets called with NULL as id argument.
Is being hit by users causing the following oops on resume and causing
wifi to stop working:
BUG: kernel NULL pointer dereference, address: 0000000000000018
<snip> Hardware name: Dell Inc. XPS 13 9350/0PWNCR, BIDS 1.13.0 02/10/2020 Workgueue: events_unbound async_run_entry_fn RIP: 0010:brcmf_pcie_probe+Ox16b/0x7a0 [brcmfmac] <snip> Call Trace: <TASK> brcmf_pcie_pm_leave_D3+0xc5/8x1a0 [brcmfmac be3b4cefca451e190fa35be8f00db1bbec293887] ? pci_pm_resume+0x5b/0xf0 ? pci_legacy_resume+0x80/0x80 dpm_run_callback+0x47/0x150 device_resume+0xa2/0x1f0 async_resume+0x1d/0x30 <snip>
Fix this by checking for id being NULL.
In the case of brcmf_pcie_probe() also add a manual lookup of the id when it is NULL so that fwvid will still get set correctly on reprobe on resume.
Hi Hans,
Thanks for looking into this and coming up with a fix. Does it make sense to proceed with BRCMF_FWVENDOR_INVALID though. I think it will end up in probe failure in brcmf_fwvid_attach() because of it so why not bail out right away.
Regards, Arend
Fixes: da6d9c8ecd00 ("wifi: brcmfmac: add firmware vendor info in driver info") Reported-by: Felix nimrod4garoa@gmail.com Link: https://lore.kernel.org/regressions/4ef3f252ff530cbfa336f5a0d80710020fc5cb1e... Cc: Arend van Spriel arend.vanspriel@broadcom.com Cc: stable@vger.kernel.org Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 2 +- drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 8 +++++++- drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-)
Hi Arend,
On 5/10/23 14:12, Arend van Spriel wrote:
On 5/10/2023 12:00 PM, Hans de Goede wrote:
The probe() id argument may be NULL in 2 scenarios:
- brcmf_pcie_pm_leave_D3() calling brcmf_pcie_probe() to reprobe
the device.
- If a user tries to manually bind the driver from sysfs then the sdio /
pcie / usb probe() function gets called with NULL as id argument.
- Is being hit by users causing the following oops on resume and causing
wifi to stop working:
BUG: kernel NULL pointer dereference, address: 0000000000000018
<snip> Hardware name: Dell Inc. XPS 13 9350/0PWNCR, BIDS 1.13.0 02/10/2020 Workgueue: events_unbound async_run_entry_fn RIP: 0010:brcmf_pcie_probe+Ox16b/0x7a0 [brcmfmac] <snip> Call Trace: <TASK> brcmf_pcie_pm_leave_D3+0xc5/8x1a0 [brcmfmac be3b4cefca451e190fa35be8f00db1bbec293887] ? pci_pm_resume+0x5b/0xf0 ? pci_legacy_resume+0x80/0x80 dpm_run_callback+0x47/0x150 device_resume+0xa2/0x1f0 async_resume+0x1d/0x30 <snip>
Fix this by checking for id being NULL.
In the case of brcmf_pcie_probe() also add a manual lookup of the id when it is NULL so that fwvid will still get set correctly on reprobe on resume.
Hi Hans,
Thanks for looking into this and coming up with a fix. Does it make sense to proceed with BRCMF_FWVENDOR_INVALID though. I think it will end up in probe failure in brcmf_fwvid_attach() because of it so why not bail out right away.
You are right since the vendor stuff is in separate modules I thought the driver would just continue without, but that indeed does not work.
I have just prepared a version 2 which immediately bails from probe() if id is still null after doing a pci_match_id() / usb_match_id() to try and lookup the id. Note for the SDIO case v2 will immediately bail on a NULL id since there is no sdio_match_id() helper.
I'll send out v2 right away.
Regards,
Hans
Fixes: da6d9c8ecd00 ("wifi: brcmfmac: add firmware vendor info in driver info") Reported-by: Felix nimrod4garoa@gmail.com Link: https://lore.kernel.org/regressions/4ef3f252ff530cbfa336f5a0d80710020fc5cb1e... Cc: Arend van Spriel arend.vanspriel@broadcom.com Cc: stable@vger.kernel.org Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 2 +- drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 8 +++++++- drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-)
linux-stable-mirror@lists.linaro.org