From: Jinmo Yang <jinmo44.yang@gmail.com>
To: Jiri Kosina <jikos@kernel.org>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jinmo Yang <jinmo44.yang@gmail.com>,
Ping Cheng <ping.cheng@wacom.com>,
Jason Gerecke <jason.gerecke@wacom.com>,
Benjamin Tissoires <bentiss@kernel.org>,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] HID: wacom: fix NULL pointer dereference in wacom_intuos_pad()
Date: Sun, 27 Sep 2026 03:37:57 +0900 [thread overview]
Message-ID: <20260926183757.3347378-1-jinmo44.yang@gmail.com> (raw)
In-Reply-To: <579o8so3-09o3-o312-4sq0-0qp6n672q46p@xreary.bet>
On Wed, 10 Jun 2026, Jiri Kosina wrote:
> Jinmo, are you planning to submit extended version of the patch, please?
Yes - sorry for the long delay. Dmitry's observation turned out to be
broader than pad_input, so I audited the whole driver before replying.
The check already exists in three places, just not everywhere:
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. The clearest case 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);
The pointers can be NULL on a fully successful probe: the handler is
chosen by features.type from the id table (wacom_sys.c:2945), while the
input devices are allocated from what the descriptor declares. When
wacom_setup_{pen,touch,pad}_input_capabilities() returns -ENODEV,
wacom_setup_inputs() frees that input, NULLs the pointer and still
returns 0 (wacom_sys.c:2172-2196) - "no pen in use on this interface".
wacom_wac_irq() then still routes reports to a handler that dereferences
it. No error injection is needed; a descriptor declaring only touch
usages is enough.
15 locations, each reproduced as a KASAN NULL-pointer dereference on
linux-next 20260925, x86_64, from one /dev/uhid device plus a single
UHID_INPUT2 write:
wacom_wac.c:137 wacom_penpartner_irq pen_input
wacom_wac.c:227 wacom_pl_irq pen_input
wacom_wac.c:244 wacom_ptu_irq pen_input
wacom_wac.c:303 wacom_dtus_irq pad_input
wacom_wac.c:326 wacom_dtus_irq pen_input
wacom_wac.c:391 wacom_graphire_irq pen_input
wacom_wac.c:444 wacom_graphire_irq pad_input
wacom_wac.c:594 wacom_intuos_pad pad_input
wacom_wac.c:1209 wacom_intuos_bt_process_data pen_input
wacom_wac.c:1232 wacom_intuos_bt_irq pen_input (dev_warn)
wacom_wac.c:3100 wacom_bpt_touch touch_input
wacom_wac.c:3117 wacom_bpt_touch pad_input
wacom_wac.c:3130 wacom_bpt3_touch_msg touch_input
wacom_wac.c:3178 wacom_bpt3_button_msg pad_input
wacom_wac.c:4229 wacom_report_numbered_buttons pad_input
Jason, your review of my earlier series ("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 I would like to fold the two together
and do one edit per handler: take len, use a named constant, and check
the input device in the same place.
On the shape of the fix: I plan to put a check where each pointer is
taken, as wacom_tpc_irq() already does. A per-features.type table of
required inputs would be one place instead of ~15, but a handler's needs
vary by report id - wacom_graphire_irq() takes pen_input on one branch
and pad_input on another - so the mask would have to demand every input
the handler might touch, and would then reject reports that work today
on a pen-only interface, which the "no pen in use on this interface"
case says is legitimate. Say the word if you would rather have that
anyway.
I will post it as a series split by device family shortly, with the
shared helpers (wacom_report_numbered_buttons(),
wacom_wac_finger_count_touches()) guarded once each, since several
handlers converge on them.
Thanks,
Jinmo
next prev parent reply other threads:[~2026-09-26 18:38 UTC|newest]
Thread overview: 14+ 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-23 15:50 ` sashiko-bot
2026-05-29 21:44 ` Dmitry Torokhov
2026-06-10 15:48 ` Jiri Kosina
2026-09-26 18:37 ` Jinmo Yang [this message]
2026-09-27 4:11 ` [PATCH 0/5] HID: wacom: check input devices in the report handlers Jinmo Yang
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:24 ` sashiko-bot
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
2026-05-23 15:45 ` [PATCH] HID: wacom: fix NULL pointer dereference in wacom_intuos_pad() sashiko-bot
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=20260926183757.3347378-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.