All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] HID: wacom: Prevent NULL dereference in wireless work
@ 2026-10-30 17:31 Devarjya Purkayastha
  2026-10-30 17:45 ` sashiko-bot
  2026-10-30 20:43 ` Jason Gerecke
  0 siblings, 2 replies; 3+ messages in thread
From: Devarjya Purkayastha @ 2026-10-30 17:31 UTC (permalink / raw)
  To: linux-input
  Cc: ping.cheng, jason.gerecke, jikos, bentiss, linux-kernel,
	Devarjya Purkayastha, Indrath

wacom_wireless_work() assumes that the active USB configuration has
three interfaces and unconditionally accesses interface[1] and
interface[2].

A malformed configuration with fewer interfaces can therefore cause a
NULL pointer dereference in the wireless workqueue. Return early unless
the configuration has at least three interfaces.

Reported-by: Indrath <indrathftw@gmail.com>
Signed-off-by: Devarjya Purkayastha <unseentactician27062006@gmail.com>
---
 drivers/hid/wacom_sys.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 628372ab818a..dcf2811a5c48 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2619,7 +2619,7 @@ static void wacom_wireless_work(struct work_struct *work)
 
 	wacom_destroy_battery(wacom);
 
-	if (!usbdev)
+	if (!usbdev || usbdev->config->desc.bNumInterfaces < 3)
 		return;
 
 	/* Stylus interface */
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] HID: wacom: Prevent NULL dereference in wireless work
  2026-10-30 17:31 [PATCH] HID: wacom: Prevent NULL dereference in wireless work Devarjya Purkayastha
@ 2026-10-30 17:45 ` sashiko-bot
  2026-10-30 20:43 ` Jason Gerecke
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-10-30 17:45 UTC (permalink / raw)
  To: Devarjya Purkayastha, linux-input; +Cc: dmitry.torokhov

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] HID: wacom: incomplete NULL and binding checks in wacom_wireless_work()
--

commit 53aa61197952eeb0781af2deacd78d22c186ee96
Author: Devarjya Purkayastha <unseentactician27062006@gmail.com>

HID: wacom: Prevent NULL dereference in wireless work

This commit checks that usbdev->config->desc.bNumInterfaces is at least 3 in
wacom_wireless_work() before accessing interface[1] and interface[2].

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 628372ab818a2..dcf2811a5c483 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2619,7 +2619,7 @@ static void wacom_wireless_work(struct work_struct *work)
>  
>  	wacom_destroy_battery(wacom);
>  
> -	if (!usbdev)
> +	if (!usbdev || usbdev->config->desc.bNumInterfaces < 3)
>  		return;

[Severity: High]
Is checking usbdev->config->desc.bNumInterfaces < 3 sufficient to prevent a
NULL pointer dereference in wacom_wireless_work()?

First, the commit message states that this checks the active USB
configuration, but usbdev->config points to the array of all configurations
(&usbdev->config[0]), whereas usbdev->actconfig points to the currently
active configuration populated by usb_set_configuration(). On a malformed
multi-configuration device where config[0] has bNumInterfaces >= 3 while a
different configuration is active in usbdev->actconfig,
usbdev->config->interface[1] and usbdev->config->interface[2] will be NULL,
causing usb_get_intfdata() to dereference a NULL pointer:

	/* Stylus interface */
	hdev1 = usb_get_intfdata(usbdev->config->interface[1]);
	wacom1 = hid_get_drvdata(hdev1);
	wacom_wac1 = &(wacom1->wacom_wac);
	wacom_release_resources(wacom1);

Second, even when interface[1] and interface[2] are non-NULL, can hdev1 or
hdev2 be NULL? During usb_set_configuration(), interfaces are probed
sequentially. When interface[0] is probed, wacom_parse_and_register() opens
the HID device and starts interrupt URB reception before interface[1] and
interface[2] have been probed:

	if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR) {
		error = hid_hw_open(hdev);
		if (error) {
			hid_err(hdev, "hw open failed\n");
			goto fail_hw_stop;
		}
	}

In addition, on a malformed USB device where interface[1] or interface[2]
fails usbhid_probe() (for example, due to missing an interrupt-IN endpoint)
or is unbound, usb_get_intfdata() returns NULL for hdev1 or hdev2, and
hid_get_drvdata() will dereference a NULL pointer.

Third, even if usbhid_probe() succeeds so hdev1 and hdev2 are non-NULL,
wacom_probe() can fail on interface[1] or interface[2] (for example, if the
HID report descriptor is malformed), or the wacom driver can be unbound from
those interfaces. In that case hid_get_drvdata() returns NULL for wacom1 or
wacom2, and wacom_release_resources() immediately dereferences it:

static void wacom_release_resources(struct wacom *wacom)
{
	struct hid_device *hdev = wacom->hdev;

	if (!wacom->resources)
		return;

Should wacom_wireless_work() check usbdev->actconfig as well as verify that
the sibling interfaces and their hdev and wacom driver data pointers are
valid and bound before dereferencing them?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030173108.45408-1-unseentactician27062006@gmail.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] HID: wacom: Prevent NULL dereference in wireless work
  2026-10-30 17:31 [PATCH] HID: wacom: Prevent NULL dereference in wireless work Devarjya Purkayastha
  2026-10-30 17:45 ` sashiko-bot
@ 2026-10-30 20:43 ` Jason Gerecke
  1 sibling, 0 replies; 3+ messages in thread
From: Jason Gerecke @ 2026-10-30 20:43 UTC (permalink / raw)
  To: Devarjya Purkayastha
  Cc: linux-input, ping.cheng, jason.gerecke, jikos, bentiss,
	linux-kernel, Indrath

Sashiko responses notwithstanding (which the rest of the function
would be need to be updated to implement, not *just* this patch):

Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>

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


On Fri, Oct 30, 2026 at 10:35 AM Devarjya Purkayastha
<unseentactician27062006@gmail.com> wrote:
>
> wacom_wireless_work() assumes that the active USB configuration has
> three interfaces and unconditionally accesses interface[1] and
> interface[2].
>
> A malformed configuration with fewer interfaces can therefore cause a
> NULL pointer dereference in the wireless workqueue. Return early unless
> the configuration has at least three interfaces.
>
> Reported-by: Indrath <indrathftw@gmail.com>
> Signed-off-by: Devarjya Purkayastha <unseentactician27062006@gmail.com>
> ---
>  drivers/hid/wacom_sys.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 628372ab818a..dcf2811a5c48 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2619,7 +2619,7 @@ static void wacom_wireless_work(struct work_struct *work)
>
>         wacom_destroy_battery(wacom);
>
> -       if (!usbdev)
> +       if (!usbdev || usbdev->config->desc.bNumInterfaces < 3)
>                 return;
>
>         /* Stylus interface */
> --
> 2.53.0
>
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-30 20:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-30 17:31 [PATCH] HID: wacom: Prevent NULL dereference in wireless work Devarjya Purkayastha
2026-10-30 17:45 ` sashiko-bot
2026-10-30 20:43 ` Jason Gerecke

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.