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  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]
Date:	Sat, 1 Feb 2014 17:38:41 +0000
From:	Mark Brown <broonie@...nel.org>
To:	Maxime Ripard <maxime.ripard@...e-electrons.com>
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

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.

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

Powered by blists - more mailing lists