>>> On 09.08.16 at 15:49, <mark.rutland@xxxxxxx> wrote:
> On Tue, Aug 09, 2016 at 07:39:58AM -0600, Jan Beulich wrote:
>> >>> On 09.08.16 at 15:03, <jslaby@xxxxxxx> wrote:
>> > On 08/09/2016, 12:16 PM, Jan Beulich wrote:
>> >> While commit 55f1ea15216 ("efi: Fix for_each_efi_memory_desc_in_map()
>> >> for empty memmaps") made an attempt to deal with empty memory maps, it
>> >> didn't address the case where the desc_size field never gets set, as is
>> >> apparently the case when running under Xen.
>> >>
>> >> Reported-by: <lists@xxxxxxxxxxxx>
>> >> Cc: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
>> >> Cc: Jiri Slaby <jslaby@xxxxxxx>
>> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> >> Tested-by: <lists@xxxxxxxxxxxx>
>> >> ---
>> >> include/linux/efi.h | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> --- 4.8-rc1/include/linux/efi.h
>> >> +++ 4.8-rc1-EFI-memdesc-iterator-Xen/include/linux/efi.h
>> >> @@ -946,7 +946,7 @@ extern int efi_memattr_apply_permissions
>> >> /* Iterate through an efi_memory_map */
>> >> #define for_each_efi_memory_desc_in_map(m, md) \
>> >> for ((md) = (m)->map; \
>> >> - ((void *)(md) + (m)->desc_size) <= (m)->map_end; \
>> >> + ((void *)(md) + (m)->desc_size - 1) < (m)->map_end; \
>> >
>> > Is there any specific reason you change both the size and the comparator?
>> >
>> > IMO, either (readable)
>> > ((void *)(md) + (m)->desc_size) < (m)->map_end;
>> > or (mindfuck version)
>> > ((void *)(md) + (m)->desc_size - 1) <= (m)->map_end;
>> > is correct, not their mix.
>>
>> We're not talking about an off-by-one getting fixed here: map_end
>> points past the valid range. The adjustment leverages overflow (or
>> underflow, to be precise) to produce correct behavior when
>> (m)->desc_size is zero.
>
> That deserves a comment in the code, given the Xen case is not
> particularly obvious, and it's caught out at least one reviewer.
>
> What is map_end initialised to in the Xen case? Is that Xen with an
> empty map, of Xen generally?
All fields are zero under Xen, as the raw EFI memory map doesn't
get exposed to the Dom0 kernel.
Jan
--
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
