All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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.