Re: [PATCH] HID: wacom: check for NULL pad_input before reporting pad events

[Jason Gerecke]

On Thu, Sep 24, 2026 at 10:53 PM Rohinthan P <rokinthanp03@xxxxxxxxx> wrote:
>
> When a Wacom device interface does not have pad capabilities (for
> example, when wacom_setup_pad_input_capabilities() returns an error
> or the interface is configured as pen-only), wacom_wac->pad_input is
> set to NULL.
>

I imagine that similar problems could occur for the pen and touch
interfaces too since they're initialized and used in the exact same
ways (e.g. wacom_intuos_inout and wacom_intuos_general both implicitly
assume that pen_input is non-NULL). I would recommend creating a
similar version of this patch that adds the necessary checks for uses
of pen_input and touch_input.

> If incoming HID reports contain pad packets (such as
> WACOM_REPORT_INTUOSPAD, WACOM_REPORT_INTUOS5PAD, or
> WACOM_REPORT_CINTIQPAD), wacom_intuos_pad() assigns
> input = wacom->pad_input and proceeds without verifying that input is
> non-NULL. It calls wacom_report_numbered_buttons(input, ...), which
> dereferences input via input_get_drvdata(input), triggering a general
> protection fault / NULL pointer dereference:
>
>   Oops: general protection fault, probably for non-canonical address 0xdffffc000000006c: 0000 [#1] SMP KASAN NOPTI
>   KASAN: null-ptr-deref in range [0x0000000000000360-0x0000000000000367]
>   RIP: 0010:dev_get_drvdata include/linux/device.h:991 [inline]
>   RIP: 0010:input_get_drvdata include/linux/input.h:396 [inline]
>   RIP: 0010:wacom_report_numbered_buttons+0x37/0x210 drivers/hid/wacom_wac.c:4225
>   Call Trace:
>    <TASK>
>    wacom_intuos_pad drivers/hid/wacom_wac.c:643 [inline]
>    wacom_intuos_irq+0x2f8/0x3430 drivers/hid/wacom_wac.c:1042
>    wacom_bpt_irq drivers/hid/wacom_wac.c:3290 [inline]
>    wacom_wac_irq+0x196b/0xab80 drivers/hid/wacom_wac.c:3560
>    wacom_raw_event+0x6bb/0xba0 drivers/hid/wacom_sys.c:183
>
> Add checks to verify that pad_input / input_dev is not NULL in
> wacom_intuos_pad(), wacom_report_numbered_buttons(),
> wacom_intuos_pro2_bt_pad(), and wacom_intuos_gen3_bt_pad(). In addition,
> in wacom_intuos_irq(), return immediately if the packet is a pad report
> to prevent pad packets from erroneously falling through to pen event
> handling routines when pad_input is not allocated.
>
> Fixes: 49005b9fd052 ("HID: wacom: Refactor button-to-key translation into function")
> Reported-by: syzbot+ae7f998154426e723cbf@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=ae7f998154426e723cbf
> Signed-off-by: Rohinthan P <rokinthanp03@xxxxxxxxx>
> ---
>  drivers/hid/wacom_wac.c | 38 ++++++++++++++++++++++++++++----------
>  1 file changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index af76e49..6a13724 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -529,6 +529,9 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
>               data[0] == WACOM_REPORT_CINTIQPAD))
>                 return 0;
>
> +       if (!input)
> +               return 0;
> +

Please add a dev_warn message before returning. It is extremely
strange to be processing a pad packet (identified by report ID) and
*not* have the pad device allocated...

>         if (features->type >= INTUOS4S && features->type <= INTUOS4L) {
>                 buttons = (data[3] << 1) | (data[2] & 0x01);
>                 ring1 = data[1];
> @@ -1039,9 +1042,10 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
>         }
>
>         /* process pad events */
> -       result = wacom_intuos_pad(wacom);
> -       if (result)
> -               return result;
> +       if (data[0] == WACOM_REPORT_INTUOSPAD ||
> +           data[0] == WACOM_REPORT_INTUOS5PAD ||
> +           data[0] == WACOM_REPORT_CINTIQPAD)
> +               return wacom_intuos_pad(wacom);
>

This check is already performed within wacom_intuos_pad. Duplicating
it here is just asking for trouble as one check gets updated but not
the other. The pad function knows best what kind of reports it should
handle, so overall I am going to NAK to this particular hunk.

>         /* process in/out prox events */
>         result = wacom_intuos_inout(wacom);
> @@ -1475,12 +1479,17 @@ static void wacom_intuos_pro2_bt_pad(struct wacom_wac *wacom)
>         struct input_dev *pad_input = wacom->pad_input;
>         unsigned char *data = wacom->data;
>         int nbuttons = wacom->features.numbered_buttons;
> +       int expresskeys, center, ring;
> +       bool ringstatus, prox;
> +
> +       if (!pad_input)
> +               return;
>
> -       int expresskeys = data[282];
> -       int center = (data[281] & 0x40) >> 6;
> -       int ring = data[285] & 0x7F;
> -       bool ringstatus = data[285] & 0x80;
> -       bool prox = expresskeys || center || ringstatus;
> +       expresskeys = data[282];
> +       center = (data[281] & 0x40) >> 6;
> +       ring = data[285] & 0x7F;
> +       ringstatus = data[285] & 0x80;
> +       prox = expresskeys || center || ringstatus;
>

Most of this hunk is cleanup that has nothing to do with the new
pad_input check. Please remove the cleanup so this patch can be
focused on the fix.

Also: similar to wacom_intuos_pad, please add a dev_warn message
before returning due to NULL pad_input.

>         /* Fix touchring data: userspace expects 0 at left and increasing clockwise */
>         ring = 71 - ring;
> @@ -1515,8 +1524,12 @@ static void wacom_intuos_gen3_bt_pad(struct wacom_wac *wacom)
>  {
>         struct input_dev *pad_input = wacom->pad_input;
>         unsigned char *data = wacom->data;
> +       int buttons;
>
> -       int buttons = data[44];
> +       if (!pad_input)
> +               return;
> +
> +       buttons = data[44];
>
>         wacom_report_numbered_buttons(pad_input, 4, buttons);
>

Same comments as the hunk above. Remove the cleanup-related change to
'buttons' and introduce a dev_warn before returning due to NULL
pad_input.


> @@ -4220,9 +4233,14 @@ static void wacom_update_led(struct wacom *wacom, int button_count, int mask,
>  static void wacom_report_numbered_buttons(struct input_dev *input_dev,
>                                 int button_count, int mask)
>  {
> -       struct wacom *wacom = input_get_drvdata(input_dev);
> +       struct wacom *wacom;
>         int i;
>
> +       if (!input_dev)
> +               return;
> +
> +       wacom = input_get_drvdata(input_dev);
> +
>         for (i = 0; i < wacom->led.count; i++)
>                 wacom_update_led(wacom,  button_count, mask, i);
>

This hunk appears to be redundant since the above hunks appear to
fully cover and protect all callers of wacom_report_numbered_buttons.
Generally we try to avoid repeating NULL checks, but since this is a
utility function maybe it makes sense to keep here. If the NULL check
is kept, I would request another dev_warn log here as well.

Jason (she/they)
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....

> --
> 2.53.0
>
>