[PATCH] HID: wacom: stop hardware after post-start probe failures

[PATCH] HID: wacom: stop hardware after post-start probe failures

[Myeonghun Pak]

wacom_parse_and_register() starts HID hardware before registering inputs
and initializing pad LEDs/remotes. Those later steps can fail, but their
error paths currently release Wacom resources without stopping the HID
hardware.

Route post-hid_hw_start() failures through hid_hw_stop() before
releasing driver resources.

This issue was identified during our ongoing static-analysis research while
reviewing kernel code.

Fixes: c1d6708bf0d3 ("HID: wacom: Do not register input devices until after hid_hw_start")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/hid/wacom_sys.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0d1c6d90fe..c824d9c224 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2456,16 +2456,16 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 
 	error = wacom_register_inputs(wacom);
 	if (error)
-		goto fail;
+		goto fail_hw_stop;
 
 	if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD) {
 		error = wacom_initialize_leds(wacom);
 		if (error)
-			goto fail;
+			goto fail_hw_stop;
 
 		error = wacom_initialize_remotes(wacom);
 		if (error)
-			goto fail;
+			goto fail_hw_stop;
 	}
 
 	if (!wireless) {
@@ -2496,6 +2496,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 	return 0;
 
 fail_quirks:
+fail_hw_stop:
 	hid_hw_stop(hdev);
 fail:
 	wacom_release_resources(wacom);
-- 
2.47.1

Re: [PATCH] HID: wacom: stop hardware after post-start probe failures

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The error path in `wacom_parse_and_register()` fails to cancel asynchronous works and timers, leading to a Use-After-Free when the `wacom` structure is freed.
- [High] Dismantling software state before the hardware is stopped on failure paths creates race conditions with active interrupt handlers, leading to a NULL pointer dereference.
- [High] Releasing driver resources before stopping HID hardware in `wacom_wireless_work()` leads to a Use-After-Free on incoming events.
--

commit 6396bf377f7e81b67088a1a9500d030c3776a230
Author: Myeonghun Pak <mhun512@gmail.com>

HID: wacom: stop hardware after post-start probe failures

This commit modifies wacom_parse_and_register() to route post-hid_hw_start()
failures through hid_hw_stop() before releasing driver resources.

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index a32320b351e3e..76dc833f5f2e9 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c

[ ... ]

> @@ -2456,16 +2456,16 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
>  
>  	error = wacom_register_inputs(wacom);
>  	if (error)
> -		goto fail;
> +		goto fail_hw_stop;

[Severity: High]
This is a pre-existing issue, but does setting the input pointers to NULL
before stopping the hardware create a race condition?

When wacom_register_inputs() fails, its error path sets wacom_wac->pad_input
to NULL. However, because hid_hw_stop() hasn't been called yet, the
hardware can still generate interrupts:

wacom_parse_and_register()
  wacom_register_inputs()
    fail:
      wacom_wac->pad_input = NULL;
      return error;
  goto fail_hw_stop;

If an interrupt arrives before hid_hw_stop() completes, will the interrupt
handler dereference the NULL pad_input pointer?

>  
>  	if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD) {
>  		error = wacom_initialize_leds(wacom);
>  		if (error)
> -			goto fail;
> +			goto fail_hw_stop;
>  
>  		error = wacom_initialize_remotes(wacom);
>  		if (error)
> -			goto fail;
> +			goto fail_hw_stop;
>  	}
>  
>  	if (!wireless) {

[ ... ]

> @@ -2496,6 +2496,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
>  	return 0;
>  
>  fail_quirks:
> +fail_hw_stop:
>  	hid_hw_stop(hdev);

[Severity: High]
This is a pre-existing issue, but does this error path leave pending
asynchronous works and timers running?

While hid_hw_stop() stops new interrupts from the hardware, works like
wireless_work, battery_work, remote_work, or timers that were already
scheduled while the hardware was active might still be pending. 

Because wacom_probe() will propagate this error and trigger devres cleanup
to free the wacom structure, could these pending works execute later and
attempt to dereference the freed wacom pointer?

>  fail:
>  	wacom_release_resources(wacom);

[Severity: High]
This is a pre-existing issue, but does releasing resources before
stopping the hardware in other code paths create a similar race
condition?

For example, in wacom_wireless_work(), wacom_release_resources() is
called before stopping the HID hardware. This frees resources like
pen_fifo via devres_release_group().

If an incoming HID report arrives after devres_release_group() but
before the hardware is stopped, could wacom_raw_event() attempt to
push data to the freed pen_fifo via wacom_wac_queue_insert(), causing
a use-after-free?

>  	return error;
>  }

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

