All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 6.1 1/2] HID: asus: use spinlock to protect concurrent accesses
@ 2023-03-01 22:38 Stefan Ghinea
  2023-03-01 22:38 ` [PATCH 6.1 2/2] HID: asus: use spinlock to safely schedule workers Stefan Ghinea
  2023-03-03 15:46 ` [PATCH 6.1 1/2] HID: asus: use spinlock to protect concurrent accesses Greg KH
  0 siblings, 2 replies; 3+ messages in thread
From: Stefan Ghinea @ 2023-03-01 22:38 UTC (permalink / raw)
  To: stable; +Cc: Pietro Borrello, Benjamin Tissoires, Stefan Ghinea

From: Pietro Borrello <borrello@diag.uniroma1.it>

commit 315c537068a13f0b5681d33dd045a912f4bece6f upstream

asus driver has a worker that may access data concurrently.
Proct the accesses using a spinlock.

Fixes: af22a610bc38 ("HID: asus: support backlight on USB keyboards")
Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
Link: https://lore.kernel.org/r/20230125-hid-unregister-leds-v4-4-7860c5763c38@diag.uniroma1.it
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Stefan Ghinea <stefan.ghinea@windriver.com>
---
 drivers/hid/hid-asus.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index f99752b998f3..9f767baf39fb 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -98,6 +98,7 @@ struct asus_kbd_leds {
 	struct hid_device *hdev;
 	struct work_struct work;
 	unsigned int brightness;
+	spinlock_t lock;
 	bool removed;
 };
 
@@ -495,7 +496,12 @@ static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
 {
 	struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
 						 cdev);
+	unsigned long flags;
+
+	spin_lock_irqsave(&led->lock, flags);
 	led->brightness = brightness;
+	spin_unlock_irqrestore(&led->lock, flags);
+
 	schedule_work(&led->work);
 }
 
@@ -503,8 +509,14 @@ static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
 {
 	struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
 						 cdev);
+	enum led_brightness brightness;
+	unsigned long flags;
 
-	return led->brightness;
+	spin_lock_irqsave(&led->lock, flags);
+	brightness = led->brightness;
+	spin_unlock_irqrestore(&led->lock, flags);
+
+	return brightness;
 }
 
 static void asus_kbd_backlight_work(struct work_struct *work)
@@ -512,11 +524,14 @@ static void asus_kbd_backlight_work(struct work_struct *work)
 	struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
 	u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
 	int ret;
+	unsigned long flags;
 
 	if (led->removed)
 		return;
 
+	spin_lock_irqsave(&led->lock, flags);
 	buf[4] = led->brightness;
+	spin_unlock_irqrestore(&led->lock, flags);
 
 	ret = asus_kbd_set_report(led->hdev, buf, sizeof(buf));
 	if (ret < 0)
@@ -584,6 +599,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
 	drvdata->kbd_backlight->cdev.brightness_set = asus_kbd_backlight_set;
 	drvdata->kbd_backlight->cdev.brightness_get = asus_kbd_backlight_get;
 	INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work);
