Re: [Xen-devel] [PATCH 2/2] hvmloader: cast to 64bit before multiplication in get_module_entry

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.08.16 at 13:37, <wei.liu2@xxxxxxxxxx> wrote:
> On Fri, Aug 19, 2016 at 06:00:12AM -0600, Jan Beulich wrote:
>> >>> On 19.08.16 at 12:09, <andrew.cooper3@xxxxxxxxxx> wrote:
>> > On 19/08/16 09:31, Jan Beulich wrote:
>> >>>>> On 19.08.16 at 10:06, <wei.liu2@xxxxxxxxxx> wrote:
>> >>> --- a/tools/firmware/hvmloader/hvmloader.c
>> >>> +++ b/tools/firmware/hvmloader/hvmloader.c
>> >>> @@ -272,8 +272,8 @@ const struct hvm_modlist_entry *get_module_entry(
>> >>>  
>> >>>      if ( !modlist ||
>> >>>           info->modlist_paddr > UINTPTR_MAX ||
>> >>> -         (info->modlist_paddr + info->nr_modules * sizeof(*modlist) - 1)
>> >>> -            > UINTPTR_MAX )
>> >>> +         (info->modlist_paddr +
>> >>> +          (uint64_t)info->nr_modules * sizeof(*modlist) - 1) > 
>> >>> UINTPTR_MAX )
>> >>>          return NULL;
>> >> This can be had without resorting to 64-bit multiplication, by bounds
>> >> checking
>> >>
>> >>          (UINTPTR_MAX - (uintptr_t)info->modlist_paddr) / sizeof(*modlist)
>> >>
>> >> instead. While we would certainly hope that compilers don't resort
>> >> to a libgcc helper for 64-bit multiplication, I think we'd better avoid
>> >> that risk altogether.
>> > 
>> > In this case, using libgcc would cause a link error because of
>> > -fno-builtin, so I don't think it is too bad.
>> 
>> And it's this link error which I want to avoid.
> 
> What approach should I use? I would like to clear this minor issue as
> quick as possible.

Well, if Andrew wants to ack the patch with the above unchanged, I
won't object. I continue to prefer the suggested alternative though.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel