In i2c_amd_probe(), amd_mp2_find_device() utilizes driver_find_next_device() which internally calls driver_find_device() to locate the matching device. driver_find_device() increments the reference count of the found device by calling get_device(), but amd_mp2_find_device() fails to call put_device() to decrement the reference count before returning. This results in a reference count leak of the PCI device each time i2c_amd_probe() is executed, which may prevent the device from being properly released and cause a memory leak.
Found by code review.
Cc: stable@vger.kernel.org Fixes: 529766e0a011 ("i2c: Add drivers for the AMD PCIe MP2 I2C controller") Signed-off-by: Ma Ke make24@iscas.ac.cn --- Changes in v2: - modified the missing initialization in the patch. Sorry for the omission. --- drivers/i2c/busses/i2c-amd-mp2-pci.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-amd-mp2-pci.c b/drivers/i2c/busses/i2c-amd-mp2-pci.c index ef7370d3dbea..60edbabc2986 100644 --- a/drivers/i2c/busses/i2c-amd-mp2-pci.c +++ b/drivers/i2c/busses/i2c-amd-mp2-pci.c @@ -458,13 +458,16 @@ struct amd_mp2_dev *amd_mp2_find_device(void) { struct device *dev; struct pci_dev *pci_dev; + struct amd_mp2_dev *mp2_dev;
dev = driver_find_next_device(&amd_mp2_pci_driver.driver, NULL); if (!dev) return NULL;
pci_dev = to_pci_dev(dev); - return (struct amd_mp2_dev *)pci_get_drvdata(pci_dev); + mp2_dev = (struct amd_mp2_dev *)pci_get_drvdata(pci_dev); + put_device(dev); + return mp2_dev; } EXPORT_SYMBOL_GPL(amd_mp2_find_device);
In i2c_amd_probe(), amd_mp2_find_device() utilizes driver_find_next_device() which internally calls driver_find_device() to locate the matching device. driver_find_device() increments the reference count of the found device by calling get_device(), but amd_mp2_find_device() fails to call put_device() to decrement the reference count before returning. This results in a reference count leak of the PCI device each time i2c_amd_probe() is executed, which may prevent the device from being properly released and cause a memory leak.
Under which circumstances would you become interested to apply an attribute like “__free(put_device)”? https://elixir.bootlin.com/linux/v6.17-rc7/source/include/linux/device.h#L11...
Regards, Markus
On Sun, Sep 28, 2025 at 01:00:13PM +0200, Markus Elfring wrote:
In i2c_amd_probe(), amd_mp2_find_device() utilizes driver_find_next_device() which internally calls driver_find_device() to locate the matching device. driver_find_device() increments the reference count of the found device by calling get_device(), but amd_mp2_find_device() fails to call put_device() to decrement the reference count before returning. This results in a reference count leak of the PCI device each time i2c_amd_probe() is executed, which may prevent the device from being properly released and cause a memory leak.
Under which circumstances would you become interested to apply an attribute like “__free(put_device)”? https://elixir.bootlin.com/linux/v6.17-rc7/source/include/linux/device.h#L11...
Regards, Markus
Hi,
This is the semi-friendly patch-bot of Greg Kroah-Hartman.
Markus, you seem to have sent a nonsensical or otherwise pointless review comment to a patch submission on a Linux kernel developer mailing list. I strongly suggest that you not do this anymore. Please do not bother developers who are actively working to produce patches and features with comments that, in the end, are a waste of time.
Patch submitter, please ignore Markus's suggestion; you do not need to follow it at all. The person/bot/AI that sent it is being ignored by almost all Linux kernel maintainers for having a persistent pattern of behavior of producing distracting and pointless commentary, and inability to adapt to feedback. Please feel free to also ignore emails from them.
thanks,
greg k-h's patch email bot
Hi,
diff --git a/drivers/i2c/busses/i2c-amd-mp2-pci.c b/drivers/i2c/busses/i2c-amd-mp2-pci.c index ef7370d3dbea..60edbabc2986 100644 --- a/drivers/i2c/busses/i2c-amd-mp2-pci.c +++ b/drivers/i2c/busses/i2c-amd-mp2-pci.c @@ -458,13 +458,16 @@ struct amd_mp2_dev *amd_mp2_find_device(void) { struct device *dev; struct pci_dev *pci_dev;
- struct amd_mp2_dev *mp2_dev;
dev = driver_find_next_device(&amd_mp2_pci_driver.driver, NULL); if (!dev) return NULL; pci_dev = to_pci_dev(dev);
- return (struct amd_mp2_dev *)pci_get_drvdata(pci_dev);
- mp2_dev = (struct amd_mp2_dev *)pci_get_drvdata(pci_dev);
- put_device(dev);
- return mp2_dev;
the patch is good, but I don't think you need to declare mp2_dev because to_pci_dev(dev) should work even without hodling the reference of dev.
I also have to agree with Markus that something like:
struct device *dev __free(put_device) = ...; /* it can also be NULL */
would work nicer.
Thanks, Andi
} EXPORT_SYMBOL_GPL(amd_mp2_find_device); -- 2.17.1
Hi again,
On Thu, Oct 02, 2025 at 01:56:30AM +0200, Andi Shyti wrote:
Hi,
diff --git a/drivers/i2c/busses/i2c-amd-mp2-pci.c b/drivers/i2c/busses/i2c-amd-mp2-pci.c index ef7370d3dbea..60edbabc2986 100644 --- a/drivers/i2c/busses/i2c-amd-mp2-pci.c +++ b/drivers/i2c/busses/i2c-amd-mp2-pci.c @@ -458,13 +458,16 @@ struct amd_mp2_dev *amd_mp2_find_device(void) { struct device *dev; struct pci_dev *pci_dev;
- struct amd_mp2_dev *mp2_dev;
dev = driver_find_next_device(&amd_mp2_pci_driver.driver, NULL); if (!dev) return NULL; pci_dev = to_pci_dev(dev);
- return (struct amd_mp2_dev *)pci_get_drvdata(pci_dev);
- mp2_dev = (struct amd_mp2_dev *)pci_get_drvdata(pci_dev);
- put_device(dev);
- return mp2_dev;
the patch is good, but I don't think you need to declare mp2_dev because to_pci_dev(dev) should work even without hodling the reference of dev.
I also have to agree with Markus that something like:
struct device *dev __free(put_device) = ...; /* it can also be NULL */
sorry, please ignore this last comment, because if !dev we shouldn't call put_device().
Andi
linux-stable-mirror@lists.linaro.org