NetBSD Problem Report #40271

From mlelstv@pepew.1st.de  Thu Dec 25 23:28:36 2008
Return-Path: <mlelstv@pepew.1st.de>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 8A2AF63BC5B
	for <gnats-bugs@gnats.NetBSD.org>; Thu, 25 Dec 2008 23:28:36 +0000 (UTC)
Message-Id: <20081225232806.2B2BD2CBAA@pepew.1st.de>
Date: Fri, 26 Dec 2008 00:28:06 +0100 (CET)
From: mlelstv@serpens.de
Reply-To: mlelstv@serpens.de
To: gnats-bugs@gnats.NetBSD.org
Subject: irframe crashes with LOCKDEBUG
X-Send-Pr-Version: 3.95

>Number:         40271
>Category:       kern
>Synopsis:       irframe crashes with LOCKDEBUG
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Dec 25 23:30:00 +0000 2008
>Closed-Date:    Wed Apr 15 15:20:15 +0000 2009
>Last-Modified:  Wed Apr 15 15:20:15 +0000 2009
>Originator:     Michael van Elst
>Release:        NetBSD 5.0_BETA
>Organization:
-- 
                                Michael van Elst
Internet: mlelstv@serpens.de
                                "A potential Snark may lurk in every tree."
>Environment:


System: NetBSD pepew 5.0_BETA NetBSD 5.0_BETA (PEPEW) #5: Thu Dec 25 23:59:05 CET 2008 mlelstv@henery:/home/netbsd5/obj.i386/home/netbsd5/src/sys/arch/i386/compile/PEPEW i386
Architecture: i386
Machine: i386
>Description:
When running a netbsd-5 (or -current) kernel with LOCKDEBUG then
irdaattach crashes the kernel when it exits because a memory region
freed mit kmem_free contains an active lock.

The memory region in question is the irframet_softc structure
which contains the mutex sc_wr_lk. That mutex is initialized
by never destroyed before the structure is deallocated.

>How-To-Repeat:

>Fix:

The following patch moves/adds several init/destroy functions into
the attach/detach functions and also protects the irframe device
against an already detached line discipline.


Index: sys/dev/ir/irframe_tty.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ir/irframe_tty.c,v
retrieving revision 1.54
diff -u -r1.54 irframe_tty.c
--- sys/dev/ir/irframe_tty.c	25 May 2008 19:22:21 -0000	1.54
+++ sys/dev/ir/irframe_tty.c	25 Dec 2008 23:05:30 -0000
@@ -218,9 +218,16 @@
 static void
 irframet_attach(device_t parent, device_t self, void *aux)
 {
+	struct irframet_softc *sc = device_private(self);

 	/* pseudo-device attachment does not print name */
 	aprint_normal("%s", device_xname(self));
+
+	callout_init(&sc->sc_timeout, 0);
+	mutex_init(&sc->sc_wr_lk, MUTEX_DEFAULT, IPL_NONE);
+	selinit(&sc->sc_rsel);
+	selinit(&sc->sc_wsel);
+	
 #if 0 /* XXX can't do it yet because pseudo-devices don't get aux */
 	struct ir_attach_args ia;

@@ -234,8 +241,19 @@
 static int
 irframet_detach(struct device *dev, int flags)
 {
+	struct irframet_softc *sc = device_private(dev);
+	int rc;
+
+	callout_stop(&sc->sc_timeout);
+
+	rc = irframe_detach(dev, flags);
+
+	callout_destroy(&sc->sc_timeout);
+	mutex_destroy(&sc->sc_wr_lk);
+	seldestroy(&sc->sc_wsel);
+	seldestroy(&sc->sc_rsel);

-	return (irframe_detach(dev, flags));
+	return rc;
 }

 /*
@@ -529,7 +547,6 @@
 {
 	struct tty *tp = h;
 	struct irframet_softc *sc = (struct irframet_softc *)tp->t_sc;
-	static bool again;

 	DPRINTF(("%s: tp=%p\n", __func__, tp));

@@ -541,15 +558,6 @@
 	sc->sc_framei = 0;
 	sc->sc_frameo = 0;

-	/* XXX */
-	if (!again) {
-		again = true;
-		callout_init(&sc->sc_timeout, 0);
-		mutex_init(&sc->sc_wr_lk, MUTEX_DEFAULT, IPL_NONE);
-		selinit(&sc->sc_rsel);
-		selinit(&sc->sc_wsel);
-	}
-
 	return (0);
 }

@@ -563,6 +571,10 @@

 	DPRINTF(("%s: tp=%p\n", __func__, tp));

+	/* XXX how can the line discipline be closed? */
+	if (sc == NULL || tp != (struct tty *)sc->sc_tp)
+		return (0);
+
 	callout_stop(&sc->sc_timeout);
 	s = splir();
 	if (sc->sc_inbuf != NULL) {

>Release-Note:

>Audit-Trail:
From: Michael van Elst <mlelstv@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/40271 CVS commit: src/sys/dev/ir
Date: Sun, 11 Jan 2009 14:28:13 +0000 (UTC)

 Module Name:	src
 Committed By:	mlelstv
 Date:		Sun Jan 11 14:28:13 UTC 2009

 Modified Files:
 	src/sys/dev/ir: irframe_tty.c

 Log Message:
 Avoid crash under LOCKDEBUG by destroying a mutex correctly.
 Also: move mutex init/destroy into attach/detach functions and
 handle case where line discipline is closed before the irframe
 device. Fixes PR kern/40271.


 To generate a diff of this commit:
 cvs rdiff -r1.55 -r1.56 src/sys/dev/ir/irframe_tty.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sun, 05 Apr 2009 21:32:43 +0000
State-Changed-Why:
Is this fixed? (Did it get pulled up to -5?)


State-Changed-From-To: feedback->pending-pullups
State-Changed-By: mlelstv@NetBSD.org
State-Changed-When: Fri, 10 Apr 2009 10:48:44 +0000
State-Changed-Why:
pullup to netbsd-5 requested
.


From: Soren Jacobsen <snj@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/40271 CVS commit: [netbsd-5] src/sys/dev/ir
Date: Fri, 10 Apr 2009 18:12:53 +0000

 Module Name:	src
 Committed By:	snj
 Date:		Fri Apr 10 18:12:53 UTC 2009

 Modified Files:
 	src/sys/dev/ir [netbsd-5]: irframe_tty.c

 Log Message:
 Pull up following revision(s) (requested by mlelstv in ticket #684):
 	sys/dev/ir/irframe_tty.c: revision 1.56
 Avoid crash under LOCKDEBUG by destroying a mutex correctly.
 Also: move mutex init/destroy into attach/detach functions and
 handle case where line discipline is closed before the irframe
 device. Fixes PR kern/40271.


 To generate a diff of this commit:
 cvs rdiff -u -r1.54 -r1.54.8.1 src/sys/dev/ir/irframe_tty.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Wed, 15 Apr 2009 15:20:15 +0000
State-Changed-Why:
pullup to netbsd-5 completed
(thanks)


>Unformatted:

Home	PR Database Search