Re: [PULL] param sysfs oops (simple, leaky) fix, bool arrays fix

[Takashi Iwai <tiwai@suse.de>]

At Wed, 05 May 2010 10:25:14 +0300,
Artem Bityutskiy wrote:
> 
> On Wed, 2010-05-05 at 15:03 +0930, Rusty Russell wrote:
> > On Wed, 5 May 2010 03:37:19 am Artem Bityutskiy wrote:
> > > On Tue, 2010-05-04 at 11:53 +0930, Rusty Russell wrote:
> > > > On Tue, 27 Apr 2010 08:23:24 pm Artem Bityutskiy wrote:
> > > > > Rusty, correct me if I'm wrong, but it looks like the above memleak was
> > > > > introduced by e180a6b7759a99a28cbcce3547c4c80822cb6c2a, where you added
> > > > > the kstrdup(). So you kinda fixed the sysfs case (it still memleaks
> > > > > though), but at the cost of additional insmod/rmmod leak, right?
> > > > 
> > > > Yep!
> > > 
> > > Are you working/planning to work on fixing this regression?
> > 
> > I'm still ambivalent on it; I have patches but it's a lot of churn for not
> > much gain.
> > 
> > To fix this, we need a way to lock parameters against changing by sysfs, and
> > we need to use it everywhere.  Past experience has demonstrated that this will
> > never be maintained.  
> > 
> > On the other hand, the leak is trivial.
> 
> Well, I am not very concerned with the "changing by sysfs" leak. This is
> not a big deal, IMHO. I am concerned with the "rmmod" leak, which did
> not exist before your patches, but exists now. People may do a lot of
> insmod/rmmod, and on each rmmod they will loose this kstrdup-ed string. 

I don't think there are so many real cases that actually do leaking.
This is only for charp type parameters (not string), and no leak
happens unless user gives the value explicitly via a module option.

Fixing in the way of the later upstream is a bit too intrusive as a
stable patch.  So, I'm also not sure whether we should take it,
too...


thanks,

Takashi