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]

Date:	Mon, 30 Nov 2015 19:01:37 +0100
From:	Paul Bolle <pebolle@...cali.nl>
To:	Tilman Schmidt <tilman@...p.cc>,
	Peter Hurley <peter@...leysoftware.com>,
	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 ma, 2015-11-30 at 00:23 +0100, Paul Bolle wrote:
> Relevant part of dmesg attached at the end of this message. This
> should give me (and Tilman too?) an entry to get to bottom of this. 
> Since this is relevant for anyone with just the ser-gigaset module 
> installed, I hope to do that soon.

I'm planning to send something similar to the attached draft to netdev
in a few days. It fixes the issue on my machine. Sascha, does it fix
this issue for syzkaller too? 

Should (something like) this go into stable too?

Any further comments on that draft are appreciated too, of course.


Paul Bolle
------
[DRAFT] gigaset: don't free() a struct platform_device

One is not supposed to free() a struct platform_device. Instead one
should, in the common case, only call platform_device_unregister(). That
will drop the platform device's reference count. (Actually it's the
reference count of the embedded kobject that is important here. But for
users of platform devices that's basically irrelevant.)

So move struct platform_device dev out of struct ser_cardstate, because
ser_cardstate is (malloc'ed and) free'd.

Reported-by: Sasha Levin <sasha.levin@...cle.com>
Not-yet-signed-off-by: Paul Bolle <pebolle@...cali.nl>
---
 drivers/isdn/gigaset/ser-gigaset.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/isdn/gigaset/ser-gigaset.c b/drivers/isdn/gigaset/ser-gigaset.c
index 375be509e95f..f8ffa253496e 100644
--- 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);
 	cs->hw.ser = NULL;
 }
@@ -401,17 +402,17 @@ static int gigaset_initcshw(struct cardstate *cs)
 	}
 	cs->hw.ser = scs;
 
-	cs->hw.ser->dev.name = GIGASET_MODULENAME;
-	cs->hw.ser->dev.id = cs->minor_index;
-	cs->hw.ser->dev.dev.release = gigaset_device_release;
-	rc = platform_device_register(&cs->hw.ser->dev);
+	pdev.name = GIGASET_MODULENAME;
+	pdev.id = cs->minor_index;
+	pdev.dev.release = gigaset_device_release;
+	rc = platform_device_register(&pdev);
 	if (rc != 0) {
 		pr_err("error %d registering platform device\n", rc);
 		kfree(cs->hw.ser);
 		cs->hw.ser = NULL;
 		return rc;
 	}
-	dev_set_drvdata(&cs->hw.ser->dev.dev, cs);
+	dev_set_drvdata(&pdev.dev, cs);
 
 	tasklet_init(&cs->write_tasklet,
 		     gigaset_modem_fill, (unsigned long) cs);
@@ -520,7 +521,7 @@ gigaset_tty_open(struct tty_struct *tty)
 		goto error;
 	}
 
-	cs->dev = &cs->hw.ser->dev.dev;
+	cs->dev = &pdev.dev;
 	cs->hw.ser->tty = tty;
 	atomic_set(&cs->hw.ser->refcnt, 1);
 	init_completion(&cs->hw.ser->dead_cmp);
-- 
2.4.3
--
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