Re: [PATCH 0/5] HID: wacom: check input devices in the report handlers

On Sat, Sep 26, 2026 at 9:11 PM Jinmo Yang [off-list ref] wrote:

This is the extended version Jiri asked for in
[ref], after Dmitry's
observation that "there are many more places in the driver where it used
wacom->pad_input without verifying that it exists".

The audit turned out broader than pad_input. Of the 155 lines in
wacom_wac.c that reference pen_input, touch_input or pad_input, three
places already test the pointer first:

  wacom_wac.c:1807/1815  wacom_tpc_irq                 pen, touch
  wacom_wac.c:1210       wacom_intuos_bt_process_data  pad
  wacom_wac.c:2192       wacom_wac_pad_event           (HID_GENERIC)

so the inconsistency is inside the legacy dispatch rather than between it
and the HID_GENERIC path. The clearest illustration is two adjacent lines
in wacom_intuos_bt_process_data(), one checked and one not:

        input_sync(wacom->pen_input);           /* :1209 */
        if (wacom->pad_input)                   /* :1210 */
                input_sync(wacom->pad_input);

Why the pointers can be NULL on a *successful* probe
====================================================
For a non-HID_GENERIC device the report handler is selected by
features.type, which comes from the id table (wacom_sys.c:2945), while the
three input devices are allocated from what the report descriptor declares.
When wacom_setup_{pen,touch,pad}_input_capabilities() returns -ENODEV,
wacom_setup_inputs() frees that input device, sets the pointer to NULL and
still returns 0 (wacom_sys.c:2172-2196) - the "no pen in use on this
interface" case. wacom_wac_irq() then still routes reports to a handler
that dereferences it.

No error injection is needed. A descriptor declaring only touch usages
leaves pen_input and pad_input NULL; one declaring only pen usages leaves
touch_input and pad_input NULL.

What the series fixes
=====================
15 locations, each reproduced as a KASAN NULL-pointer dereference from one
/dev/uhid device plus a single UHID_INPUT2 write, with no privilege beyond
access to /dev/uhid:

  patch  file:line          function                       pointer
  1      wacom_wac.c:4229   wacom_report_numbered_buttons  pad
  2      wacom_wac.c:137    wacom_penpartner_irq           pen
  2      wacom_wac.c:227    wacom_pl_irq                   pen
  2      wacom_wac.c:244    wacom_ptu_irq                  pen
  2      wacom_wac.c:303    wacom_dtus_irq                 pad
  2      wacom_wac.c:326    wacom_dtus_irq                 pen
  2      wacom_wac.c:391    wacom_graphire_irq             pen
  2      wacom_wac.c:444    wacom_graphire_irq             pad
  3      wacom_wac.c:594    wacom_intuos_pad               pad
  4      wacom_wac.c:1209   wacom_intuos_bt_process_data   pen
  4      wacom_wac.c:1232   wacom_intuos_bt_irq            pen
         (both covered by one check at wacom_intuos_bt_irq() entry)
  5      wacom_wac.c:3100   wacom_bpt_touch                touch
  5      wacom_wac.c:3117   wacom_bpt_touch                pad
  5      wacom_wac.c:3130   wacom_bpt3_touch_msg           touch
  5      wacom_wac.c:3178   wacom_bpt3_button_msg          pad

Patch 1 also guards wacom_wac_finger_count_touches(), and patch 2 also
guards wacom_dtu_irq(); both are reachable with a NULL pointer but their
only dereference is in a dev_dbg(), so they do not fault with
CONFIG_DYNAMIC_DEBUG=n.

Dereferences that are reached only through dev_dbg() are otherwise out of
scope here. A few remain, on unknown-report paths in wacom_dtus_irq(),
wacom_graphire_irq() and wacom_intuos_irq(); guarding those needs a
separate look at what a pen-only or pad-only interface should still
report, so I have left them for a follow-up rather than mixing them in.

There are a large class of devices in the legacy codepath for which a
pen-only interface is not valid. There are exceptions (e.g. GRAPHIRE
is a pen-only interface), but in many cases (see
wacom_setup_device_quirks) interfaces which have a pen interface will
also have a pad. I'm not aware of *any* Wacom device which has a
pad-only interface (though I could be wrong; I can try to dig up old
spec documents or original commits that would shed light about any
cases where there is doubt).

Approach
========
A check where each pointer is taken, which is what wacom_tpc_irq() already
does. Where a handler serves both pen and pad reports - wacom_dtus_irq(),
wacom_graphire_irq(), wacom_bpt_touch() - the checks are per branch rather
than at function entry, so an interface that has a pad but no pen keeps
delivering pad events.

A per-features.type table of required input devices would be one place
instead of 15, but a handler's needs vary by report id, so such a mask
would have to demand every input the handler might touch and would then
reject reports that work today on a partial interface. I am happy to build
that instead if you would rather have it.

Testing
=======
linux-next 20260925 (7.3.0-rc4-next-20260925-gf5f84daefcd9), x86_64,
CONFIG_KASAN_GENERIC=y, CONFIG_DYNAMIC_DEBUG=n. 27 uhid reproducer cases
covering the sites above plus the dev_dbg-only ones, one fresh VM per case
because the oops leaves driver_input_lock held:

                              before    after
    KASAN faults                  17        0
    probe failures                 0        0
    hidraw nodes created          26       26
    input devices registered      25       25

and the set of input devices registered is identical in all 27 cases, so
the checks remove the faults without changing what a device exposes. The
one case that creates no device is a product whose table entry sets
.check_for_hid_type, which uhid cannot satisfy; it behaves the same before
and after.

Each patch builds standalone with no new warnings, and checkpatch --strict
reports 0 errors, 0 warnings and 0 checks for all five.

Not in this series
==================
Jason, your review of "HID: wacom: add report length validation in irq
handlers" (17 May) asked for the length checks to move into the
sub-functions with len passed in, plus WACOM_PKGLEN_* names. Eight
handlers already take len and eight do not, and six of those eight are
also on the list above, so that work overlaps this series closely. I have
kept it out here because I have only reproduced and verified the
input-device faults; I would rather send the length revision once it has
the same evidence behind it, on top of this series.

This is understandable. I will note, however, that while evidence of
misbehavior is appreciated, it is not required. Consistently
performing various precondition checks across similar functions, even
if they are not *strictly* necessary at this time, can still encourage
good habits for future code.

Jinmo Yang (5):
  HID: wacom: check the input device in the shared report helpers
  HID: wacom: check the input devices in the legacy irq handlers
  HID: wacom: check the input device in wacom_intuos_pad()
  HID: wacom: check the input device in wacom_intuos_bt_irq()
  HID: wacom: check the input devices in the Bamboo handlers

I do somewhat question the decision to split this series into five
pieces. I feel that most could be combined, especially after I have
checked that the few per-branch checks in this series can be
simplified to per-function checks. Combining the patches is not a hard
request, however.

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....

 drivers/hid/wacom_wac.c | 97 +++++++++++++++++++++++++++++------------
 1 file changed, 70 insertions(+), 27 deletions(-)


base-commit: f5f84daefcd92d7a630066635ecea1433ed5eac7
--
2.53.0