On Tue, 20 Jul 2021 at 13:14, Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx> wrote: > > The EFI stub random allocator used for kaslr on arm64 has a subtle > bug. In function get_entry_num_slots() which counts the number of > possible allocation "slots" for the image in a given chunk of free > EFI memory, "last_slot" can become negative if the chunk is smaller > than the requested allocation size. > > The test "if (first_slot > last_slot)" doesn't catch it because > both first_slot and last_slot are unsigned. > > I chose not to make them signed to avoid problems if this is ever > used on architectures where there are meaningful addresses with the > top bit set. Instead, fix it with an additional test against the > allocation size. > > This can cause a boot failure in addition to a loss of randomisation > due to another bug in the arm64 stub fixed separately. > > Signed-off-by: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx> > Fixes: 2ddbfc81eac8 (efi: stub: add implementation of efi_random_alloc()) Thanks for fixing this. Acked-by: Ard Biesheuvel <ardb@xxxxxxxxxx> > --- > > drivers/firmware/efi/libstub/randomalloc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/firmware/efi/libstub/randomalloc.c b/drivers/firmware/efi/libstub/randomalloc.c > index a408df474d83..724155b9e10d 100644 > --- a/drivers/firmware/efi/libstub/randomalloc.c > +++ b/drivers/firmware/efi/libstub/randomalloc.c > @@ -30,6 +30,8 @@ static unsigned long get_entry_num_slots(efi_memory_desc_t *md, > > region_end = min(md->phys_addr + md->num_pages * EFI_PAGE_SIZE - 1, > (u64)ULONG_MAX); > + if (region_end < size) > + return 0; > > first_slot = round_up(md->phys_addr, align); > last_slot = round_down(region_end - size + 1, align); > >
