linux-kernel - Re: uvcvideo: shift exponent -7 is negative

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <158697922381.105027.9312351935387255622@swboyd.mtv.corp.google.com>
Date:   Wed, 15 Apr 2020 12:33:43 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     LKML <linux-kernel@...r.kernel.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        linux-media <linux-media@...r.kernel.org>,
        linux-uvc-devel@...ts.sourceforge.net,
        Fritz Koenig <frkoenig@...gle.com>
Subject: Re: uvcvideo: shift exponent -7 is negative

Quoting Randy Dunlap (2020-03-29 15:43:28)
> This is kernel version 5.6-rc6.
> 
> UBSAN detected a bad shift value:
> 
> [  511.693411] UBSAN: Undefined behaviour in ../drivers/media/usb/uvc/uvc_ctrl.c:781:13
> [  511.694043] shift exponent -7 is negative

I saw a similar problem. This patch fixed it for me but I'm not sure if
it's correct. The negative shift is done on the mask but we're going to
break out of the loop in that case so it isn't going to be used. Maybe
the loop should be a do while instead and then the mask can be
calculated at the start?

---8<----
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index e399b9fad757..ea6eb68329f3 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -778,7 +778,8 @@ 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;
+		if (bits > 0)
+			mask = (1 << bits) - 1;
 	}
 
 	/* Sign-extend the value if needed. */