Subject: extent(9) bug
To: None <tech-kern@netbsd.org>
From: Manuel Bouyer <bouyer@antioche.lip6.fr>
List: tech-kern
Date: 03/14/2005 19:22:16
--HcAYCG3uE/tztfnV
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi
i found a bug in the extent manager. If we have something like that:
extent `test11' (0x10 - 0x20), flags = EX_NOCOALESCE
  0x10 - 0x13
  0x1e - 0x1f
  0x20 - 0x20

Then we could get an allocation of size 1 at 0x20 (either by restricting the
region, or because there are no free region of size 1). This is because of
the change in rev 1.43 of subr_extent.c: if there is an entry starting at
the end of the available region, it would be ignored in the
for (; rp != NULL; rp = LIST_NEXT(rp, er_link)) loop.
It should be safe to change the >= to > here, but I'd prefer to have a second
pair eyes confirm this.

Note: the second change is in a DIAGNOSTIC check. I think this one was to
check that the freed region is inside the map, so we need to check that we
really have (ex_start < start && end < ex_end).

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

--HcAYCG3uE/tztfnV
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff

? regress/sys/kern/extent/.gdbinit
? regress/sys/kern/extent/extest
? regress/sys/kern/extent/extest.c
? regress/sys/kern/extent/extest.out
Index: sys/kern/subr_extent.c
===================================================================
RCS file: /cvsroot/src/sys/kern/subr_extent.c,v
retrieving revision 1.50
diff -u -r1.50 subr_extent.c
--- sys/kern/subr_extent.c	23 Mar 2004 13:22:33 -0000	1.50
+++ sys/kern/subr_extent.c	14 Mar 2005 18:17:33 -0000
@@ -677,7 +677,7 @@
 		 * If the region pasts the subend, bail out and see
 		 * if we fit against the subend.
 		 */
-		if (rp->er_start >= subend) {
+		if (rp->er_start > subend) {
 			exend = rp->er_start;
 			break;
 		}
@@ -923,7 +923,7 @@
 	 */
 	if (ex == NULL)
 		panic("extent_free: NULL extent");
-	if ((start < ex->ex_start) || (start > ex->ex_end)) {
+	if ((start < ex->ex_start) || (end > ex->ex_end)) {
 		extent_print(ex);
 		printf("extent_free: extent `%s', start 0x%lx, size 0x%lx\n",
 		    ex->ex_name, start, size);
Index: regress/sys/kern/extent/extest.exp
===================================================================
RCS file: /cvsroot/src/regress/sys/kern/extent/extest.exp,v
retrieving revision 1.8
diff -u -r1.8 extest.exp
--- regress/sys/kern/extent/extest.exp	21 Feb 2002 03:59:25 -0000	1.8
+++ regress/sys/kern/extent/extest.exp	14 Mar 2005 18:17:33 -0000
@@ -63,3 +63,14 @@
 extent `test10' (0xc0002000 - 0xffffe000), flags = 0x0
      0xc0010000 - 0xc0011fff
      0xc0022000 - 0xc0031fff
+output for test11
+result: 0x10
+result: 0x1e
+result: 0x20
+error: Resource temporarily unavailable
+result: 0x14
+extent `test11' (0x10 - 0x20), flags = 0x2
+     0x10 - 0x13
+     0x14 - 0x14
+     0x1e - 0x1f
+     0x20 - 0x20
Index: regress/sys/kern/extent/tests
===================================================================
RCS file: /cvsroot/src/regress/sys/kern/extent/tests,v
retrieving revision 1.8
diff -u -r1.8 tests
--- regress/sys/kern/extent/tests	21 Feb 2002 03:59:25 -0000	1.8
+++ regress/sys/kern/extent/tests	14 Mar 2005 18:17:33 -0000
@@ -84,3 +84,16 @@
 print
 alloc_subregion 0xc0002000 0xffffe000 0x10000
 print
+#If we have something like that in the EX_NOCOALESCE case:
+#extent `test11' (0x10 - 0x20), flags = 0x2
+#  0x10 - 0x13
+#  0x1e - 0x1f
+#  0x20 - 0x20
+#then a new extent of size 1 could be allocated at 0x20.
+extent test11 0x10 0x20 EX_NOCOALESCE
+alloc_subregion 0x10 0x13 0x4
+alloc_subregion 0x1e 0x1f 0x2
+alloc_subregion 0x20 0x20 0x1
+alloc_subregion 0x20 0x20 0x1
+alloc_subregion 0x10 0x20 0x1
+print

--HcAYCG3uE/tztfnV--