2023-04-17 16:03:54

by Nikita Zhandarovich

Subject: [PATCH] HID: wacom: avoid integer overflow in wacom_intuos_inout()

If high bit is set to 1 in ((data[3] & 0x0f << 28), after all arithmetic
operations and integer promotions are done, high bits in
wacom->serial[idx] will be filled with 1s as well.
Avoid this, albeit unlikely, issue by specifying left operand's __u64
type for the right operand.

Found by Linux Verification Center (linuxtesting.org) with static
analysis tool SVACE.

Fixes: 3bea733ab212 ("USB: wacom tablet driver reorganization")
Signed-off-by: Nikita Zhandarovich <[email protected]>
---
drivers/hid/wacom_wac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 9312d611db8e..0e4404f3801e 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -826,7 +826,7 @@ static int wacom_intuos_inout(struct wacom_wac *wacom)
/* Enter report */
if ((data[1] & 0xfc) == 0xc0) {
/* serial number of the tool */
- wacom->serial[idx] = ((data[3] & 0x0f) << 28) +
+ wacom->serial[idx] = ((__u64)(data[3] & 0x0f) << 28) +
(data[4] << 20) + (data[5] << 12) +
(data[6] << 4) + (data[7] >> 4);

--
2.25.1

2023-04-17 17:16:39

by Ping Cheng

Subject: Re: [PATCH] HID: wacom: avoid integer overflow in wacom_intuos_inout()

On Mon, Apr 17, 2023 at 9:03 AM Nikita Zhandarovich
<[email protected]> wrote:
>
> If high bit is set to 1 in ((data[3] & 0x0f << 28), after all arithmetic
> operations and integer promotions are done, high bits in
> wacom->serial[idx] will be filled with 1s as well.
> Avoid this, albeit unlikely, issue by specifying left operand's __u64
> type for the right operand.

Thank you Nikita for the fix! It's good to avoid potential issues.

> Found by Linux Verification Center (linuxtesting.org) with static
> analysis tool SVACE.
>
> Fixes: 3bea733ab212 ("USB: wacom tablet driver reorganization")
> Signed-off-by: Nikita Zhandarovich <[email protected]>

Reviewed-by: Ping Cheng <[email protected]>
Cc: [email protected]

Cheers,
Ping
> ---
> drivers/hid/wacom_wac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 9312d611db8e..0e4404f3801e 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -826,7 +826,7 @@ static int wacom_intuos_inout(struct wacom_wac *wacom)
> /* Enter report */
> if ((data[1] & 0xfc) == 0xc0) {
> /* serial number of the tool */
> - wacom->serial[idx] = ((data[3] & 0x0f) << 28) +
> + wacom->serial[idx] = ((__u64)(data[3] & 0x0f) << 28) +
> (data[4] << 20) + (data[5] << 12) +
> (data[6] << 4) + (data[7] >> 4);
>
> --
> 2.25.1
>

2023-05-23 13:13:05

by Jiri Kosina

Subject: Re: [PATCH] HID: wacom: avoid integer overflow in wacom_intuos_inout()

On Mon, 17 Apr 2023, Nikita Zhandarovich wrote:

> If high bit is set to 1 in ((data[3] & 0x0f << 28), after all arithmetic
> operations and integer promotions are done, high bits in
> wacom->serial[idx] will be filled with 1s as well.
> Avoid this, albeit unlikely, issue by specifying left operand's __u64
> type for the right operand.
>
> Found by Linux Verification Center (linuxtesting.org) with static
> analysis tool SVACE.
>
> Fixes: 3bea733ab212 ("USB: wacom tablet driver reorganization")
> Signed-off-by: Nikita Zhandarovich <[email protected]>

Applied. Thanks for the fix, and sorry for the delay.

--
Jiri Kosina
SUSE Labs