Re: [PATCH] HID: wacom: stop hardware after post-start probe failures

[Dmitry Torokhov]

Hi Myeonghun,

On Mon, May 25, 2026 at 02:53:33AM +0900, Myeonghun Pak wrote:
> wacom_parse_and_register() starts HID hardware before registering inputs
> and initializing pad LEDs/remotes. Those later steps can fail, but their
> error paths currently release Wacom resources without stopping the HID
> hardware.
> 
> Route post-hid_hw_start() failures through hid_hw_stop() before
> releasing driver resources.
> 
> This issue was identified during our ongoing static-analysis research while
> reviewing kernel code.
> 
> Fixes: c1d6708bf0d3 ("HID: wacom: Do not register input devices until after hid_hw_start")
> Cc: stable@vger.kernel.org
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---
>  drivers/hid/wacom_sys.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 0d1c6d90fe..c824d9c224 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2456,16 +2456,16 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
>  
>  	error = wacom_register_inputs(wacom);
>  	if (error)
> -		goto fail;
> +		goto fail_hw_stop;
>  
>  	if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD) {
>  		error = wacom_initialize_leds(wacom);
>  		if (error)
> -			goto fail;
> +			goto fail_hw_stop;
>  
>  		error = wacom_initialize_remotes(wacom);
>  		if (error)
> -			goto fail;
> +			goto fail_hw_stop;
>  	}
>  
>  	if (!wireless) {
> @@ -2496,6 +2496,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
>  	return 0;
>  
>  fail_quirks:
> +fail_hw_stop:
>  	hid_hw_stop(hdev);
>  fail:
>  	wacom_release_resources(wacom);

I'd get rid of 'fail_quirks' and use 'fail_hw_stop' everywhere,
otherwise:

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.

-- 
Dmitry

Re: [PATCH] HID: wacom: stop hardware after post-start probe failures

[Jiri Kosina]

On Wed, 27 May 2026, Dmitry Torokhov wrote:

> > wacom_parse_and_register() starts HID hardware before registering inputs
> > and initializing pad LEDs/remotes. Those later steps can fail, but their
> > error paths currently release Wacom resources without stopping the HID
> > hardware.
> > 
> > Route post-hid_hw_start() failures through hid_hw_stop() before
> > releasing driver resources.
> > 
> > This issue was identified during our ongoing static-analysis research while
> > reviewing kernel code.
> > 
> > Fixes: c1d6708bf0d3 ("HID: wacom: Do not register input devices until after hid_hw_start")
> > Cc: stable@vger.kernel.org
> > Co-developed-by: Ijae Kim <ae878000@gmail.com>
> > Signed-off-by: Ijae Kim <ae878000@gmail.com>
> > Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> > ---
> >  drivers/hid/wacom_sys.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > index 0d1c6d90fe..c824d9c224 100644
> > --- a/drivers/hid/wacom_sys.c
> > +++ b/drivers/hid/wacom_sys.c
> > @@ -2456,16 +2456,16 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> >  
> >  	error = wacom_register_inputs(wacom);
> >  	if (error)
> > -		goto fail;
> > +		goto fail_hw_stop;
> >  
> >  	if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD) {
> >  		error = wacom_initialize_leds(wacom);
> >  		if (error)
> > -			goto fail;
> > +			goto fail_hw_stop;
> >  
> >  		error = wacom_initialize_remotes(wacom);
> >  		if (error)
> > -			goto fail;
> > +			goto fail_hw_stop;
> >  	}
> >  
> >  	if (!wireless) {
> > @@ -2496,6 +2496,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> >  	return 0;
> >  
> >  fail_quirks:
> > +fail_hw_stop:
> >  	hid_hw_stop(hdev);
> >  fail:
> >  	wacom_release_resources(wacom);
> 
> I'd get rid of 'fail_quirks' and use 'fail_hw_stop' everywhere,

Agreed. Myeonghun, will you send v2 please?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: wacom: stop hardware after post-start probe failures

[Myeonghun Pak]

Hi Jiri and Dmitry,

Thank you both for the review. I agree with Dmitry's suggestion to
remove 'fail_quirks' and use 'fail_hw_stop' throughout the probe path.
I will send out v2 with this change shortly.

Best regards,
Myeonghun Pak

2026년 6월 3일 (수) 오후 9:23, Jiri Kosina <jikos@kernel.org>님이 작성:
>
> On Wed, 27 May 2026, Dmitry Torokhov wrote:
>
> > > wacom_parse_and_register() starts HID hardware before registering inputs
> > > and initializing pad LEDs/remotes. Those later steps can fail, but their
> > > error paths currently release Wacom resources without stopping the HID
> > > hardware.
> > >
> > > Route post-hid_hw_start() failures through hid_hw_stop() before
> > > releasing driver resources.
> > >
> > > This issue was identified during our ongoing static-analysis research while
> > > reviewing kernel code.
> > >
> > > Fixes: c1d6708bf0d3 ("HID: wacom: Do not register input devices until after hid_hw_start")
> > > Cc: stable@vger.kernel.org
> > > Co-developed-by: Ijae Kim <ae878000@gmail.com>
> > > Signed-off-by: Ijae Kim <ae878000@gmail.com>
> > > Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> > > ---
> > >  drivers/hid/wacom_sys.c | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > > index 0d1c6d90fe..c824d9c224 100644
> > > --- a/drivers/hid/wacom_sys.c
> > > +++ b/drivers/hid/wacom_sys.c
> > > @@ -2456,16 +2456,16 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> > >
> > >     error = wacom_register_inputs(wacom);
> > >     if (error)
> > > -           goto fail;
> > > +           goto fail_hw_stop;
> > >
> > >     if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD) {
> > >             error = wacom_initialize_leds(wacom);
> > >             if (error)
> > > -                   goto fail;
> > > +                   goto fail_hw_stop;
> > >
> > >             error = wacom_initialize_remotes(wacom);
> > >             if (error)
> > > -                   goto fail;
> > > +                   goto fail_hw_stop;
> > >     }
> > >
> > >     if (!wireless) {
> > > @@ -2496,6 +2496,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> > >     return 0;
> > >
> > >  fail_quirks:
> > > +fail_hw_stop:
> > >     hid_hw_stop(hdev);
> > >  fail:
> > >     wacom_release_resources(wacom);
> >
> > I'd get rid of 'fail_quirks' and use 'fail_hw_stop' everywhere,
>
> Agreed. Myeonghun, will you send v2 please?
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

[PATCH v2] HID: wacom: stop hardware after post-start probe failures

[Myeonghun Pak]

wacom_parse_and_register() starts HID hardware before registering inputs
and initializing pad LEDs/remotes. Those later steps can fail, but their
error paths currently release Wacom resources without stopping the HID
hardware.

Route post-hid_hw_start() failures through hid_hw_stop() before
releasing driver resources.

This issue was identified during our ongoing static-analysis research while
reviewing kernel code.

Fixes: c1d6708bf0d3 ("HID: wacom: Do not register input devices until after hid_hw_start")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Changes in v2:
- Drop fail_quirks and use fail_hw_stop for every post-hid_hw_start()
  failure path, as suggested by Dmitry.

 drivers/hid/wacom_sys.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0d1c6d90f..ee53186e0 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2456,16 +2456,16 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 
 	error = wacom_register_inputs(wacom);
 	if (error)
-		goto fail;
+		goto fail_hw_stop;
 
 	if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD) {
 		error = wacom_initialize_leds(wacom);
 		if (error)
-			goto fail;
+			goto fail_hw_stop;
 
 		error = wacom_initialize_remotes(wacom);
 		if (error)
-			goto fail;
+			goto fail_hw_stop;
 	}
 
 	if (!wireless) {
@@ -2479,14 +2479,14 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 		cancel_delayed_work_sync(&wacom->init_work);
 		_wacom_query_tablet_data(wacom);
 		error = -ENODEV;
-		goto fail_quirks;
+		goto fail_hw_stop;
 	}
 
 	if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR) {
 		error = hid_hw_open(hdev);
 		if (error) {
 			hid_err(hdev, "hw open failed\n");
-			goto fail_quirks;
+			goto fail_hw_stop;
 		}
 	}
 
