[v2] mfd: sm501: fix reference leak on failed device registration

[PATCH v2] mfd: sm501: fix reference leak on failed device registration

Posted by Valery Borovsky 1 month, 1 week ago

When platform_device_register() fails in sm501_register_device(), the
platform device allocated by sm501_create_subdev() has its struct device
initialized by device_initialize() inside platform_device_register(). The
error path logs the error but returns without dropping the device reference,
leaking the memory allocated by sm501_create_subdev():

  sm501_register_device()
    -> platform_device_register(pdev)
       -> device_initialize(&pdev->dev)   /* kref = 1 */
       -> platform_device_add(pdev)       /* fails */
    <- dev_err() called, kref still 1, sm501_device_release never called

The device's release callback (sm501_device_release) calls kfree() on the
containing sm501_device structure. Without platform_device_put(), this
memory is never freed.

Per platform_device_register() kernel-doc:

  NOTE: _Never_ directly free @pdev after calling this function, even if
  it returned an error! Always use platform_device_put() to give up the
  reference initialised in this function instead.

Fix this by calling platform_device_put() in the error branch, which
triggers sm501_device_release() and frees the allocated memory.

Fixes: b6d6454fdb66 ("[PATCH] mfd: SM501 core driver")
Cc: stable@vger.kernel.org
Signed-off-by: Valery Borovsky <vebohr@gmail.com>
---
 drivers/mfd/sm501.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
index 0ee6d8940e69..8276456b142f 100644
--- a/drivers/mfd/sm501.c
+++ b/drivers/mfd/sm501.c
@@ -704,9 +704,11 @@ static int sm501_register_device(struct sm501_devdata *sm,
 	if (ret >= 0) {
 		dev_dbg(sm->dev, "registered %s\n", pdev->name);
 		list_add_tail(&smdev->list, &sm->devices);
-	} else
+	} else {
 		dev_err(sm->dev, "error registering %s (%d)\n",
 			pdev->name, ret);
+		platform_device_put(pdev);
+	}
 
 	return ret;
 }
-- 
2.51.0

Re: [PATCH v2] mfd: sm501: fix reference leak on failed device registration

Posted by Miquel Raynal 1 month, 1 week ago

On Mon, 04 May 2026 15:48:41 +0300, Valery Borovsky wrote:
> When platform_device_register() fails in sm501_register_device(), the
> platform device allocated by sm501_create_subdev() has its struct device
> initialized by device_initialize() inside platform_device_register(). The
> error path logs the error but returns without dropping the device reference,
> leaking the memory allocated by sm501_create_subdev():
> 
>   sm501_register_device()
>     -> platform_device_register(pdev)
>        -> device_initialize(&pdev->dev)   /* kref = 1 */
>        -> platform_device_add(pdev)       /* fails */
>     <- dev_err() called, kref still 1, sm501_device_release never called
> 
> [...]

Applied to mtd/next, thanks!

[1/1] mfd: sm501: fix reference leak on failed device registration
      commit: faa9bba3fe2f37e7dcb26d4501d890fbfd7df160

Patche(s) should be available on mtd/linux.git and will be
part of the next PR (provided that no robot complains by then).

Kind regards,
Miquèl

Re: [PATCH v2] mfd: sm501: fix reference leak on failed device registration

Posted by Lee Jones 1 month, 1 week ago

On Mon, 04 May 2026, Miquel Raynal wrote:

> On Mon, 04 May 2026 15:48:41 +0300, Valery Borovsky wrote:
> > When platform_device_register() fails in sm501_register_device(), the
> > platform device allocated by sm501_create_subdev() has its struct device
> > initialized by device_initialize() inside platform_device_register(). The
> > error path logs the error but returns without dropping the device reference,
> > leaking the memory allocated by sm501_create_subdev():
> > 
> >   sm501_register_device()
> >     -> platform_device_register(pdev)
> >        -> device_initialize(&pdev->dev)   /* kref = 1 */
> >        -> platform_device_add(pdev)       /* fails */
> >     <- dev_err() called, kref still 1, sm501_device_release never called
> > 
> > [...]
> 
> Applied to mtd/next, thanks!

I think you misread the subject line.

> [1/1] mfd: sm501: fix reference leak on failed device registration
>       commit: faa9bba3fe2f37e7dcb26d4501d890fbfd7df160
> 
> Patche(s) should be available on mtd/linux.git and will be
> part of the next PR (provided that no robot complains by then).

Please remove this from your tree.  It should be handled via M[F]D.

Thanks.

-- 
Lee Jones

Re: [PATCH v2] mfd: sm501: fix reference leak on failed device registration

Posted by Miquel Raynal 1 month, 1 week ago

Hi Lee,

On 05/05/2026 at 16:00:13 +01, Lee Jones <lee@kernel.org> wrote:

