Re: [PATCH] leds: fix typo "retrive" in comment

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Low] The commit message fails to follow the LED subsystem guidelines for prefix format and capitalization.

Pre-existing issues:
- [High] If pci_register_driver() does not bind a device (e.g., due to nodetect=1 on a system without the hardware), nas_gpio_pci_dev remains NULL, causing an invalid pointer dereference during LED registration.
- [High] The driver registers LEDs globally during module init rather than in probe, and fails to unregister them upon device removal, violating subsystem rules and leading to use-after-release bugs.
--

commit 933a15295df77c1222bb18879d12d9d6414867f1
Author: Hemanth Selam <hemanth.selam@gmail.com>

leds: fix typo "retrive" in comment

Correct "retrive" to "retrieve", reported by scripts/checkpatch.pl using
the misspelling list in scripts/spelling.txt. Only touches comments, no
code changes.

> leds: fix typo "retrive" in comment

[Severity: Low]
Does this commit message violate the LED subsystem conventions? According to
the subsystem guidelines, the prefix format should be
leds: <Driver>: <Capitalized description>, but this subject omits the driver
name and does not capitalize the description.


> diff --git a/drivers/leds/leds-ss4200.c b/drivers/leds/leds-ss4200.c
> index fcaa34706b6ca..1b08a40961ba4 100644
> --- a/drivers/leds/leds-ss4200.c
> +++ b/drivers/leds/leds-ss4200.c
> @@ -339,7 +339,7 @@ static void ich7_lpc_cleanup(struct device *dev)
>  
>  /*
>   * The OS has determined that the LPC of the Intel ICH7 Southbridge is present
> - * so we can retrive the required operational information and prepare the GPIO.
> + * so we can retrieve the required operational information and prepare the GPIO.
>   */
>  static struct pci_dev *nas_gpio_pci_dev;
>  static int ich7_lpc_probe(struct pci_dev *dev,

[Severity: High]
This is a pre-existing issue, but if pci_register_driver() does not bind a
device (for example, due to nodetect=1 on a system without the hardware),
ich7_lpc_probe() is never called and nas_gpio_pci_dev remains NULL.

Can this cause an invalid pointer dereference during LED registration?

drivers/leds/leds-ss4200.c:nas_gpio_init() {
    ...
	ret = pci_register_driver(&nas_gpio_pci_driver);
	if (ret)
		return ret;
	for (i = 0; i < ARRAY_SIZE(nasgpio_leds); i++) {
		ret = register_nasgpio_led(i);
    ...
}

drivers/leds/leds-ss4200.c:register_nasgpio_led() {
    ...
	return led_classdev_register(&nas_gpio_pci_dev->dev, led);
}

Passing &nas_gpio_pci_dev->dev yields an offset pointer from NULL, which will
be dereferenced by the driver core.


[Severity: High]
This is a pre-existing issue, but the driver registers LEDs globally during
module initialization, decoupled from the PCI device lifecycle.

Can this lead to a use-after-release issue?

When the PCI device is unbound via sysfs, ich7_lpc_remove() is called:

drivers/leds/leds-ss4200.c:ich7_lpc_remove() {
	ich7_lpc_cleanup(&dev->dev);
	pci_disable_device(dev);
}

This releases the I/O region and disables the PCI device, but fails to
unregister the globally registered LEDs. A privileged user could interact with
dangling LED sysfs nodes that access the released I/O ports.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904122738.33145-1-hemanth.selam@gmail.com?part=1