Re: [Xen-devel] [PATCH] Increment buffer used to read first boot sector in order to accomodate space for 4k sector

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.08.12 at 16:50, Frediano Ziglio <frediano.ziglio@xxxxxxxxxx> wrote:
> If a 4k disk is used for first BIOS disk loader corrupt itself.

If such is really permitted by the specification (which I doubt it
is for the standard, old-style INT13 functions - it's a different
story for functions 42 and 43, where the caller can be expected
to call function 48 first).

> This patch increase sector buffer in order to avoid this overflow

And if we indeed need to adjust for this, then let's fix this properly:
Don't just increase the buffer size, but also check that the sector
size reported actually fits. That may require calling Fn48 first,
before doing the actual read.

> --- a/xen/arch/x86/boot/edd.S
> +++ b/xen/arch/x86/boot/edd.S
> @@ -154,4 +154,4 @@ boot_mbr_signature_nr:
>  boot_mbr_signature:
>          .fill   EDD_MBR_SIG_MAX*8,1,0
>  boot_edd_info:
> -        .fill   512,1,0                         # big enough for a disc 
> sector
> +        .fill   4096,1,0                         # big enough for a disc 
> sector

Also I wonder whether it wouldn't be more smart to re-use the
wakeup stack (which is already 4k in size), and shrink this buffer
to the maximum size ever used without reading sectors into it
(EDD_INFO_MAX*(EDDEXTSIZE+EDDPARMSIZE)).

> --- a/xen/arch/x86/boot/trampoline.S
> +++ b/xen/arch/x86/boot/trampoline.S
> @@ -224,6 +224,6 @@ skip_realmode:
>  rm_idt: .word   256*4-1, 0, 0
>  
>  #include "mem.S"
> -#include "edd.S"
>  #include "video.S"
>  #include "wakeup.S"
> +#include "edd.S"

Finally, you should also explain why this change is needed.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel