Re: gigaset: freeing an active object

[Paul Bolle <pebolle@tiscali.nl>]

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@oracle.com>
Not-yet-signed-off-by: Paul Bolle <pebolle@tiscali.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