linux-kernel - Re: [PATCH 25/25] leds: convert One-shot LED trigger driver to devm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1341520528.3076.3.camel@lorien2>
Date:	Thu, 05 Jul 2012 14:35:28 -0600
From:	Shuah Khan <shuahkhan@...il.com>
To:	Fabio Baltieri <fabio.baltieri@...il.com>
Cc:	shuahkhan@...il.com, Bryan Wu <bryan.wu@...onical.com>,
	linux-leds@...r.kernel.org, rpurdie@...ys.net,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 25/25] leds: convert One-shot LED trigger driver to
 devm_kzalloc() and cleanup error exit path

On Thu, 2012-07-05 at 21:48 +0200, Fabio Baltieri wrote:

> > >  
> > Bryan,
> > 
> > I don't believe memory triggers allocate in their activate routine
> > should be converted to devm_kzalloc(). Based on my understanding, the
> > memory allocated using devm_kzalloc() us free'ed when driver is
> > detached. In the case of led triggers, driver stays registered while the
> > triggers it supports can be activated and deactivated many times. By
> > converting these allocations into devm_kzalloc()s could lead to memory
> > leaks. Please correct me if my understanding is incorrect.
> 
> Shuah,
> 
> I'm as the same opinion - as triggers are not really "driver", in the
> sense that they are not using the driver structures, using devm_
> functions doesn't seems to be really appropriate.
> 

Correct. Each driver could support multiple triggers and triggers are
not at the same level as the device.

> Specifically:
> 
> > > +	oneshot_data = devm_kzalloc(led_cdev->dev, sizeof(*oneshot_data),
> > > +				GFP_KERNEL);
> 
> this one is registering the data on the relative LED's resources, so
> that they are not freed until you unload the LED's driver itself.
> I also think that this leads to memory leak if you keep activating
> triggers, as the deactivate function is not actually freeing the
> resources, so in that case the correct procedure should be the usual
> kalloc/kfree.
> 
> Is this consistent with what your conclusions, Shuah?

Correct. Using devm_ in triggers is not the correct and there is no need
to change from using kalloc()/kfree().

-- Shuah
> 
> Fabio
> 
> > If what I am saying makes sense, please take this as comment that is
> > applicable to all led triggers, not just this one.
> > 
> > Thanks,
> > -- Shuah
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-leds" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/