Re: [multiple_bits_set()] New Defects reported by Coverity Scan for XenProject

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 04/09/2024 10:59 am, scan-admin@xxxxxxxxxxxx wrote:
> Hi,
>
> Please find the latest report on new defect(s) introduced to XenProject found 
> with Coverity Scan.
>
> 1 new defect(s) introduced to XenProject found with Coverity Scan.
>
>
> New defect(s) Reported-by: Coverity Scan
> Showing 1 of 1 defect(s)
>
>
> ** CID 1618898:  Integer handling issues  (INTEGER_OVERFLOW)
> /tools/firmware/xen-dir/xen-root/xen/arch/x86/cpu/vpmu.c: 676 in 
> do_xenpmu_op()
>
>
> ________________________________________________________________________________________________________
> *** CID 1618898:  Integer handling issues  (INTEGER_OVERFLOW)
> /tools/firmware/xen-dir/xen-root/xen/arch/x86/cpu/vpmu.c: 676 in 
> do_xenpmu_op()
> 670         switch ( op )
> 671         {
> 672         case XENPMU_mode_set:
> 673         {
> 674             if ( (pmu_params.val &
> 675                   ~(XENPMU_MODE_SELF | XENPMU_MODE_HV | XENPMU_MODE_ALL)) 
> ||
>>>>     CID 1618898:  Integer handling issues  (INTEGER_OVERFLOW)
>>>>     Expression "_v - 1UL", which is equal to 18446744073709551615, where 
>>>> "_v" is known to be equal to 0, underflows the type that receives it, an 
>>>> unsigned integer 64 bits wide.
> 676                  multiple_bits_set(pmu_params.val) )
> 677                 return -EINVAL;
> 678     
> 679             /* 32-bit dom0 can only sample itself. */
> 680             if ( is_pv_32bit_vcpu(current) &&
> 681                  (pmu_params.val & (XENPMU_MODE_HV | XENPMU_MODE_ALL)) )

I have to admit to being rather disappointed with this.

Yes there is a chance of underflow, but this is bit-pattern logic on
unsigned numbers and works irrespective.

But, I have no idea why this was singled out of the 3 examples, or
indeed how to persuade Coverity that underflows aren't relevant to this
expression.

~Andrew