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.
--
Ard.
--
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
