Re: UBSAN: shift-out-of-bounds in drivers/media/usb/uvc/uvc_ctrl.c:781

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hello,

On Tue, Aug 18, 2020 at 02:58:22PM -0300, Fabio Estevam wrote:
> Hi Bart/Laurent,
> 
> On Tue, Aug 18, 2020 at 2:41 PM Bart Van Assche <bvanassche@acm.org> wrote:
> >
> > Hi,
> >
> > This morning I installed a debug build of kernel v5.8.1 on my laptop.
> > The complaint shown below appeared in the kernel log. Is this a known
> > issue?
> >
> > ================================================================================
> > UBSAN: shift-out-of-bounds in drivers/media/usb/uvc/uvc_ctrl.c:781:13
> > shift exponent -7 is negative
> 
> Should we fix it like this?
> 
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -778,7 +778,7 @@ static s32 uvc_get_le_value(struct
> uvc_control_mapping *mapping,
>                 value |= offset > 0 ? (byte >> offset) : (byte << (-offset));
>                 bits -= 8 - (offset > 0 ? offset : 0);
>                 offset -= 8;
> -               mask = (1 << bits) - 1;
> +               mask = (1LL << bits) - 1;
>         }

No, the issue is that bits is equal to -7, 1LL won't change that.

Once bits become negative, the loop stops, and the mask value isn't used
afterwards. This would only cause an issue if a shift with a negative
value generated side effects (such as a trap for instance) on top of
producing an incorrect result. Can this happen ? I suppose we should
silence the warning even if it's a false positive, as it doesn't look
good in the kernel log.

-- 
Regards,

Laurent Pinchart