Re: [PATCH] Off-by-one in cpu_gdt_init

[David Hopwood <david.nospam.hopwood@blueyonder.co.uk>]

George Washington Dunlap III wrote:
> I forget what triggered this bug (it was a long time ago), but 
> cpu_gdt_init() is trying to allocate an array, one per frame, based on 
> gdt_descr->size.  However, the math currently rounds down instead of up! 
> (I'm pretty sure that when I triggered it, (gdt_descr->size>>PAGE_SHIFT) 
> was 0.)
> 
> diff -urN --exclude=SCCS --exclude=BitKeeper xen-unstable.latest/linux-2.6.11-xen-sparse/arch/xen/i386/kernel/cpu/common.c xeno-ft/linux-2.6.11-xen-sparse/arch/xen/i386/kernel/cpu/common.c
> --- xen-unstable.latest/linux-2.6.11-xen-sparse/arch/xen/i386/kernel/cpu/common.c	2005-05-16 13:05:03.000000000 -0400
> +++ xeno-ft/linux-2.6.11-xen-sparse/arch/xen/i386/kernel/cpu/common.c	2005-05-16 13:55:06.000000000 -0400
> @@ -554,7 +554,7 @@
>  
>  void __init cpu_gdt_init(struct Xgt_desc_struct *gdt_descr)
>  {
> -	unsigned long frames[gdt_descr->size >> PAGE_SHIFT];
> +	unsigned long frames[(gdt_descr->size >> PAGE_SHIFT)+1];

Variable-length arrays? Never use variable-length arrays in code that needs
to be robust: you can't guarantee that the stack won't overflow. If it does,
there is no way to detect that situtation (unlike malloc et al where you can
check for NULL), you just get undefined behaviour.

-- 
David Hopwood <david.nospam.hopwood@blueyonder.co.uk>