Subject: use-after-free bug in acpi_ec.c?
To: None <current-users@NetBSD.org>
From: Tobias Nygren <tnn@NetBSD.org>
List: current-users
Date: 12/03/2007 01:28:13
This is a multi-part message in MIME format.

--Multipart=_Mon__3_Dec_2007_01_28_13_+0100_w+Aac/rXJ4QLq_Lv
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

Hi all,

Lately my laptop has been panicing during bootup. The problem appears
to be located in acpi_ec.c.

What happens is that an "early" acpi_ec softc is allocated in
acpiec_early_attach. Then some GPE queries are scheduled with a pointer
to this early softc as the argument. Later the real attach routine for
acpi_ec free()s the early softc and sets up the real softc. But the
initial callbacks haven't been run yet. When they do they will attempt
to use a softc that was already free'd. The attached patch should
illustrate the problem. When I have it applied I see several warning
messages before the system goes multiuser. No more crashes though.

Am I the only one seeing this? If you apply the patch and see the
messages I guess it's just pure luck that it doesn't crash for most
people ...

-Tobias

--Multipart=_Mon__3_Dec_2007_01_28_13_+0100_w+Aac/rXJ4QLq_Lv
Content-Type: text/plain;
 name="acpi_ec.c.diff.txt"
Content-Disposition: attachment;
 filename="acpi_ec.c.diff.txt"
Content-Transfer-Encoding: 7bit

Index: sys/dev/acpi/acpi_ec.c
===================================================================
RCS file: /cvsroot/src/sys/dev/acpi/acpi_ec.c,v
retrieving revision 1.42
diff -u -r1.42 acpi_ec.c
--- sys/dev/acpi/acpi_ec.c	19 Oct 2007 11:59:34 -0000	1.42
+++ sys/dev/acpi/acpi_ec.c	3 Dec 2007 00:20:23 -0000
@@ -269,6 +269,7 @@
     acpiec_match, acpiec_attach, NULL, NULL);
 
 static struct acpi_ec_softc *ecdt_sc;
+static struct acpi_ec_softc *old_ec_sc;
 
 static inline int
 EcIsLocked(struct acpi_ec_softc *sc)
@@ -531,6 +532,7 @@
 		    ecdt_sc->sc_data_sh, 1);
 
 		free(ecdt_sc, M_ACPI);
+		old_ec_sc = ecdt_sc;
 		ecdt_sc = NULL;
 	}
 
@@ -648,6 +650,10 @@
 	ACPI_STATUS rv;
 	char qxx[5];
 
+	if (sc == old_ec_sc) {
+		printf("WARNING: This context is not valid anymore\n");
+		return;
+	}
 	ACPI_FUNCTION_TRACE(__FUNCTION__);
 
 	for (;;) {

--Multipart=_Mon__3_Dec_2007_01_28_13_+0100_w+Aac/rXJ4QLq_Lv--