[PATCH] MIPS: ralink: Fix refcount leak in ill_acc_of_setup()

[PATCH] MIPS: ralink: Fix refcount leak in ill_acc_of_setup()

[Thorsten Blum]

The of_find_device_by_node() function increments the reference count of
the embedded device, which should be released with put_device() when it
is no longer needed.

In ill_acc_of_setup(), put_device() is only called on error paths, but
not on the success path. Fix this by calling put_device() before
returning successfully.

Compile-tested only.

Cc: stable@vger.kernel.org
Fixes: 5433acd81e873 ("MIPS: ralink: add illegal access driver")
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 arch/mips/ralink/ill_acc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/ralink/ill_acc.c b/arch/mips/ralink/ill_acc.c
index 25341b2319d0..6d1d829854b6 100644
--- a/arch/mips/ralink/ill_acc.c
+++ b/arch/mips/ralink/ill_acc.c
@@ -84,6 +84,7 @@ static int __init ill_acc_of_setup(void)
 	rt_memc_w32(ILL_INT_STATUS, REG_ILL_ACC_TYPE);
 
 	dev_info(&pdev->dev, "irq registered\n");
+	put_device(&pdev->dev);
 
 	return 0;
 }
-- 
2.49.0

Re: [PATCH] MIPS: ralink: Fix refcount leak in ill_acc_of_setup()

[Jonas Gorski]

Hi,

On Mon, Apr 7, 2025 at 10:32 AM Thorsten Blum <thorsten.blum@linux.dev> wrote:
>
> The of_find_device_by_node() function increments the reference count of
> the embedded device, which should be released with put_device() when it
> is no longer needed.
>
> In ill_acc_of_setup(), put_device() is only called on error paths, but
> not on the success path. Fix this by calling put_device() before
> returning successfully.

I would think this is very much deliberate as the device is used as
the priv argument of the registered IRQ handler. AFAIU as long as that
one is live the reference of the device needs to be kept.

Dropping the reference of the device should only be done after
freeing/unregistering the IRQ again, which currently never happens.

Best regards,
Jonas

Re: [PATCH] MIPS: ralink: Fix refcount leak in ill_acc_of_setup()

[Thorsten Blum]

On 9. Apr 2025, at 14:57, Jonas Gorski wrote:
> On Mon, Apr 7, 2025 at 10:32 AM Thorsten Blum wrote:
>> 
>> The of_find_device_by_node() function increments the reference count of
>> the embedded device, which should be released with put_device() when it
>> is no longer needed.
>> 
>> In ill_acc_of_setup(), put_device() is only called on error paths, but
>> not on the success path. Fix this by calling put_device() before
>> returning successfully.
> 
> I would think this is very much deliberate as the device is used as
> the priv argument of the registered IRQ handler. AFAIU as long as that
> one is live the reference of the device needs to be kept.
> 
> Dropping the reference of the device should only be done after
> freeing/unregistering the IRQ again, which currently never happens.

Thanks for the explanation. I assumed request_irq() would increment the
refcount, but that's apparently not the case because it's just a cookie.

Thanks,
Thorsten