+	spin_lock_init(&drvdata->kbd_backlight->lock);
 
 	ret = devm_led_classdev_register(&hdev->dev, &drvdata->kbd_backlight->cdev);
 	if (ret < 0) {
@@ -1119,9 +1135,13 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 static void asus_remove(struct hid_device *hdev)
 {
 	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+	unsigned long flags;
 
 	if (drvdata->kbd_backlight) {
+		spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags);
 		drvdata->kbd_backlight->removed = true;
+		spin_unlock_irqrestore(&drvdata->kbd_backlight->lock, flags);
+
 		cancel_work_sync(&drvdata->kbd_backlight->work);
 	}
 
-- 
2.39.1


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

* [PATCH 6.1 2/2] HID: asus: use spinlock to safely schedule workers
  2023-03-01 22:38 [PATCH 6.1 1/2] HID: asus: use spinlock to protect concurrent accesses Stefan Ghinea
@ 2023-03-01 22:38 ` Stefan Ghinea
  2023-03-03 15:46 ` [PATCH 6.1 1/2] HID: asus: use spinlock to protect concurrent accesses Greg KH
  1 sibling, 0 replies; 3+ messages in thread
From: Stefan Ghinea @ 2023-03-01 22:38 UTC (permalink / raw)
  To: stable; +Cc: Pietro Borrello, Benjamin Tissoires, Stefan Ghinea

From: Pietro Borrello <borrello@diag.uniroma1.it>

commit 4ab3a086d10eeec1424f2e8a968827a6336203df upstream

Use spinlocks to deal with workers introducing a wrapper
asus_schedule_work(), and several spinlock checks.
Otherwise, 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>
Link: https://lore.kernel.org/r/20230125-hid-unregister-leds-v4-5-7860c5763c38@diag.uniroma1.it
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Stefan Ghinea <stefan.ghinea@windriver.com>
---
 drivers/hid/hid-asus.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 9f767baf39fb..d1094bb1aa42 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -491,6 +491,16 @@ static int rog_nkey_led_init(struct hid_device *hdev)
 	return ret;
 }
 
+static void asus_schedule_work(struct asus_kbd_leds *led)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&led->lock, flags);
+	if (!led->removed)
+		schedule_work(&led->work);
+	spin_unlock_irqrestore(&led->lock, flags);
+}
+
 static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
 				   enum led_brightness brightness)
 {
@@ -502,7 +512,7 @@ static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
 	led->brightness = brightness;
 	spin_unlock_irqrestore(&led->lock, flags);
 
-	schedule_work(&led->work);
+	asus_schedule_work(led);
 }
 
 static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
@@ -526,9 +536,6 @@ static void asus_kbd_backlight_work(struct work_struct *work)
 	int ret;
 	unsigned long flags;
 
-	if (led->removed)
-		return;
-
 	spin_lock_irqsave(&led->lock, flags);
 	buf[4] = led->brightness;
 	spin_unlock_irqrestore(&led->lock, flags);
-- 
2.39.1


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

* Re: [PATCH 6.1 1/2] HID: asus: use spinlock to protect concurrent accesses
  2023-03-01 22:38 [PATCH 6.1 1/2] HID: asus: use spinlock to protect concurrent accesses Stefan Ghinea
  2023-03-01 22:38 ` [PATCH 6.1 2/2] HID: asus: use spinlock to safely schedule workers Stefan Ghinea
@ 2023-03-03 15:46 ` Greg KH
  1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2023-03-03 15:46 UTC (permalink / raw)
  To: Stefan Ghinea; +Cc: stable, Pietro Borrello, Benjamin Tissoires

On Thu, Mar 02, 2023 at 12:38:52AM +0200, Stefan Ghinea wrote:
> From: Pietro Borrello <borrello@diag.uniroma1.it>
> 
> commit 315c537068a13f0b5681d33dd045a912f4bece6f upstream
> 
> asus driver has a worker that may access data concurrently.
> Proct the accesses using a spinlock.
> 
> Fixes: af22a610bc38 ("HID: asus: support backlight on USB keyboards")
> Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
> Link: https://lore.kernel.org/r/20230125-hid-unregister-leds-v4-4-7860c5763c38@diag.uniroma1.it
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Stefan Ghinea <stefan.ghinea@windriver.com>
> ---
>  drivers/hid/hid-asus.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)

What about 6.2.y as well?

Why skip that?

thanks,

greg k-h

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

end of thread, other threads:[~2023-03-03 15:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-01 22:38 [PATCH 6.1 1/2] HID: asus: use spinlock to protect concurrent accesses Stefan Ghinea
2023-03-01 22:38 ` [PATCH 6.1 2/2] HID: asus: use spinlock to safely schedule workers Stefan Ghinea
2023-03-03 15:46 ` [PATCH 6.1 1/2] HID: asus: use spinlock to protect concurrent accesses Greg KH

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.