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

Myeonghun Pak posted 1 patch 2 weeks, 1 day ago
There is a newer version of this series
drivers/hid/wacom_sys.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
[PATCH] HID: wacom: stop hardware after post-start probe failures
Posted by Myeonghun Pak 2 weeks, 1 day ago
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
Posted by Dmitry Torokhov 1 week, 5 days ago
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
Posted by Jiri Kosina 5 days, 11 hours ago
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
Posted by Myeonghun Pak 4 days, 19 hours ago
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
Posted by Myeonghun Pak 4 days, 18 hours ago
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