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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.08.16 at 10:06, <wei.liu2@xxxxxxxxxx> wrote:
> Coverity complains:
> 
> overflow_before_widen: Potentially overflowing expression
> info->nr_modules * 32U with type unsigned int (32 bits, unsigned) is
> evaluated using 32-bit arithmetic, and then used in a context that
> expects an expression of type uint64_t (64 bits, unsigned).

To me this is Coverity splitting hair, to be honest.

> --- 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.

Jan


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