@@ -2495,7 +2495,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 	return 0;
 
-fail_quirks:
+fail_hw_stop:
 	hid_hw_stop(hdev);
 fail:
 	wacom_release_resources(wacom);
 	return error;
-- 
2.47.1

Re: [PATCH v2] HID: wacom: stop hardware after post-start probe failures

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Failing to cancel scheduled delayed works and timers on the probe error path leads to a Use-After-Free when the `wacom` structure is freed.
- [High] Nullifying input pointers in `wacom_register_inputs` before stopping the hardware leaves a race window where incoming interrupts can cause a NULL pointer dereference.
- [High] Hardware interrupts can arrive before `wacom->remote` is allocated, causing a NULL pointer dereference in the IRQ handlers.
--

commit 517a367ac93aa0d6c6470c3a21cdb477449345e3
Author: Myeonghun Pak <mhun512@gmail.com>
Subject: HID: wacom: stop hardware after post-start probe failures

This patch routes post-hid_hw_start() failures in wacom_parse_and_register()
through hid_hw_stop() before releasing driver resources. This ensures that the
HID hardware is properly stopped before the wacom resources are freed.

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 2220168bf1164..252771dcfd8f8 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2462,16 +2462,16 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
>  
>  	error = wacom_register_inputs(wacom);
>  	if (error)
> -		goto fail;
> +		goto fail_hw_stop;

