mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Thorsten Scherer <T.Scherer@eckelmann.de>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Markus Elfring <Markus.Elfring@web.de>,
	 Guangshuo Li <lgs201920130244@gmail.com>,
	kernel@pengutronix.de, stable@vger.kernel.org,
	 LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] siox: fix master memory leak on registration failure
Date: Thu, 24 Sep 2026 15:05:33 +0200	[thread overview]
Message-ID: <arUeFAzLtzsIQl4G@NB822> (raw)
In-Reply-To: <arEpuhA24xUlA-vw@monoceros>

Hello everyone,

thank you for the patch and the input.

On Mon, Sep 21, 2026 at 03:02:01PM +0200, Uwe Kleine-König wrote:
> 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?

At a first glance I'm with you.  Assignment doesn't change anything.
But I don't have a clear view (on the whole context) yet.  The patch
seems legit and fixes a real issue.  Having said that,  it seems that
there is an additional imbalance at siox_poll_thread.

I will have to take a little time to swap siox in and get a clearer picture.

Coming back to you soon.

> Best regards
> Uwe

Best regards
Thorsten

      reply	other threads:[~2026-09-24 13:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-19 18:10 Guangshuo Li
2026-09-21 10:51 ` Markus Elfring
2026-09-21 13:02   ` Uwe Kleine-König
2026-09-24 13:05     ` Thorsten Scherer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=arUeFAzLtzsIQl4G@NB822 \
    --to=t.scherer@eckelmann.de \
    --cc=Markus.Elfring@web.de \
    --cc=kernel@pengutronix.de \
    --cc=lgs201920130244@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®