On Tue, 16 Aug, at 12:49:17PM, Matt Fleming wrote:
> On Mon, 15 Aug, at 09:05:45AM, 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>
> > ---
> > v2: Add comment.
> > ---
> > include/linux/efi.h | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > --- 4.8-rc2/include/linux/efi.h
> > +++ 4.8-rc2-EFI-memdesc-iterator-Xen/include/linux/efi.h
> > @@ -946,7 +946,11 @@ 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; \
> > + /* \
> > + * Leverage wrapping of the calculation to avoid an infinite \
> > + * loop when all three values are zero. \
> > + */ \
> > + ((void *)(md) + (m)->desc_size - 1) < (m)->map_end; \
> > (md) = (void *)(md) + (m)->desc_size)
> >
>
> Thanks Jan, applied to the 'urgent' EFI queue and tagged for stable.
> I'll send it out this week.
Jiri caught me on IRC and repeated his concern that this patch relies
on undefined behaviour per the C language standard.
I really don't want to get into a discussion of compiler
implementations, so instead can we check whether the 'md' is NULL and
skip the loop if so? e.g. would something like this work?
---
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 7f5a58225385..23cd3ced8c1a 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -946,7 +946,7 @@ extern int efi_memattr_apply_permissions(struct mm_struct *mm,
/* 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; \
+ (md) && ((void *)(md) + (m)->desc_size) <= (m)->map_end; \
(md) = (void *)(md) + (m)->desc_size)
/**
--
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
