FYI, I commited my change.

On Mon, Mar 14, 2005 at 07:22:16PM +0100, Manuel Bouyer wrote:
> 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
> --

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

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