Re: [PATCH] efi: fix boundary checking in efi_high_alloc()

[Matt Fleming]

On Thu, 19 Feb, at 08:18:03PM, Yinghai Lu wrote:
> While adding support loading kernel and initrd above 4G to grub2 in legacy
> mode, I was referring to efi_high_alloc().
> That will allocate buffer for kernel and then initrd, and initrd will
> use kernel buffer start as limit.
> 
> During testing found two buffers will be overlapped when initrd size is
> very big like 400M.
> 
> It turns out efi_high_alloc() boundary checking is not right.
> end - size will be the new start, and should not compare new
> start with max, we need to make sure end is smaller than max.
> 
> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
> 
> ---
>  drivers/firmware/efi/libstub/efi-stub-helper.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

The patch looks fine but this changelog could do with some finessing
because I spent 30 minutes trying to understand what the bug was, and
more specifically, what values to feed into the allocation algorithm to
trigger it.

Basically, with the current efi_high_alloc() code it's possible to
allocate memory above 'max', because efi_high_alloc() doesn't check that
the tail of the allocation is below 'max'.

If you have an EFI memory map with a single entry that looks like so,

 [0xc0000000-0xc0004000]

And want to allocate 0x3000 bytes below 0xc0003000 the current code will
allocate [0xc0001000-0xc0004000], not [0xc0000000-0xc0003000] like you
would expect.

I've queued up the below patch. If the Linaro/ARM gang could test it out
I'd really appreciate it.

---