> On Mon, 04 May 2026, Miquel Raynal wrote:
>
>> On Mon, 04 May 2026 15:48:41 +0300, Valery Borovsky wrote:
>> > When platform_device_register() fails in sm501_register_device(), the
>> > platform device allocated by sm501_create_subdev() has its struct device
>> > initialized by device_initialize() inside platform_device_register(). The
>> > error path logs the error but returns without dropping the device reference,
>> > leaking the memory allocated by sm501_create_subdev():
>> > 
>> >   sm501_register_device()
>> >     -> platform_device_register(pdev)
>> >        -> device_initialize(&pdev->dev)   /* kref = 1 */
>> >        -> platform_device_add(pdev)       /* fails */
>> >     <- dev_err() called, kref still 1, sm501_device_release never called
>> > 
>> > [...]
>> 
>> Applied to mtd/next, thanks!
>
> I think you misread the subject line.
>
>> [1/1] mfd: sm501: fix reference leak on failed device registration
>>       commit: faa9bba3fe2f37e7dcb26d4501d890fbfd7df160
>> 
>> Patche(s) should be available on mtd/linux.git and will be
>> part of the next PR (provided that no robot complains by then).
>
> Please remove this from your tree.  It should be handled via M[F]D.

Yes, it took a bit of time for me to receive my own answer so I replied
to the original patch immediately stating that I dropped it. For some
reason b4 applied this patch, whereas I was applying another m*t*d patch
from apparently the same series (?). Both the contribution and b4 behaviour
was strange.

Sorry for the noise.

Thanks,
Miquèl

[PATCH v3] mfd: sm501: fix reference leak on failed device registration

Posted by Valery Borovsky 1 month, 1 week ago

When platform_device_register() fails in sm501_register_device(), the
platform device allocated by sm501_create_subdev() has its struct device
initialized by device_initialize() inside platform_device_register(). The
error path logs the error but returns without dropping the device reference,
leaking the memory allocated by sm501_create_subdev():

  sm501_register_device()
    -> platform_device_register(pdev)
       -> device_initialize(&pdev->dev)   /* kref = 1 */
       -> platform_device_add(pdev)       /* fails */
    <- dev_err() called, kref still 1, sm501_device_release never called

The device's release callback (sm501_device_release) calls kfree() on the
containing sm501_device structure. Without platform_device_put(), this
memory is never freed.

Per platform_device_register() kernel-doc:

  NOTE: _Never_ directly free @pdev after calling this function, even if
  it returned an error! Always use platform_device_put() to give up the
  reference initialised in this function instead.

Fix this by calling platform_device_put() in the error branch, which
triggers sm501_device_release() and frees the allocated memory.

Fixes: b6d6454fdb66 ("[PATCH] mfd: SM501 core driver")
Cc: stable@vger.kernel.org
Signed-off-by: Valery Borovsky <vebohr@gmail.com>
---
 drivers/mfd/sm501.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
index 0ee6d8940e69..8276456b142f 100644
--- a/drivers/mfd/sm501.c
+++ b/drivers/mfd/sm501.c
@@ -704,9 +704,11 @@ static int sm501_register_device(struct sm501_devdata *sm,
 	if (ret >= 0) {
 		dev_dbg(sm->dev, "registered %s\n", pdev->name);
 		list_add_tail(&smdev->list, &sm->devices);
-	} else
+	} else {
 		dev_err(sm->dev, "error registering %s (%d)\n",
 			pdev->name, ret);
+		platform_device_put(pdev);
+	}
 
 	return ret;
 }
-- 
2.51.0

Re: [PATCH v3] mfd: sm501: fix reference leak on failed device registration

Posted by Valery Borovsky 1 month ago

Hi Lee,

Great, thanks for picking it up — confirmed on my end.

For context: an earlier version of this patch ended up briefly in mtd/next
by accident (Miquel applied it to the wrong tree, then dropped it), so I
appreciate you taking it directly into MFD.

Best,
Valery

Re: [PATCH v3] mfd: sm501: fix reference leak on failed device registration

Posted by Lee Jones 1 month ago

On Wed, 06 May 2026, Valery Borovsky wrote:

> When platform_device_register() fails in sm501_register_device(), the
> platform device allocated by sm501_create_subdev() has its struct device
> initialized by device_initialize() inside platform_device_register(). The
> error path logs the error but returns without dropping the device reference,
> leaking the memory allocated by sm501_create_subdev():
> 
>   sm501_register_device()
>     -> platform_device_register(pdev)
>        -> device_initialize(&pdev->dev)   /* kref = 1 */
>        -> platform_device_add(pdev)       /* fails */
>     <- dev_err() called, kref still 1, sm501_device_release never called
> 
> The device's release callback (sm501_device_release) calls kfree() on the
> containing sm501_device structure. Without platform_device_put(), this
> memory is never freed.
> 
> Per platform_device_register() kernel-doc:
> 
>   NOTE: _Never_ directly free @pdev after calling this function, even if
>   it returned an error! Always use platform_device_put() to give up the
>   reference initialised in this function instead.
> 
> Fix this by calling platform_device_put() in the error branch, which
> triggers sm501_device_release() and frees the allocated memory.
> 
> Fixes: b6d6454fdb66 ("[PATCH] mfd: SM501 core driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Valery Borovsky <vebohr@gmail.com>
> ---
>  drivers/mfd/sm501.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

I don't see a message from me acknowledging this, but it is applied to my tree.

-- 
Lee Jones