From 9fefb6201c4f8dd9f58c581b2a66e5cde2895ea2 Mon Sep 17 00:00:00 2001 From: Pietro Borrello Date: Sun, 12 Feb 2023 18:59:59 +0000 Subject: [PATCH] HID: bigben: use spinlock to protect concurrent accesses bigben driver has a worker that may access data concurrently. Proct the accesses using a spinlock. Fixes: 256a90ed9e46 ("HID: hid-bigbenff: driver for BigBen Interactive PS3OFMINIPAD gamepad") Signed-off-by: Pietro Borrello Link: https://lore.kernel.org/r/20230125-hid-unregister-leds-v4-1-7860c5763c38@diag.uniroma1.it Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-bigbenff.c | 52 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-bigbenff.c b/drivers/hid/hid-bigbenff.c index e8b16665860d..ed3d2d7bc1dd 100644 --- a/drivers/hid/hid-bigbenff.c +++ b/drivers/hid/hid-bigbenff.c @@ -174,6 +174,7 @@ static __u8 pid0902_rdesc_fixed[] = { struct bigben_device { struct hid_device *hid; struct hid_report *report; + spinlock_t lock; bool removed; u8 led_state; /* LED1 = 1 .. LED4 = 8 */ u8 right_motor_on; /* right motor off/on 0/1 */ @@ -190,12 +191,27 @@ static void bigben_worker(struct work_struct *work) struct bigben_device *bigben = container_of(work, struct bigben_device, worker); struct hid_field *report_field = bigben->report->field[0]; + bool do_work_led = false; + bool do_work_ff = false; + u8 *buf; + u32 len; + unsigned long flags; if (bigben->removed || !report_field) return; + buf = hid_alloc_report_buf(bigben->report, GFP_KERNEL); + if (!buf) + return; + + len = hid_report_len(bigben->report); + + /* LED work */ + spin_lock_irqsave(&bigben->lock, flags); + if (bigben->work_led) { bigben->work_led = false; + do_work_led = true; report_field->value[0] = 0x01; /* 1 = led message */ report_field->value[1] = 0x08; /* reserved value, always 8 */ report_field->value[2] = bigben->led_state; @@ -204,11 +220,22 @@ static void bigben_worker(struct work_struct *work) report_field->value[5] = 0x00; /* padding */ report_field->value[6] = 0x00; /* padding */ report_field->value[7] = 0x00; /* padding */ - hid_hw_request(bigben->hid, bigben->report, HID_REQ_SET_REPORT); + hid_output_report(bigben->report, buf); } + spin_unlock_irqrestore(&bigben->lock, flags); + + if (do_work_led) { + hid_hw_raw_request(bigben->hid, bigben->report->id, buf, len, + bigben->report->type, HID_REQ_SET_REPORT); + } + + /* FF work */ + spin_lock_irqsave(&bigben->lock, flags); + if (bigben->work_ff) { bigben->work_ff = false; + do_work_ff = true; report_field->value[0] = 0x02; /* 2 = rumble effect message */ report_field->value[1] = 0x08; /* reserved value, always 8 */ report_field->value[2] = bigben->right_motor_on; @@ -217,8 +244,17 @@ static void bigben_worker(struct work_struct *work) report_field->value[5] = 0x00; /* padding */ report_field->value[6] = 0x00; /* padding */ report_field->value[7] = 0x00; /* padding */ - hid_hw_request(bigben->hid, bigben->report, HID_REQ_SET_REPORT); + hid_output_report(bigben->report, buf); } + + spin_unlock_irqrestore(&bigben->lock, flags); + + if (do_work_ff) { + hid_hw_raw_request(bigben->hid, bigben->report->id, buf, len, + bigben->report->type, HID_REQ_SET_REPORT); + } + + kfree(buf); } static int hid_bigben_play_effect(struct input_dev *dev, void *data, @@ -228,6 +264,7 @@ static int hid_bigben_play_effect(struct input_dev *dev, void *data, struct bigben_device *bigben = hid_get_drvdata(hid); u8 right_motor_on; u8 left_motor_force; + unsigned long flags; if (!bigben) { hid_err(hid, "no device data\n"); @@ -242,9 +279,12 @@ static int hid_bigben_play_effect(struct input_dev *dev, void *data, if (right_motor_on != bigben->right_motor_on || left_motor_force != bigben->left_motor_force) { + spin_lock_irqsave(&bigben->lock, flags); bigben->right_motor_on = right_motor_on; bigben->left_motor_force = left_motor_force; bigben->work_ff = true; + spin_unlock_irqrestore(&bigben->lock, flags); + schedule_work(&bigben->worker); } @@ -259,6 +299,7 @@ static void bigben_set_led(struct led_classdev *led, struct bigben_device *bigben = hid_get_drvdata(hid); int n; bool work; + unsigned long flags; if (!bigben) { hid_err(hid, "no device data\n"); @@ -267,6 +308,7 @@ static void bigben_set_led(struct led_classdev *led, for (n = 0; n < NUM_LEDS; n++) { if (led == bigben->leds[n]) { + spin_lock_irqsave(&bigben->lock, flags); if (value == LED_OFF) { work = (bigben->led_state & BIT(n)); bigben->led_state &= ~BIT(n); @@ -274,6 +316,7 @@ static void bigben_set_led(struct led_classdev *led, work = !(bigben->led_state & BIT(n)); bigben->led_state |= BIT(n); } + spin_unlock_irqrestore(&bigben->lock, flags); if (work) { bigben->work_led = true; @@ -307,8 +350,12 @@ static enum led_brightness bigben_get_led(struct led_classdev *led) static void bigben_remove(struct hid_device *hid) { struct bigben_device *bigben = hid_get_drvdata(hid); + unsigned long flags; + spin_lock_irqsave(&bigben->lock, flags); bigben->removed = true; + spin_unlock_irqrestore(&bigben->lock, flags); + cancel_work_sync(&bigben->worker); hid_hw_stop(hid); } @@ -362,6 +409,7 @@ static int bigben_probe(struct hid_device *hid, set_bit(FF_RUMBLE, hidinput->input->ffbit); INIT_WORK(&bigben->worker, bigben_worker); + spin_lock_init(&bigben->lock); error = input_ff_create_memless(hidinput->input, NULL, hid_bigben_play_effect);