[Severity: High]
This is a pre-existing issue, but does this leave a small race window where
an interrupt could cause a NULL pointer dereference?

If wacom_register_inputs() fails, its error path sets the input pointers to
NULL:

    fail:
        wacom_wac->pad_input = NULL;
        wacom_wac->touch_input = NULL;
        wacom_wac->pen_input = NULL;
        return error;

The code then jumps to fail_hw_stop. Since hid_hw_start() has already been
called, the hardware is active. Could an interrupt arrive before hid_hw_stop()
completes, triggering an IRQ handler like wacom_intuos_irq() which
dereferences wacom->pen_input without checking for NULL?

>  
>  	if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD) {
>  		error = wacom_initialize_leds(wacom);
>  		if (error)
> -			goto fail;
> +			goto fail_hw_stop;
>  
>  		error = wacom_initialize_remotes(wacom);
>  		if (error)
> -			goto fail;
> +			goto fail_hw_stop;

[Severity: High]
This is a pre-existing issue, but I have a question about how this handles
interrupts arriving before initialization is complete.

Since hid_hw_start() is called prior to wacom_initialize_remotes():

    error = hid_hw_start(hdev, connect_mask);
    ...
    error = wacom_initialize_remotes(wacom);

Could an early hardware event or a malicious USB report arrive and trigger
wacom_remote_irq() or wacom_remote_status_irq()? Those handlers appear to
dereference wacom->remote to acquire a spinlock:

    spin_lock_irqsave(&remote->remote_lock, flags);

If the interrupt arrives before wacom_initialize_remotes() has allocated
wacom->remote, would this lead to a NULL pointer dereference?

[ ... ]

> @@ -2501,7 +2501,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
>  
>  	return 0;
>  
> -fail_quirks:
> +fail_hw_stop:
>  	hid_hw_stop(hdev);
>  fail:
>  	wacom_release_resources(wacom);

[Severity: High]
This isn't a bug introduced by this patch, but it seems there might be a
pre-existing issue with pending works on the error path.

If wacom_query_tablet_data() is called earlier:

    wacom_query_tablet_data(struct wacom *wacom)
    {
        schedule_delayed_work(&wacom->init_work, msecs_to_jiffies(1000));
    }

And then a subsequent step like hid_hw_open() fails, we jump to fail_hw_stop
which stops the hardware and releases resources:

    fail_hw_stop:
        hid_hw_stop(hdev);
    fail:
        wacom_release_resources(wacom);
        return error;

Since the error is returned to wacom_probe() which can cause the devres core
to free the wacom structure, could the uncancelled wacom->init_work (or other
works/timers scheduled by interrupts) execute later and dereference the freed
wacom memory?

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