Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Missing NULL validation for the ACPI companion of the parent device leads to a potential NULL pointer dereference in `acpi_remove_pm_notifier()`. --
--- Patch [1]: [PATCH] pci: Add broken-GPE quirk for Lenovo Legion 16APH8 and 16AHP9 Nvidia GPUs --- commit c5f31e20fc4140461290146739333c87be79e5ce Author: Lyude Paul <[email protected]> pci: Add broken-GPE quirk for Lenovo Legion 16APH8 and 16AHP9 Nvidia GPUs This commit adds a PCI quirk for specific Lenovo Legion laptops. It disables spurious GPE wakeup events on the parent PCIe port of the Nvidia GPU after it enters D3Cold. Fixes: https://github.com/NVIDIA/open-gpu-kernel-modules/issues/905 > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index b09f27f7846fc..4d1f1565b61d3 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c [ ... ] > @@ -6165,6 +6165,82 @@ DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, > 0x13b1, > PCI_CLASS_DISPLAY_VGA, 8, > quirk_reset_lenovo_thinkpad_p50_nvgpu); > > +#ifdef CONFIG_ACPI > +#ifdef CONFIG_DMI [ ... ] > +static void quirk_nvidia_dgpu_broken_gpe(struct pci_dev *pdev) > +{ > + struct acpi_device *parent_adev; > + struct device *parent_dev; > + int ret; [ ... ] > + parent_dev = pci_physfn(pdev)->dev.parent; > + if (!parent_dev) { > + pci_err(pdev, > + "Can't find PCIe parent? Your Nvidia GPU will have > broken runtime PM\n"); > + return; > + } > + parent_adev = ACPI_COMPANION(parent_dev); > + > + /* The spurious GPEs will be sent from the ACPI device for the PCIe > port this GPU is > + * connected to, so remove our PM notifier to turn them into a no-op. > + */ > + ret = acpi_remove_pm_notifier(ACPI_COMPANION(parent_dev)); [Severity: High] Can this cause a NULL pointer dereference if the parent PCIe bridge lacks an ACPI companion? ACPI_COMPANION() can return NULL if the system is booted with acpi=off or if the node simply lacks an ACPI companion. If that happens, this passes NULL to acpi_remove_pm_notifier(), which dereferences its argument directly: drivers/acpi/device_pm.c:acpi_remove_pm_notifier() { ... if (!adev->wakeup.flags.notifier_present) ... } Should parent_adev be checked for NULL before calling acpi_remove_pm_notifier()? Also, it looks like parent_adev is assigned but then ACPI_COMPANION(parent_dev) is called again instead of using the local variable. > + if (ACPI_FAILURE(ret)) > + pci_err(pdev, "Removing PM notifier failed: %d\n", ret); > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
