netdev - Re: gigaset: freeing an active object

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: <1449408690.2515.15.camel@tiscali.nl>
Date:	Sun, 06 Dec 2015 14:31:30 +0100
From:	Paul Bolle <pebolle@...cali.nl>
To:	Peter Hurley <peter@...leysoftware.com>,
	Tilman Schmidt <tilman@...p.cc>,
	Sasha Levin <sasha.levin@...cle.com>
Cc:	isdn@...ux-pingi.de, davem@...emloft.net,
	gigaset307x-common@...ts.sourceforge.net,
	LKML <linux-kernel@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	syzkaller <syzkaller@...glegroups.com>
Subject: Re: gigaset: freeing an active object

On wo, 2015-12-02 at 18:48 -0500, Peter Hurley wrote:
> On 11/30/2015 01:01 PM, Paul Bolle wrote:
> > Should (something like) this go into stable too?
> 
> Definitely for stable since it has a userspace triggerable component.

Thanks, will do.

> > --- a/drivers/isdn/gigaset/ser-gigaset.c
> > +++ b/drivers/isdn/gigaset/ser-gigaset.c
> > @@ -42,8 +42,9 @@ MODULE_PARM_DESC(cidmode, "stay in CID mode when
> > idle");
> >  
> >  static struct gigaset_driver *driver;
> >  
> > +static struct platform_device pdev;
> > +
> >  struct ser_cardstate {
> > -	struct platform_device	dev;
> >  	struct tty_struct	*tty;
> >  	atomic_t		refcnt;
> >  	struct completion	dead_cmp;
> > @@ -370,8 +371,8 @@ static void gigaset_freecshw(struct cardstate
> > *cs)
> >  	tasklet_kill(&cs->write_tasklet);
> >  	if (!cs->hw.ser)
> >  		return;
> > -	dev_set_drvdata(&cs->hw.ser->dev.dev, NULL);
> > -	platform_device_unregister(&cs->hw.ser->dev);
> > +	dev_set_drvdata(&pdev.dev, NULL);
> > +	platform_device_unregister(&pdev);
> >  	kfree(cs->hw.ser);
> 
> Tilman,
> 
> Is there a 1:1 correspondence and lifetime for the embedded platform
> device and it's containing memory?

(Haven't heard from Tilman, so I'll give this a try.)

That containing memory is a struct ser_cardstate. And currently
instances of struct _ser_cardstate are malloced and freed in routines
that also call platform_device_register() and
platform_device_unregister(). So yes, I think there's a 1:1
correspondence.

> I ask because the typical approach for device teardown is to put the
> kfree() in the release method;

(Side note: the (struct device) release method of this driver 
-gigaset_device_release() - is actually a nop. It only frees device
->platform_data and platform_device->resource, but neither are actually
used: they remain NULL through their entire life.)

>  naturally, that won't work if there
> is some other lifetime issue.

I'm not sure I follow what you mean here. Could you point me at a driver
that uses that approach, so that I can have a look at it?

Thanks,


Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html