[PATCH] siox: fix master memory leak on registration failure

[PATCH] siox: fix master memory leak on registration failure

[Guangshuo Li]

siox_master_register() takes an additional reference on the master
device with get_device() so that the SIOX core owns a reference until
siox_master_unregister() is called.

If kthread_run() fails, siox_master_register() returns without dropping
this reference. Similarly, if device_add() fails, the poll thread is
stopped but the reference acquired by siox_master_register() is not
released.

For devm-allocated masters, the devres cleanup only drops the original
allocation reference. The additional registration reference therefore
remains held, preventing siox_master_release() from being called and
leaking the siox_master allocation.

Drop the reference acquired by siox_master_register() on both failure
paths. On device_add() failure, kthread_stop() first lets the poll thread
drop its own reference before the registration reference is released.

The issue was identified by a static analysis tool I developed and
confirmed by manual review.

Fixes: 2c12932b8e65 ("siox: Don't pass the reference on a master in siox_master_register()")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/siox/siox-core.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/siox/siox-core.c b/drivers/siox/siox-core.c
index 3e8f3b6a4555..ea1ef0a5c991 100644
--- a/drivers/siox/siox-core.c
+++ b/drivers/siox/siox-core.c
@@ -753,14 +753,21 @@ int siox_master_register(struct siox_master *smaster)
 	smaster->poll_thread = kthread_run(siox_poll_thread, smaster,
 					   "siox-%d", smaster->busno);
 	if (IS_ERR(smaster->poll_thread)) {
+		ret = PTR_ERR(smaster->poll_thread);
 		smaster->active = 0;
-		return PTR_ERR(smaster->poll_thread);
+		goto err_put_device;
 	}
 
 	ret = device_add(&smaster->dev);
-	if (ret)
+	if (ret) {
 		kthread_stop(smaster->poll_thread);
+		goto err_put_device;
+	}
 
+	return 0;
+
+err_put_device:
+	put_device(&smaster->dev);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(siox_master_register);
-- 
2.43.0

Re: [PATCH] siox: fix master memory leak on registration failure

[Markus Elfring]

…
> +++ b/drivers/siox/siox-core.c
> @@ -753,14 +753,21 @@ int siox_master_register(struct siox_master *smaster)
>  	smaster->poll_thread = kthread_run(siox_poll_thread, smaster,
>  					   "siox-%d", smaster->busno);
>  	if (IS_ERR(smaster->poll_thread)) {
> +		ret = PTR_ERR(smaster->poll_thread);

Would it be nicer to use this variable assignment directly before
the goto statement?


>  		smaster->active = 0;
> -		return PTR_ERR(smaster->poll_thread);
> +		goto err_put_device;
>  	}
…


Regards,
Markus

Re: [PATCH] siox: fix master memory leak on registration failure

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]

On Mon, Sep 21, 2026 at 12:51:00PM +0200, Markus Elfring wrote:
> …
> > +++ b/drivers/siox/siox-core.c
> > @@ -753,14 +753,21 @@ int siox_master_register(struct siox_master *smaster)
> >  	smaster->poll_thread = kthread_run(siox_poll_thread, smaster,
> >  					   "siox-%d", smaster->busno);
> >  	if (IS_ERR(smaster->poll_thread)) {
> > +		ret = PTR_ERR(smaster->poll_thread);
> 
> Would it be nicer to use this variable assignment directly before
> the goto statement?

No, please don't, IMHO it's fine to have IS_ERR and PTR_ERR together.

But I wonder if it's sensible to create a function to all the usual init
stuff such that the error handling in siox_master_register() can become
just:

	get_device(&smaster->dev);

	ret = siox_master_init(...);
	if (ret)
		put_device();

	return ret;

Having said that, I wonder about the smaster->active = 0 assignment.
It's quite some time ago that I wrote that code, but either it's
useless (that's where my bet is on), or this assignment is missing in
the error path of device_add()? Thorsten?

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]