Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230417160148.60011-1-n.zhandarovich@fintech.ru>
In-Reply-To: <20230417160148.60011-1-n.zhandarovich@fintech.ru>
From:   Ping Cheng <pinglinux@gmail.com>
Date:   Mon, 17 Apr 2023 10:13:43 -0700
Message-ID: <CAF8JNh+Ba1SNT5JFy3EGAAtcfZdRYcQ_M4LtQoV0H0dViFyGcw@mail.gmail.com>
Subject: Re: [PATCH] HID: wacom: avoid integer overflow in wacom_intuos_inout()
To:     Nikita Zhandarovich <n.zhandarovich@fintech.ru>
Cc:     Ping Cheng <ping.cheng@wacom.com>,
        Jason Gerecke <jason.gerecke@wacom.com>,
        Jiri Kosina <jikos@kernel.org>,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
        lvc-project@linuxtesting.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Apr 17, 2023 at 9:03=E2=80=AFAM Nikita Zhandarovich
<n.zhandarovich@fintech.ru> 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 <n.zhandarovich@fintech.ru>

Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
Cc: stable@vger.kernel.org

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) =3D=3D 0xc0) {
>                 /* serial number of the tool */
> -               wacom->serial[idx] =3D ((data[3] & 0x0f) << 28) +
> +               wacom->serial[idx] =3D ((__u64)(data[3] & 0x0f) << 28) +
>                         (data[4] << 20) + (data[5] << 12) +
>                         (data[6] << 4) + (data[7] >> 4);
>
> --
> 2.25.1
>