From: Jinmo Yang <jinmo44.yang@gmail.com>
To: ping.cheng@wacom.com, jason.gerecke@wacom.com, jikos@kernel.org,
bentiss@kernel.org
Cc: dmitry.torokhov@gmail.com, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, Jinmo Yang <jinmo44.yang@gmail.com>
Subject: [PATCH 0/5] HID: wacom: check input devices in the report handlers
Date: Sun, 27 Sep 2026 13:11:33 +0900 [thread overview]
Message-ID: <20260927041138.4112920-1-jinmo44.yang@gmail.com> (raw)
In-Reply-To: <579o8so3-09o3-o312-4sq0-0qp6n672q46p@xreary.bet>
This is the extended version Jiri asked for in
<579o8so3-09o3-o312-4sq0-0qp6n672q46p@xreary.bet>, 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.
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.
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
drivers/hid/wacom_wac.c | 97 +++++++++++++++++++++++++++++------------
1 file changed, 70 insertions(+), 27 deletions(-)
base-commit: f5f84daefcd92d7a630066635ecea1433ed5eac7
--
2.53.0
next prev parent reply other threads:[~2026-09-27 4:11 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-23 15:01 [PATCH] HID: wacom: fix NULL pointer dereference in wacom_intuos_pad() Jinmo Yang
2026-05-23 15:06 ` [PATCH v2] " Jinmo Yang
2026-05-29 21:44 ` Dmitry Torokhov
2026-06-10 15:48 ` Jiri Kosina
2026-09-26 18:37 ` Jinmo Yang
2026-09-27 4:11 ` Jinmo Yang [this message]
2026-09-27 4:11 ` [PATCH 1/5] HID: wacom: check the input device in the shared report helpers Jinmo Yang
2026-09-27 4:11 ` [PATCH 2/5] HID: wacom: check the input devices in the legacy irq handlers Jinmo Yang
2026-09-27 4:11 ` [PATCH 3/5] HID: wacom: check the input device in wacom_intuos_pad() Jinmo Yang
2026-09-27 4:11 ` [PATCH 4/5] HID: wacom: check the input device in wacom_intuos_bt_irq() Jinmo Yang
2026-09-27 4:11 ` [PATCH 5/5] HID: wacom: check the input devices in the Bamboo handlers Jinmo Yang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260927041138.4112920-1-jinmo44.yang@gmail.com \
--to=jinmo44.yang@gmail.com \
--cc=bentiss@kernel.org \
--cc=dmitry.torokhov@gmail.com \
--cc=jason.gerecke@wacom.com \
--cc=jikos@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ping.cheng@wacom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®