linux-kernel - Re: Fwd: Re: unbind/bind w1-gpio with device tree produce a crash

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

Message-ID: <20150305073524.GC8062@pengutronix.de>
Date:	Thu, 5 Mar 2015 08:35:24 +0100
From:	Markus Pargmann <mpa@...gutronix.de>
To:	Ingo Flaschberger <ingo.flaschberger@...il.com>
Cc:	gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org
Subject: Re: Fwd: Re: unbind/bind w1-gpio with device tree produce a crash

Hi Ingo,

On Thu, Mar 05, 2015 at 03:56:20AM +0100, Ingo Flaschberger wrote:
> Dear Markus,
> 
> as I get no answer at the kernel mailinglist:
> your patch to use devm_kzalloc instead of kzalloc produce driver crashes
> when the 1wire bus is unbound and bound again (see details below).
> 
> What do you suggest to solve this problem?

Good point. The easy way to solve this may be to check in the remove
function if the devicetree node for this device is available. If it is,
we know that platform_data was allocated with devm_* and we can set
platform_data to NULL.

I think the better solution would be to create a private structure with
the same data as w1_gpio_platform_data. It may even use the same struct.
But it should not be stored in platform_data. Instead it should be
handled seperately. For DT we can allocate it using devm_kzalloc(). And
for probing with pdata, this could just be the pointer to the pdata
within the device struct.

Adding the list as CC again.

Best Regards,

Markus

> 
> Kind regards,
>     Ingo Flaschberger
> 
> -------- Weitergeleitete Nachricht --------
> Betreff: 	Re: unbind/bind w1-gpio with device tree produce a crash
> Datum: 	Wed, 04 Mar 2015 17:38:48 +0100
> Von: 	Ingo Flaschberger <ingo.flaschberger@...il.com>
> An: 	linux-kernel@...r.kernel.org
> 
> 
> 
> is it ok, to create a new global variable that tracks if pdata was
> alloced via devm_kzalloc and sets pdata to NULL in w1_gpio_remove?
> 
> Am 04.03.2015 um 04:53 schrieb Ingo Flaschberger:
> >If w1-gpio is probed via device-tree configuration, pdata is allocated
> >via devm_kzalloc.
> >When the device is unbind (and bind later) the allocated memory of
> >pdate is freed - but it will not be allocacted again.
> >
> >static int w1_gpio_probe(struct platform_device *pdev)
> >        struct w1_bus_master *master;
> >        struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
> >        int err;
> >
> >        if(pdata == NULL) {
> >                if (of_have_populated_dt()) {
> >                        err = w1_gpio_probe_dt(pdev);
> >                        if (err < 0)
> >                                return err;
> >                }
> >        }
> >
> >How to detect if pdata was allocated via device-tree devm_kzalloc and
> >not "hardcoded"?
> >Then I could set pdev->dev.platform_data to NULL in w1_gpio_remove
> >which will solve the crash.
> >
> >Kind regards,
> >   Ingo Flascherger
> >
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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