2022-11-23 09:23:59

by Xiongfeng Wang

Subject: [PATCH] hwrng: amd - Fix PCI device refcount leak

for_each_pci_dev() is implemented by pci_get_device(). The comment of
pci_get_device() says that it will increase the reference count for the
returned pci_dev and also decrease the reference count for the input
pci_dev @from if it is not NULL.

If we break for_each_pci_dev() loop with pdev not NULL, we need to call
pci_dev_put() to decrease the reference count. Add the missing
pci_dev_put() for the normal and error path.

Fixes: 96d63c0297cc ("[PATCH] Add AMD HW RNG driver")
Signed-off-by: Xiongfeng Wang <[email protected]>
---
drivers/char/hw_random/amd-rng.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/char/hw_random/amd-rng.c b/drivers/char/hw_random/amd-rng.c
index c22d4184bb61..46e997366b15 100644
--- a/drivers/char/hw_random/amd-rng.c
+++ b/drivers/char/hw_random/amd-rng.c
@@ -143,15 +143,19 @@ static int __init amd_rng_mod_init(void)
found:
err = pci_read_config_dword(pdev, 0x58, &pmbase);
if (err)
- return err;
+ goto put_dev;

pmbase &= 0x0000FF00;
- if (pmbase == 0)
- return -EIO;
+ if (pmbase == 0) {
+ err = -EIO;
+ goto put_dev;
+ }

priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
+ if (!priv) {
+ err = -ENOMEM;
+ goto put_dev;
+ }

if (!request_region(pmbase + PMBASE_OFFSET, PMBASE_SIZE, DRV_NAME)) {
dev_err(&pdev->dev, DRV_NAME " region 0x%x already in use!\n",
@@ -185,6 +189,8 @@ static int __init amd_rng_mod_init(void)
release_region(pmbase + PMBASE_OFFSET, PMBASE_SIZE);
out:
kfree(priv);
+put_dev:
+ pci_dev_put(pdev);
return err;
}

@@ -201,6 +207,8 @@ static void __exit amd_rng_mod_exit(void)
release_region(priv->pmbase + PMBASE_OFFSET, PMBASE_SIZE);

kfree(priv);
+
+ pci_dev_put(priv->pcidev);
}

module_init(amd_rng_mod_init);
--
2.20.1

2022-12-02 10:07:20

by Herbert Xu

Subject: Re: [PATCH] hwrng: amd - Fix PCI device refcount leak

On Wed, Nov 23, 2022 at 05:39:49PM +0800, Xiongfeng Wang wrote:
>
> @@ -201,6 +207,8 @@ static void __exit amd_rng_mod_exit(void)
> release_region(priv->pmbase + PMBASE_OFFSET, PMBASE_SIZE);
>
> kfree(priv);
> +
> + pci_dev_put(priv->pcidev);

Oops, this doesn't look right. Please fix and resubmit.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2022-12-02 10:11:32

by Herbert Xu

Subject: Re: [PATCH] hwrng: amd - Fix PCI device refcount leak

On Wed, Nov 23, 2022 at 05:39:49PM +0800, Xiongfeng Wang wrote:
> for_each_pci_dev() is implemented by pci_get_device(). The comment of
> pci_get_device() says that it will increase the reference count for the
> returned pci_dev and also decrease the reference count for the input
> pci_dev @from if it is not NULL.
>
> If we break for_each_pci_dev() loop with pdev not NULL, we need to call
> pci_dev_put() to decrease the reference count. Add the missing
> pci_dev_put() for the normal and error path.
>
> Fixes: 96d63c0297cc ("[PATCH] Add AMD HW RNG driver")
> Signed-off-by: Xiongfeng Wang <[email protected]>
> ---
> drivers/char/hw_random/amd-rng.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)

The driver geode-rng seems to have the same problem, could you please
fix that as well?

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2022-12-02 11:02:23

by Xiongfeng Wang

Subject: Re: [PATCH] hwrng: amd - Fix PCI device refcount leak

On 2022/12/2 17:43, Herbert Xu wrote:
> On Wed, Nov 23, 2022 at 05:39:49PM +0800, Xiongfeng Wang wrote:
>>
>> @@ -201,6 +207,8 @@ static void __exit amd_rng_mod_exit(void)
>> release_region(priv->pmbase + PMBASE_OFFSET, PMBASE_SIZE);
>>
>> kfree(priv);
>> +
>> + pci_dev_put(priv->pcidev);
>
> Oops, this doesn't look right. Please fix and resubmit.

Ah, Sorry, my mistake! I will send another version with the fix of geode-rng.

Thanks,
Xiongfeng

>
> Thanks,
>