linux-kernel - Re: [PATCH 0/3] spi: core: Introduce devm_spi_alloc

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] [day] [month] [year] [list]

Message-ID: <20140202145427.GB3157@lukather>
Date:	Sun, 2 Feb 2014 15:54:27 +0100
From:	Maxime Ripard <maxime.ripard@...e-electrons.com>
To:	Mark Brown <broonie@...nel.org>
Cc:	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-spi@...r.kernel.org
Subject: Re: [PATCH 0/3] spi: core: Introduce devm_spi_alloc_master

Hi,

On Sat, Feb 01, 2014 at 05:38:41PM +0000, Mark Brown wrote:
> On Fri, Jan 31, 2014 at 02:31:11PM +0100, Maxime Ripard wrote:
> > On Fri, Jan 31, 2014 at 12:12:15PM +0000, Mark Brown wrote:
> 
> > > This seems confusing - the idea here is that if we've handed the
> > > device off to the managed function then the managed function
> > > deals with destroying it.  Note that spi_alloc_master() says
> > > that the put is only required after errors adding the device
> > > (which would be the expected behaviour if you look at other
> > > APIs).  Looking at the code I think there is an issue here but
> > > I'm not at all clear that this is the best fix.
> 
> > Ah, right, spi_master_put doesn't free the memory either...
> 
> The memory is freed by the driver core calling the
> spi_master_release() callback when it's safe to do so (after the
> last reference to the device has gone away).
> 
> > I guess we have a few choices here, either:
> >   - Add a devm_kzalloc to spi_alloc_master, since most of the drivers
> >     I've been looking at fail to free the memory, this would be the
> >     least intrusive solution. We'd still have to remove all the kfree
> >     calls in the driver that rightfully free the memory.
> >   - Make devm_unregister_master also call kfree on the master
> >   - Add a kfree to my devm_put_master so that the memory is reclaimed,
> >     which isn't the case for now.
> 
> > I don't have a strong preference here, maybe for the third one, since
> > it makes obvious that it's managed and you don't have to do anything
> > about it, while the other do not.
> 
> None of the above, definitely nothing to do with calling kfree() once
> the device is registered.  We already have that free, it's not the
> issue.  The issue is making sure that we hold references when we need
> them and drop them when we don't.  I (or someone) needs to sit down and
> think it through since things are a bit confused in the code.

Ok. I'll drop the patches and repost the A31 SPI patches.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)