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--