* [PATCH v2 1/5] HID: bigben_remove: manually unregister leds
2023-01-31 13:08 [PATCH v2 0/5] HID: manually unregister leds on device removal to prevent UAFs Pietro Borrello
@ 2023-01-31 13:08 ` Pietro Borrello
2023-02-09 8:55 ` Benjamin Tissoires
2023-01-31 13:08 ` [PATCH v2 2/5] HID: asus_remove: manually unregister led Pietro Borrello
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Pietro Borrello @ 2023-01-31 13:08 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Pavel Machek,
Lee Jones, Roderick Colenbrander, Sven Eckelmann
Cc: linux-leds, Cristiano Giuffrida, Bos, H.J.,
Jakob Koschel, linux-input, linux-kernel, Jiri Kosina,
Roderick Colenbrander, Pietro Borrello
Unregister the LED controllers before device removal, as
bigben_set_led() may schedule bigben->worker after the structure has
been freed, causing a use-after-free.
Fixes: 4eb1b01de5b9 ("HID: hid-bigbenff: fix race condition for scheduled work during removal")
Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
---
drivers/hid/hid-bigbenff.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/hid/hid-bigbenff.c b/drivers/hid/hid-bigbenff.c
index e8b16665860d..d3201b755595 100644
--- a/drivers/hid/hid-bigbenff.c
+++ b/drivers/hid/hid-bigbenff.c
@@ -306,9 +306,14 @@ static enum led_brightness bigben_get_led(struct led_classdev *led)
static void bigben_remove(struct hid_device *hid)
{
+ int n;
struct bigben_device *bigben = hid_get_drvdata(hid);
bigben->removed = true;
+ for (n = 0; n < NUM_LEDS; n++) {
+ if (bigben->leds[n])
+ devm_led_classdev_unregister(&hid->dev, bigben->leds[n]);
+ }
cancel_work_sync(&bigben->worker);
hid_hw_stop(hid);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/5] HID: bigben_remove: manually unregister leds
2023-01-31 13:08 ` [PATCH v2 1/5] HID: bigben_remove: manually unregister leds Pietro Borrello
@ 2023-02-09 8:55 ` Benjamin Tissoires
2023-02-09 12:45 ` Pietro Borrello
0 siblings, 1 reply; 11+ messages in thread
From: Benjamin Tissoires @ 2023-02-09 8:55 UTC (permalink / raw)
To: Pietro Borrello
Cc: Jiri Kosina, Hanno Zulla, Pavel Machek, Lee Jones,
Roderick Colenbrander, Sven Eckelmann, linux-leds,
Cristiano Giuffrida, Bos, H.J.,
Jakob Koschel, linux-input, linux-kernel, Jiri Kosina,
Roderick Colenbrander
Hi Pietro,
On Jan 31 2023, Pietro Borrello wrote:
> Unregister the LED controllers before device removal, as
> bigben_set_led() may schedule bigben->worker after the structure has
> been freed, causing a use-after-free.
>
> Fixes: 4eb1b01de5b9 ("HID: hid-bigbenff: fix race condition for scheduled work during removal")
> Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
> ---
> drivers/hid/hid-bigbenff.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/hid/hid-bigbenff.c b/drivers/hid/hid-bigbenff.c
> index e8b16665860d..d3201b755595 100644
> --- a/drivers/hid/hid-bigbenff.c
> +++ b/drivers/hid/hid-bigbenff.c
> @@ -306,9 +306,14 @@ static enum led_brightness bigben_get_led(struct led_classdev *led)
>
> static void bigben_remove(struct hid_device *hid)
> {
> + int n;
> struct bigben_device *bigben = hid_get_drvdata(hid);
>
> bigben->removed = true;
> + for (n = 0; n < NUM_LEDS; n++) {
> + if (bigben->leds[n])
> + devm_led_classdev_unregister(&hid->dev, bigben->leds[n]);
> + }
> cancel_work_sync(&bigben->worker);
I don't think this is the correct fix. It would seem that we are
suddenly making the assumption that the devm mechanism would do things
in the wrong order, when the devm_led_classdev_unregister() should be
called *before* the devm_free() of the struct bigben_device.
However, you can trigger a bug, and thus we can analyse a little bit
further what is happening:
* user calls a function on the LED
* bigben_set_led() is called
* .remove() is being called at roughly the same time:
- bigben->removed is set to true
- cancel_work_sync() is called
* at that point, bigben_set_led() can not crash because
led_classdev_unregister() flushes all of its workers, and thus
prevents the call for dev_kfree(struct bigben_device)
* but now bigben_set_led() calls schedule_work()
* led_classdev_unregister() is now done and devm_kfree() is called for
struct bigben_device
* now the led worker kicks in, and tries to access struct bigben_device
and derefences it to get the value of bigben->removed (and
bigben->report), which crashes.
So without your patch, the problem seems to be that we call a
schedule_work *after* we set bigben->removed to true and we call
cancel_work_sync().
And if you look at the hid-playstation driver, you'll see that the
schedule_work() call is encapsulated in a spinlock and a check to
ds->output_worker_initialized.
And this is why you can not reproduce on the hid-playstation driver,
because it is guarded against scheduling a worker when the driver is
being removed.
I think I prefer a lot more the playstation solution: having to manually
call a devm_release_free always feels wrong in a normal path. And also
by doing so, you might paper another problem that might happen on an
error path in probe for instance. Also, this means that the pattern you
saw is specific to some drivers, not all depending on how they make use
of workers.
Would you mind respinning that series with those comments?
Cheers,
Benjamin
> hid_hw_stop(hid);
> }
>
> --
> 2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/5] HID: bigben_remove: manually unregister leds
2023-02-09 8:55 ` Benjamin Tissoires
@ 2023-02-09 12:45 ` Pietro Borrello
0 siblings, 0 replies; 11+ messages in thread
From: Pietro Borrello @ 2023-02-09 12:45 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Hanno Zulla, Pavel Machek, Lee Jones,
Roderick Colenbrander, Sven Eckelmann, linux-leds,
Cristiano Giuffrida, Bos, H.J.,
Jakob Koschel, linux-input, linux-kernel, Jiri Kosina,
Roderick Colenbrander
On Thu, 9 Feb 2023 at 09:55, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> Hi Pietro,
>
> On Jan 31 2023, Pietro Borrello wrote:
> > Unregister the LED controllers before device removal, as
> > bigben_set_led() may schedule bigben->worker after the structure has
> > been freed, causing a use-after-free.
> >
> > Fixes: 4eb1b01de5b9 ("HID: hid-bigbenff: fix race condition for scheduled work during removal")
> > Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
> > ---
> > drivers/hid/hid-bigbenff.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/hid/hid-bigbenff.c b/drivers/hid/hid-bigbenff.c
> > index e8b16665860d..d3201b755595 100644
> > --- a/drivers/hid/hid-bigbenff.c
> > +++ b/drivers/hid/hid-bigbenff.c
> > @@ -306,9 +306,14 @@ static enum led_brightness bigben_get_led(struct led_classdev *led)
> >
> > static void bigben_remove(struct hid_device *hid)
> > {
> > + int n;
> > struct bigben_device *bigben = hid_get_drvdata(hid);
> >
> > bigben->removed = true;
> > + for (n = 0; n < NUM_LEDS; n++) {
> > + if (bigben->leds[n])
> > + devm_led_classdev_unregister(&hid->dev, bigben->leds[n]);
> > + }
> > cancel_work_sync(&bigben->worker);
>
> I don't think this is the correct fix. It would seem that we are
> suddenly making the assumption that the devm mechanism would do things
> in the wrong order, when the devm_led_classdev_unregister() should be
> called *before* the devm_free() of the struct bigben_device.
>
> However, you can trigger a bug, and thus we can analyse a little bit
> further what is happening:
>
> * user calls a function on the LED
> * bigben_set_led() is called
> * .remove() is being called at roughly the same time:
> - bigben->removed is set to true
> - cancel_work_sync() is called
> * at that point, bigben_set_led() can not crash because
> led_classdev_unregister() flushes all of its workers, and thus
> prevents the call for dev_kfree(struct bigben_device)
> * but now bigben_set_led() calls schedule_work()
> * led_classdev_unregister() is now done and devm_kfree() is called for
> struct bigben_device
> * now the led worker kicks in, and tries to access struct bigben_device
> and derefences it to get the value of bigben->removed (and
> bigben->report), which crashes.
>
> So without your patch, the problem seems to be that we call a
> schedule_work *after* we set bigben->removed to true and we call
> cancel_work_sync().
Yes, this matches my intuition of what is happening here.
Thank you for the extensive description.
>
> And if you look at the hid-playstation driver, you'll see that the
> schedule_work() call is encapsulated in a spinlock and a check to
> ds->output_worker_initialized.
>
> And this is why you can not reproduce on the hid-playstation driver,
> because it is guarded against scheduling a worker when the driver is
> being removed.
>
> I think I prefer a lot more the playstation solution: having to manually
> call a devm_release_free always feels wrong in a normal path. And also
> by doing so, you might paper another problem that might happen on an
> error path in probe for instance. Also, this means that the pattern you
> saw is specific to some drivers, not all depending on how they make use
> of workers.
>
Yes, I agree this would be much cleaner.
> Would you mind respinning that series with those comments?
Sure, I'll work on that!
Best regards,
Pietro
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/5] HID: asus_remove: manually unregister led
2023-01-31 13:08 [PATCH v2 0/5] HID: manually unregister leds on device removal to prevent UAFs Pietro Borrello
2023-01-31 13:08 ` [PATCH v2 1/5] HID: bigben_remove: manually unregister leds Pietro Borrello
@ 2023-01-31 13:08 ` Pietro Borrello
2023-01-31 13:08 ` [PATCH v2 3/5] HID: dualsense_remove: manually unregister leds Pietro Borrello
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Pietro Borrello @ 2023-01-31 13:08 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Pavel Machek,
Lee Jones, Roderick Colenbrander, Sven Eckelmann
Cc: linux-leds, Cristiano Giuffrida, Bos, H.J.,
Jakob Koschel, linux-input, linux-kernel, Jiri Kosina,
Roderick Colenbrander, Pietro Borrello
Unregister the LED controller before device removal, as
asus_kbd_backlight_set() may schedule led->work after the structure
has been freed, causing a use-after-free.
Fixes: af22a610bc38 ("HID: asus: support backlight on USB keyboards")
Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
---
drivers/hid/hid-asus.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index f99752b998f3..0f274c8d1bef 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1122,6 +1122,7 @@ static void asus_remove(struct hid_device *hdev)
if (drvdata->kbd_backlight) {
drvdata->kbd_backlight->removed = true;
+ devm_led_classdev_unregister(&hdev->dev, &drvdata->kbd_backlight->cdev);
cancel_work_sync(&drvdata->kbd_backlight->work);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/5] HID: dualsense_remove: manually unregister leds
2023-01-31 13:08 [PATCH v2 0/5] HID: manually unregister leds on device removal to prevent UAFs Pietro Borrello
2023-01-31 13:08 ` [PATCH v2 1/5] HID: bigben_remove: manually unregister leds Pietro Borrello
2023-01-31 13:08 ` [PATCH v2 2/5] HID: asus_remove: manually unregister led Pietro Borrello
@ 2023-01-31 13:08 ` Pietro Borrello
2023-02-09 8:59 ` Benjamin Tissoires
2023-01-31 13:08 ` [PATCH v2 4/5] HID: dualshock4_remove: " Pietro Borrello
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Pietro Borrello @ 2023-01-31 13:08 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Pavel Machek,
Lee Jones, Roderick Colenbrander, Sven Eckelmann
Cc: linux-leds, Cristiano Giuffrida, Bos, H.J.,
Jakob Koschel, linux-input, linux-kernel, Jiri Kosina,
Roderick Colenbrander, Pietro Borrello
Unregister the LED controllers before device removal, to prevent
unnecessary runs of dualsense_player_led_set_brightness().
Fixes: 8c0ab553b072 ("HID: playstation: expose DualSense player LEDs through LED class.")
Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
---
Contrary to the other patches in this series, failing to unregister
the led controller does not results into a use-after-free thanks
to the output_worker_initialized variable and the spinlock checks.
Changes in v2:
- Unregister multicolor led controller
- Clarify UAF
- Link to v1: https://lore.kernel.org/all/20230125-hid-unregister-leds-v1-3-9a5192dcef16@diag.uniroma1.it/
---
drivers/hid/hid-playstation.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 27c40894acab..f23186ca2d76 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -1503,11 +1503,17 @@ static void dualsense_remove(struct ps_device *ps_dev)
{
struct dualsense *ds = container_of(ps_dev, struct dualsense, base);
unsigned long flags;
+ int i;
spin_lock_irqsave(&ds->base.lock, flags);
ds->output_worker_initialized = false;
spin_unlock_irqrestore(&ds->base.lock, flags);
+ for (i = 0; i < ARRAY_SIZE(ds->player_leds); i++)
+ devm_led_classdev_unregister(&ps_dev->hdev->dev, &ds->player_leds[i]);
+
+ devm_led_classdev_multicolor_unregister(&ps_dev->hdev->dev, &ds->lightbar);
+
cancel_work_sync(&ds->output_worker);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/5] HID: dualsense_remove: manually unregister leds
2023-01-31 13:08 ` [PATCH v2 3/5] HID: dualsense_remove: manually unregister leds Pietro Borrello
@ 2023-02-09 8:59 ` Benjamin Tissoires
2023-02-09 12:50 ` Pietro Borrello
0 siblings, 1 reply; 11+ messages in thread
From: Benjamin Tissoires @ 2023-02-09 8:59 UTC (permalink / raw)
To: Pietro Borrello
Cc: Jiri Kosina, Hanno Zulla, Pavel Machek, Lee Jones,
Roderick Colenbrander, Sven Eckelmann, linux-leds,
Cristiano Giuffrida, Bos, H.J.,
Jakob Koschel, linux-input, linux-kernel, Jiri Kosina,
Roderick Colenbrander
On Jan 31 2023, Pietro Borrello wrote:
> Unregister the LED controllers before device removal, to prevent
> unnecessary runs of dualsense_player_led_set_brightness().
>
> Fixes: 8c0ab553b072 ("HID: playstation: expose DualSense player LEDs through LED class.")
> Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
>
> ---
>
> Contrary to the other patches in this series, failing to unregister
> the led controller does not results into a use-after-free thanks
> to the output_worker_initialized variable and the spinlock checks.
And so we don't need that patch (nor for hid-sony.c) because we have a
guard against scheduling a worker job when the device is being removed.
So please drop 3,4,5 from this series, they are just making the code
worse.
Cheers,
Benjamin
>
> Changes in v2:
> - Unregister multicolor led controller
> - Clarify UAF
> - Link to v1: https://lore.kernel.org/all/20230125-hid-unregister-leds-v1-3-9a5192dcef16@diag.uniroma1.it/
> ---
> drivers/hid/hid-playstation.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 27c40894acab..f23186ca2d76 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -1503,11 +1503,17 @@ static void dualsense_remove(struct ps_device *ps_dev)
> {
> struct dualsense *ds = container_of(ps_dev, struct dualsense, base);
> unsigned long flags;
> + int i;
>
> spin_lock_irqsave(&ds->base.lock, flags);
> ds->output_worker_initialized = false;
> spin_unlock_irqrestore(&ds->base.lock, flags);
>
> + for (i = 0; i < ARRAY_SIZE(ds->player_leds); i++)
> + devm_led_classdev_unregister(&ps_dev->hdev->dev, &ds->player_leds[i]);
> +
> + devm_led_classdev_multicolor_unregister(&ps_dev->hdev->dev, &ds->lightbar);
> +
> cancel_work_sync(&ds->output_worker);
> }
>
>
> --
> 2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/5] HID: dualsense_remove: manually unregister leds
2023-02-09 8:59 ` Benjamin Tissoires
@ 2023-02-09 12:50 ` Pietro Borrello
0 siblings, 0 replies; 11+ messages in thread
From: Pietro Borrello @ 2023-02-09 12:50 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Hanno Zulla, Pavel Machek, Lee Jones,
Roderick Colenbrander, Sven Eckelmann, linux-leds,
Cristiano Giuffrida, Bos, H.J.,
Jakob Koschel, linux-input, linux-kernel, Jiri Kosina,
Roderick Colenbrander
On Thu, 9 Feb 2023 at 09:59, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Jan 31 2023, Pietro Borrello wrote:
> > Unregister the LED controllers before device removal, to prevent
> > unnecessary runs of dualsense_player_led_set_brightness().
> >
> > Fixes: 8c0ab553b072 ("HID: playstation: expose DualSense player LEDs through LED class.")
> > Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
> >
> > ---
> >
> > Contrary to the other patches in this series, failing to unregister
> > the led controller does not results into a use-after-free thanks
> > to the output_worker_initialized variable and the spinlock checks.
>
> And so we don't need that patch (nor for hid-sony.c) because we have a
> guard against scheduling a worker job when the device is being removed.
>
> So please drop 3,4,5 from this series, they are just making the code
> worse.
Sure.
I kept them only due to the Roderick Colenbrander's comment, but I'm happy
to remove them.
For reference:
> [...] I don't mind the change as it
> prevents the work scheduling functions to get called unnecessarily.
Link: https://lore.kernel.org/lkml/CAEc3jaCEKfqEJSV4=6GRj1Ry97xH+HwVSeEOZReNwkt=rLNvNQ@mail.gmail.com/
Thanks,
Pietro
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 4/5] HID: dualshock4_remove: manually unregister leds
2023-01-31 13:08 [PATCH v2 0/5] HID: manually unregister leds on device removal to prevent UAFs Pietro Borrello
` (2 preceding siblings ...)
2023-01-31 13:08 ` [PATCH v2 3/5] HID: dualsense_remove: manually unregister leds Pietro Borrello
@ 2023-01-31 13:08 ` Pietro Borrello
2023-01-31 13:08 ` [PATCH v2 5/5] HID: sony_remove: " Pietro Borrello
2023-01-31 17:19 ` [PATCH v2 0/5] HID: manually unregister leds on device removal to prevent UAFs Lee Jones
5 siblings, 0 replies; 11+ messages in thread
From: Pietro Borrello @ 2023-01-31 13:08 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Pavel Machek,
Lee Jones, Roderick Colenbrander, Sven Eckelmann
Cc: linux-leds, Cristiano Giuffrida, Bos, H.J.,
Jakob Koschel, linux-input, linux-kernel, Jiri Kosina,
Roderick Colenbrander, Pietro Borrello
Unregister the LED controllers before device removal, to prevent
unnecessary runs of dualshock4_led_set_brightness().
Fixes: 4521109a8f40 ("HID: playstation: support DualShock4 lightbar.")
Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
---
Contrary to the other patches in this series, failing to unregister
the led controller does not results into a use-after-free thanks
to the output_worker_initialized variable and the spinlock checks.
Changes in v2:
- Clarify UAF
- Link to v1: https://lore.kernel.org/all/20230125-hid-unregister-leds-v1-4-9a5192dcef16@diag.uniroma1.it/
---
drivers/hid/hid-playstation.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index f23186ca2d76..b41657842e26 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -2434,11 +2434,15 @@ static void dualshock4_remove(struct ps_device *ps_dev)
{
struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
unsigned long flags;
+ int i;
spin_lock_irqsave(&ds4->base.lock, flags);
ds4->output_worker_initialized = false;
spin_unlock_irqrestore(&ds4->base.lock, flags);
+ for (i = 0; i < ARRAY_SIZE(ds4->lightbar_leds); i++)
+ devm_led_classdev_unregister(&ps_dev->hdev->dev, &ds4->lightbar_leds[i]);
+
cancel_work_sync(&ds4->output_worker);
if (ps_dev->hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE)
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 5/5] HID: sony_remove: manually unregister leds
2023-01-31 13:08 [PATCH v2 0/5] HID: manually unregister leds on device removal to prevent UAFs Pietro Borrello
` (3 preceding siblings ...)
2023-01-31 13:08 ` [PATCH v2 4/5] HID: dualshock4_remove: " Pietro Borrello
@ 2023-01-31 13:08 ` Pietro Borrello
2023-01-31 17:19 ` [PATCH v2 0/5] HID: manually unregister leds on device removal to prevent UAFs Lee Jones
5 siblings, 0 replies; 11+ messages in thread
From: Pietro Borrello @ 2023-01-31 13:08 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Pavel Machek,
Lee Jones, Roderick Colenbrander, Sven Eckelmann
Cc: linux-leds, Cristiano Giuffrida, Bos, H.J.,
Jakob Koschel, linux-input, linux-kernel, Jiri Kosina,
Roderick Colenbrander, Pietro Borrello
Unregister the LED controller before device removal, as
sony_led_set_brightness() may schedule sc->state_worker
after the structure has been freed, causing a use-after-free.
Fixes: 0a286ef27852 ("HID: sony: Add LED support for Sixaxis/Dualshock3 USB")
Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
Reviewed-by: Sven Eckelmann <sven@narfation.org>
---
drivers/hid/hid-sony.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 13125997ab5e..146677c8319c 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -3083,6 +3083,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
static void sony_remove(struct hid_device *hdev)
{
struct sony_sc *sc = hid_get_drvdata(hdev);
+ int n;
if (sc->quirks & (GHL_GUITAR_PS3WIIU | GHL_GUITAR_PS4)) {
del_timer_sync(&sc->ghl_poke_timer);
@@ -3100,6 +3101,13 @@ static void sony_remove(struct hid_device *hdev)
if (sc->hw_version_created)
device_remove_file(&sc->hdev->dev, &dev_attr_hardware_version);
+ if (sc->quirks & SONY_LED_SUPPORT) {
+ for (n = 0; n < sc->led_count; n++) {
+ if (sc->leds[n])
+ devm_led_classdev_unregister(&hdev->dev, sc->leds[n]);
+ }
+ }
+
sony_cancel_work_sync(sc);
sony_remove_dev_list(sc);
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/5] HID: manually unregister leds on device removal to prevent UAFs
2023-01-31 13:08 [PATCH v2 0/5] HID: manually unregister leds on device removal to prevent UAFs Pietro Borrello
` (4 preceding siblings ...)
2023-01-31 13:08 ` [PATCH v2 5/5] HID: sony_remove: " Pietro Borrello
@ 2023-01-31 17:19 ` Lee Jones
5 siblings, 0 replies; 11+ messages in thread
From: Lee Jones @ 2023-01-31 17:19 UTC (permalink / raw)
To: Pietro Borrello
Cc: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Pavel Machek,
Roderick Colenbrander, Sven Eckelmann, linux-leds,
Cristiano Giuffrida, Bos, H.J.,
Jakob Koschel, linux-input, linux-kernel, Jiri Kosina,
Roderick Colenbrander
> To: Jiri Kosina <jikos@kernel.org>
> To: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> To: Hanno Zulla <kontakt@hanno.de>
> To: Pavel Machek <pavel@ucw.cz>
> To: Lee Jones <lee@kernel.org>
Not entirely sure why I'm receiving these?
> To: Roderick Colenbrander <roderick.colenbrander@sony.com>
> To: Sven Eckelmann <sven@narfation.org>
> Cc: linux-leds@vger.kernel.org
> Cc: Cristiano Giuffrida <c.giuffrida@vu.nl>
> Cc: "Bos, H.J." <h.j.bos@vu.nl>
> Cc: Jakob Koschel <jkl820.git@gmail.com>
> Cc: linux-input@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: Roderick Colenbrander <roderick@gaikai.com>
> Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
>
> ---
> Changes in v2:
> - dualshock4: Clarify UAF
> - dualsense: Clarify UAF
> - dualsense: Unregister multicolor led controller
> - Link to v1: https://lore.kernel.org/r/20230125-hid-unregister-leds-v1-0-9a5192dcef16@diag.uniroma1.it
>
> ---
> Pietro Borrello (5):
> HID: bigben_remove: manually unregister leds
> HID: asus_remove: manually unregister led
> HID: dualsense_remove: manually unregister leds
> HID: dualshock4_remove: manually unregister leds
> HID: sony_remove: manually unregister leds
>
> drivers/hid/hid-asus.c | 1 +
> drivers/hid/hid-bigbenff.c | 5 +++++
> drivers/hid/hid-playstation.c | 10 ++++++++++
> drivers/hid/hid-sony.c | 8 ++++++++
> 4 files changed, 24 insertions(+)
> ---
> base-commit: 2241ab53cbb5cdb08a6b2d4688feb13971058f65
> change-id: 20230125-hid-unregister-leds-4cbf67099e1d
>
> Best regards,
> --
> Pietro Borrello <borrello@diag.uniroma1.it>
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 11+ messages in thread