* [PATCH v2] EFI: make for_each_efi_memory_desc_in_map() cope with running on Xen
@ 2016-08-15 15:05 Jan Beulich
[not found] ` <57B1F6690200007800106095-rw/UEucdPrvD8XXLLHKrIiOjQekVJEpY@public.gmane.org>
0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2016-08-15 15:05 UTC (permalink / raw)
To: matt-mF/unelCI9GS6iBeEJttW/XRex20P6io
Cc: Vitaly Kuznetsov, Jiri Slaby, linux-efi-u79uwXL29TY76Z2rM5mHXA
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-Ilq5uHa1fWNWk0Htik3J/w@public.gmane.org>
Cc: Vitaly Kuznetsov <vkuznets-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Jiri Slaby <jslaby-AlSwsSmVLrQ@public.gmane.org>
Signed-off-by: Jan Beulich <jbeulich-IBi9RG/b67k@public.gmane.org>
Tested-by: <lists-Ilq5uHa1fWNWk0Htik3J/w@public.gmane.org>
---
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)
/**
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] EFI: make for_each_efi_memory_desc_in_map() cope with running on Xen
[not found] ` <57B1F6690200007800106095-rw/UEucdPrvD8XXLLHKrIiOjQekVJEpY@public.gmane.org>
@ 2016-08-16 11:49 ` Matt Fleming
[not found] ` <20160816114917.GM30909-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
0 siblings, 1 reply; 4+ messages in thread
From: Matt Fleming @ 2016-08-16 11:49 UTC (permalink / raw)
To: Jan Beulich
Cc: Vitaly Kuznetsov, Jiri Slaby, linux-efi-u79uwXL29TY76Z2rM5mHXA,
Mark Rutland
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-Ilq5uHa1fWNWk0Htik3J/w@public.gmane.org>
> Cc: Vitaly Kuznetsov <vkuznets-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Jiri Slaby <jslaby-AlSwsSmVLrQ@public.gmane.org>
> Signed-off-by: Jan Beulich <jbeulich-IBi9RG/b67k@public.gmane.org>
> Tested-by: <lists-Ilq5uHa1fWNWk0Htik3J/w@public.gmane.org>
> ---
> 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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] EFI: make for_each_efi_memory_desc_in_map() cope with running on Xen
[not found] ` <20160816114917.GM30909-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
@ 2016-08-19 10:47 ` Matt Fleming
[not found] ` <20160819104715.GR30909-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
0 siblings, 1 reply; 4+ messages in thread
From: Matt Fleming @ 2016-08-19 10:47 UTC (permalink / raw)
To: Jan Beulich
Cc: Vitaly Kuznetsov, Jiri Slaby, linux-efi-u79uwXL29TY76Z2rM5mHXA,
Mark Rutland
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-Ilq5uHa1fWNWk0Htik3J/w@public.gmane.org>
> > Cc: Vitaly Kuznetsov <vkuznets-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Cc: Jiri Slaby <jslaby-AlSwsSmVLrQ@public.gmane.org>
> > Signed-off-by: Jan Beulich <jbeulich-IBi9RG/b67k@public.gmane.org>
> > Tested-by: <lists-Ilq5uHa1fWNWk0Htik3J/w@public.gmane.org>
> > ---
> > 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)
/**
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] EFI: make for_each_efi_memory_desc_in_map() cope with running on Xen
[not found] ` <20160819104715.GR30909-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
@ 2016-08-19 12:07 ` Jan Beulich
0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2016-08-19 12:07 UTC (permalink / raw)
To: Matt Fleming
Cc: Mark Rutland, Vitaly Kuznetsov, Jiri Slaby,
linux-efi-u79uwXL29TY76Z2rM5mHXA
>>> On 19.08.16 at 12:47, <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:
> On Tue, 16 Aug, at 12:49:17PM, Matt Fleming wrote:
>> On Mon, 15 Aug, at 09:05:45AM, Jan Beulich wrote:
>> > --- 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?
I think so, yes, but I've not seen the issue myself, so I can't verify
that's the case.
Jan
> --- 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)
>
> /**
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-08-19 12:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-15 15:05 [PATCH v2] EFI: make for_each_efi_memory_desc_in_map() cope with running on Xen Jan Beulich
[not found] ` <57B1F6690200007800106095-rw/UEucdPrvD8XXLLHKrIiOjQekVJEpY@public.gmane.org>
2016-08-16 11:49 ` Matt Fleming
[not found] ` <20160816114917.GM30909-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2016-08-19 10:47 ` Matt Fleming
[not found] ` <20160819104715.GR30909-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2016-08-19 12:07 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.