Re: [PATCH 1/4] efi/libstub: Allocate headspace in efi_get_memory_map()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/18/2016 9:14 AM, Ard Biesheuvel wrote:
On 18 July 2016 at 17:11, Mark Rutland <mark.rutland@xxxxxxx> wrote:
On Mon, Jul 18, 2016 at 09:03:31AM -0600, Jeffrey Hugo wrote:
On 7/18/2016 5:00 AM, Mark Rutland wrote:
Hi,

On Sun, Jul 17, 2016 at 02:45:59PM -0600, Jeffrey Hugo wrote:
+   if (status == EFI_BUFFER_TOO_SMALL ||
+                           (*buff_size - *map_size) / sizeof(*m) < 8) {
           efi_call_early(free_pool, m);
+           /*
+            * Make sure there is 8 entries worth of headroom so that the
+            * buffer can be reused for a new map after allocations are
+            * no longer permitted.  Its unlikely that the map will grow to
+            * exceed this headroom once we are ready to trigger
+            * ExitBootServices()
+            */
+           *map_size += sizeof(*m) * 8;

As a general note, it's a shame that we don't know desc_size first time
through. If descs grow, 8 * sizeof(*m) might be too small. That's an
existing problem though, so I guess it makes sense to be consistent and
use sizeof(*m) here.

Yep.  Unless I've missed something, it seems like the two options
are either make a decent attempt to guess at desc_size the first
time (current approach), or intentionally fail the first time just
to get the size.  The current approach doesn't seem perfect, but it
does have a chance at passing with just one attempt.  I couldn't see
a better solution, but if someone else has an idea, I'd like to hear
it.

Sure, that was more of an observation than a critique. In the absence of
any suggestions better, sticking with sizeof(*m) makes sense.

If it does become a problem, we're likely to have to fix up other sites
regardless.


It is worth noting that the EDK2 implementation deliberately uses a
desc_size that exceeds the size of the type. So '8 * sizeof(*m)' is
almost never going to be 8 slots' worth of headroom.


Good point, desc_size does exceed sizeof(*m) when I test an EDK2 implementation. I think I can rework things to use desc_size instead, so that we do get the expected 8 slots of headroom. I'll incorporate that in V2.

--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux