[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 3/5] ns16550: widen an integer constant for Coverity.



>>> On 04.01.16 at 17:36, <ian.campbell@xxxxxxxxxx> wrote:
> On Mon, 2015-12-28 at 00:16 -0500, Joshua Otto wrote:
>> Fix CID 1343302 by widening a constant used with uart_param.reg_shift to
>> avoid overflow for large values of reg_shift.
> 
> A reg_shift large enough to actually expose this would be infeasibly large
> (since it would imply a UART taking practically the entire virtual address
> space of the processor).
> 
> So while Coverity is likely correct here, it is probably also a bit
> misguided in the context.
> 
> I don't especially object to this change as means to quieten coverity, but
> perhaps checking for some sane limit to reg_shift would also serve to
> quieten coverity?

Indeed I'd prefer an alternative change to be found, as 64-bit
arithmetic is quite pointless here.

>> Signed-off-by: Harley Armstrong <hjarmstr@xxxxxxxxxxxx>
>> ---
>>  xen/drivers/char/ns16550.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
>> index bc24015..546bba1 100644
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -913,7 +913,7 @@ pci_uart_config(struct ns16550 *uart, bool_t
>> skip_amt, unsigned int bar_idx)
>>                           * Force length of mmio region to be at least
>>                           * 8 bytes times (1 << reg_shift)
>>                          */
>> -                        if ( size < (0x8 * (1 << uart_param[p].reg_shift)) )
>> +                        if ( size < (0x8 * (1ull << 
>> uart_param[p].reg_shift)) )
> 
> It looks from the surrounding code like
>   ... < (0x8 * ((u64)1 << uart_param[p].reg_shift)) )
> 
> would be the preferred way of tackling this.

If we were to really stay with a change to this line, then the
multiplication should go away altogether, since just a shift
will always suffice.

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.