>>> On 09.08.16 at 15:51, <jslaby@xxxxxxx> wrote:
> On 08/09/2016, 03:39 PM, 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.
>
> And what would be map_end in case desc_size is zero so that the test fails?
Zero.
> Anyway, if I understand correctly, you subtract one from the pointer.
> Note that behaviour of this is undefined according to the C standard and
> may result in unpredictable results. Pointer can only point inside an
> object (or array) or one element *past* the end, not before.
Considering an object spanning the entire address space, I'm not
sure there's anything really undefined here. But of course the whole
arithmetic could instead be done with unsigned longs (using respective
casts).
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
