Hi Bart,
On Wed, Aug 19, 2020 at 08:25:27PM -0700, Bart Van Assche wrote:
> On 2020-08-18 17:03, Laurent Pinchart wrote:
> > UBSAN reports a shift-out-of-bounds warning in uvc_get_le_value(). The
> > report is correct, but the issue should be harmless as the computed
> > value isn't used when the shift is negative. This may however cause
> > incorrect behaviour if a negative shift could generate adverse side
> > effects (such as a trap on some architectures for instance).
> >
> > Regardless of whether that may happen or not, silence the warning as a
> > full WARN backtrace isn't nice.
> >
> > Reported-by: Bart Van Assche <bvanassche@xxxxxxx>
> > Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> > drivers/media/usb/uvc/uvc_ctrl.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index 6c37aa018ad5..b2cdee0f7763 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -773,12 +773,16 @@ static s32 uvc_get_le_value(struct uvc_control_mapping *mapping,
> > offset &= 7;
> > mask = ((1LL << bits) - 1) << offset;
> >
> > - for (; bits > 0; data++) {
> > + while (1) {
> > u8 byte = *data & mask;
> > value |= offset > 0 ? (byte >> offset) : (byte << (-offset));
> > bits -= 8 - (offset > 0 ? offset : 0);
> > + if (bits <= 0)
> > + break;
> > +
> > offset -= 8;
> > mask = (1 << bits) - 1;
> > + data++;
> > }
> >
> > /* Sign-extend the value if needed. */
>
> Reviewed-by: Bart Van Assche <bvanassche@xxxxxxx>
>
> Thanks for having addressed this quickly!
You're welcome. Would you be able to test the patch to ensure it fixes
the issue on your system (and that there are no observable side effects)
?
--
Regards,
Laurent Pinchart
