This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [PATCH] e820: fix clip_to_limit()

To: Xiao Guangrong <ericxiao.gr@xxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] e820: fix clip_to_limit()
From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Date: Mon, 09 Nov 2009 20:46:13 +0000
Cc: Xiao Guangrong <xiaoguangrong@xxxxxxxxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 09 Nov 2009 12:46:47 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4AF875E9.4040807@xxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Acphd/GJ4gVMDSgpTHKZH9ixTsljngABbss+
Thread-topic: [PATCH] e820: fix clip_to_limit()
User-agent: Microsoft-Entourage/
I think the 'break' is in the wrong place. Actually also I think the case of
successful change_range_type() is also wrong, as i=0 will be skipped on the
next iteration of the loop.

Overall I decided that modifying the e820 map inside the iterator loop was
just bad and confusing, so I've rewritten it in response to your bug
discovery. Please take a look at xen-unstable:20419 and let me know if you
see any issues.


On 09/11/2009 20:04, "Xiao Guangrong" <ericxiao.gr@xxxxxxxxx> wrote:

> In clip_to_limit(), after memmove(&e820.map[i], &e820.map[i+1], ...), the
> original
> e820.map[i+1] become current e820.map[i] but the next loop count is i+1, so
> the original
> e820.map[i+1] will be skipped
> Actually, e820 is sorted form low to high by sanitize_e820_map(), so we can
> simply break
> the loop if we meet the item which overrun "limit"
> Signed-off-by: Xiao Guangrong <ericxiao.gr@xxxxxxxxx>
> diff -r 93bc06dd1161 -r 5e06f2790d93 xen/arch/x86/e820.c
> --- a/xen/arch/x86/e820.c Tue Nov 10 02:41:59 2009 +0800
> +++ b/xen/arch/x86/e820.c Tue Nov 10 03:51:08 2009 +0800
> @@ -389,6 +389,7 @@
>                       (e820.nr_map - i - 1) * sizeof(struct e820entry));
>               e820.nr_map--;
>           }
> + break;
>       }
>       if ( old_limit )

Xen-devel mailing list