diff mbox

[U-Boot] efi_loader: fix bug in efi_get_memory_map

Message ID 20170726183405.27841-1-robdclark@gmail.com
State Accepted
Commit a1b24823b6c22ede11d311b05ea862965ea86b0b
Delegated to: Alexander Graf
Headers show

Commit Message

Rob Clark July 26, 2017, 6:34 p.m. UTC
When booting shim -> fallback -> shim -> grub -> linux the memory map is
a bit larger than the size linux passes in on the first call.  But in
the EFI_BUFFER_TOO_SMALL case we were not passing back the updated size
to linux so it would loop forever.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 lib/efi_loader/efi_memory.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Alexander Graf July 26, 2017, 8:10 p.m. UTC | #1
On 26.07.17 20:34, Rob Clark wrote:
> When booting shim -> fallback -> shim -> grub -> linux the memory map is
> a bit larger than the size linux passes in on the first call.  But in
> the EFI_BUFFER_TOO_SMALL case we were not passing back the updated size
> to linux so it would loop forever.
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>

The spec is actually very explicit about this case. It says in the 
EFI_BUFFER_TOO_SMALL case, we *have* to return the map size.


Alex
Rob Clark July 26, 2017, 8:25 p.m. UTC | #2
On Wed, Jul 26, 2017 at 4:10 PM, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 26.07.17 20:34, Rob Clark wrote:
>>
>> When booting shim -> fallback -> shim -> grub -> linux the memory map is
>> a bit larger than the size linux passes in on the first call.  But in
>> the EFI_BUFFER_TOO_SMALL case we were not passing back the updated size
>> to linux so it would loop forever.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>
>
> The spec is actually very explicit about this case. It says in the
> EFI_BUFFER_TOO_SMALL case, we *have* to return the map size.
>

yes, that is what I fixed.  We *weren't* returning the required buffer
size before :-)

BR,
-R
Stefan Brüns July 27, 2017, 2:44 p.m. UTC | #3
On Mittwoch, 26. Juli 2017 22:25:29 CEST Rob Clark wrote:
> On Wed, Jul 26, 2017 at 4:10 PM, Alexander Graf <agraf@suse.de> wrote:
> > On 26.07.17 20:34, Rob Clark wrote:
> >> When booting shim -> fallback -> shim -> grub -> linux the memory map is
> >> a bit larger than the size linux passes in on the first call.  But in
> >> the EFI_BUFFER_TOO_SMALL case we were not passing back the updated size
> >> to linux so it would loop forever.
> >> 
> >> Signed-off-by: Rob Clark <robdclark@gmail.com>
> > 
> > The spec is actually very explicit about this case. It says in the
> > EFI_BUFFER_TOO_SMALL case, we *have* to return the map size.
> 
> yes, that is what I fixed.  We *weren't* returning the required buffer
> size before :-)

Sigh, yes, this was correct in the first 3 versions of the patch series, but 
unfortunately broken in v4 which was actually committed ...

See: https://lists.denx.de/pipermail/u-boot/2016-October/268766.html

Actually, the map_size variable is no longer needed, if you assign to 
*memory_map_size directly.

Anyway, this patch is:
Reviewed-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>

Kind regards,

Stefan
Alexander Graf July 28, 2017, 10:27 p.m. UTC | #4
> When booting shim -> fallback -> shim -> grub -> linux the memory map is
> a bit larger than the size linux passes in on the first call.  But in
> the EFI_BUFFER_TOO_SMALL case we were not passing back the updated size
> to linux so it would loop forever.
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>

Thanks, applied to efi-next

Alex
diff mbox

Patch

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index f59e3ea327..9e079f1fa3 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -407,11 +407,11 @@  efi_status_t efi_get_memory_map(unsigned long *memory_map_size,
 
 	map_size = map_entries * sizeof(struct efi_mem_desc);
 
+	*memory_map_size = map_size;
+
 	if (provided_map_size < map_size)
 		return EFI_BUFFER_TOO_SMALL;
 
-	*memory_map_size = map_size;
-
 	if (descriptor_size)
 		*descriptor_size = sizeof(struct efi_mem_desc);