[PATCH 08/15] HID: hid-oxp: stop configuration work during teardown

[Andrei Aldea]

Configuration callbacks can queue delayed RGB, button-mapping, and MCU
initialization work. Normal removal previously closed the HID transport
without synchronously stopping that work, while a later configuration
probe failure could leave work queued as devres released its objects.

Track which work items have been initialized, reject new output once
teardown begins, disable initialized work synchronously, and drain an
in-flight transport write before closing the device. Use the same
shutdown path for normal removal and every probe-error unwind after work
can have been exposed.

This also avoids operating on uninitialized work for HID interfaces that
do not own configuration state.

Fixes: 84910c459d65 ("HID: hid-oxp: Add OneXPlayer configuration driver")
Assisted-by: LLM
Reviewed-by: Derek J. Clark <derekjohn.clark@xxxxxxxxx>
Signed-off-by: Andrei Aldea <andrei1998@xxxxxxxxx>
---
 drivers/hid/hid-oxp.c | 94 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 77 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
index 9c22c8e..eef57c3 100644
--- a/drivers/hid/hid-oxp.c
+++ b/drivers/hid/hid-oxp.c
@@ -188,6 +188,9 @@ static struct oxp_hid_cfg {
 	u8 rgb_effect;
 	u8 rgb_speed;
 	u8 rgb_en;
+	bool rgb_work_initialized;
+	bool gen2_work_initialized;
+	bool removing;
 } drvdata;
 
 #define OXP_FILL_PAGE_SLOT(page, btn)            \
@@ -320,7 +323,7 @@ static int oxp_hid_raw_event_gen_1(struct hid_device *hdev,
 	struct led_classdev_mc *led_mc = drvdata.led_mc;
 	struct oxp_gen_1_rgb_report *rgb_rep;
 
-	if (size < sizeof(*rgb_rep))
+	if (size < sizeof(*rgb_rep) || !led_mc)
 		return 0;
 
 	if (data[1] != OXP_FID_GEN1_RGB_REPLY)
@@ -360,6 +363,9 @@ static void oxp_mcu_init_fn(struct work_struct *work)
 	u8 gp_mode_data[3] = { OXP_GP_MODE_DEBUG, 0x01, 0x02 };
 	int ret;
 
+	if (READ_ONCE(drvdata.removing))
+		return;
+
 	/* Re-apply the button mapping */
 	ret = oxp_set_buttons();
 	if (ret)
@@ -406,13 +412,16 @@ static int oxp_hid_raw_event_gen_2(struct hid_device *hdev,
 	 * Re-apply our settings after this has been received.
 	 */
 	if (data[3] == OXP_EFFECT_MONO_TRUE) {
-		mod_delayed_work(system_dfl_wq, &drvdata.oxp_mcu_init, msecs_to_jiffies(50));
+		if (READ_ONCE(drvdata.gen2_work_initialized) &&
+		    !READ_ONCE(drvdata.removing))
+			mod_delayed_work(system_dfl_wq, &drvdata.oxp_mcu_init,
+					 msecs_to_jiffies(50));
 		return 0;
 	}
 
 	if (data[3] != OXP_GET_PROPERTY)
 		return 0;
-	if (size < sizeof(*rgb_rep))
+	if (size < sizeof(*rgb_rep) || !led_mc)
 		return 0;
 
 	rgb_rep = (struct oxp_gen_2_rgb_report *)data;
@@ -449,6 +458,9 @@ static int oxp_hid_raw_event(struct hid_device *hdev, struct hid_report *report,
 {
 	u16 up = get_usage_page(hdev);
 
+	if (!hid_get_drvdata(hdev) || READ_ONCE(drvdata.removing))
+		return 0;
+
 	dev_dbg(&hdev->dev, "raw event data: [%*ph]\n", size, data);
 
 	switch (up) {
@@ -476,6 +488,9 @@ static int mcu_property_out(u8 *header, size_t header_size, u8 *data,
 		return -EINVAL;
 
 	guard(mutex)(&drvdata.cfg_mutex);
+	if (READ_ONCE(drvdata.removing))
+		return -ENODEV;
+
 	memcpy(dmabuf, header, header_size);
 	memcpy(dmabuf + header_size, data, data_size);
 	if (footer_size)
@@ -717,6 +732,9 @@ static void oxp_btn_queue_fn(struct work_struct *work)
 {
 	int ret;
 
+	if (READ_ONCE(drvdata.removing))
+		return;
+
 	ret = oxp_set_buttons();
 	if (ret)
 		dev_err(&drvdata.hdev->dev,
@@ -802,7 +820,9 @@ static ssize_t map_button_store(struct device *dev,
 	default:
 		return -EINVAL;
 	}
-	mod_delayed_work(system_dfl_wq, &drvdata.oxp_btn_queue, msecs_to_jiffies(50));
+	if (!READ_ONCE(drvdata.removing))
+		mod_delayed_work(system_dfl_wq, &drvdata.oxp_btn_queue,
+				 msecs_to_jiffies(50));
 	return count;
 }
 
@@ -1356,6 +1376,9 @@ static void oxp_rgb_queue_fn(struct work_struct *work)
 	u8 val = 4 * brightness / max_brightness;
 	int ret;
 
+	if (READ_ONCE(drvdata.removing))
+		return;
+
 	guard(mutex)(&drvdata.rgb_mutex);
 
 	if (drvdata.rgb_brightness != val) {
@@ -1379,6 +1402,9 @@ static void oxp_rgb_queue_fn(struct work_struct *work)
 static void oxp_rgb_brightness_set(struct led_classdev *led_cdev,
 				   enum led_brightness brightness)
 {
+	if (READ_ONCE(drvdata.removing))
+		return;
+
 	led_cdev->brightness = brightness;
 	mod_delayed_work(system_dfl_wq, &drvdata.oxp_rgb_queue, msecs_to_jiffies(50));
 }
@@ -1480,6 +1506,24 @@ static bool oxp_hybrid_mcu_device(void)
 	return quirks->hybrid_mcu;
 }
 
+static void oxp_drain_output(void)
+{
+	/* Wait for any in-flight sysfs output before closing the transport. */
+	guard(mutex)(&drvdata.cfg_mutex);
+}
+
+static void oxp_quiesce_work(void)
+{
+	WRITE_ONCE(drvdata.removing, true);
+	if (drvdata.rgb_work_initialized)
+		disable_delayed_work_sync(&drvdata.oxp_rgb_queue);
+	if (drvdata.gen2_work_initialized) {
+		disable_delayed_work_sync(&drvdata.oxp_btn_queue);
+		disable_delayed_work_sync(&drvdata.oxp_mcu_init);
+	}
+	oxp_drain_output();
+}
+
 static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
 {
 	struct oxp_bmap_page_1 *bmap_1;
@@ -1490,6 +1534,7 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
 	mutex_init(&drvdata.cfg_mutex);
 	mutex_init(&drvdata.rgb_mutex);
 	drvdata.hdev = hdev;
+	drvdata.removing = false;
 
 	if (up == GEN2_USAGE_PAGE && oxp_hybrid_mcu_device())
 		goto skip_rgb;
@@ -1497,16 +1542,21 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
 	drvdata.led_mc = &oxp_cdev_rgb;
 
 	INIT_DELAYED_WORK(&drvdata.oxp_rgb_queue, oxp_rgb_queue_fn);
+	drvdata.rgb_work_initialized = true;
 	ret = devm_led_classdev_multicolor_register(&hdev->dev, &oxp_cdev_rgb);
-	if (ret)
-		return dev_err_probe(&hdev->dev, ret,
+	if (ret) {
+		dev_err_probe(&hdev->dev, ret,
 				     "Failed to create RGB device\n");
+		goto err_quiesce;
+	}
 
 	ret = devm_device_add_group(drvdata.led_mc->led_cdev.dev,
 				    &oxp_rgb_attr_group);
-	if (ret)
-		return dev_err_probe(drvdata.led_mc->led_cdev.dev, ret,
+	if (ret) {
+		dev_err_probe(drvdata.led_mc->led_cdev.dev, ret,
 				     "Failed to create RGB configuration attributes\n");
+		goto err_quiesce;
+	}
 
 	ret = oxp_rgb_status_show();
 	if (ret)
@@ -1519,14 +1569,18 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
 
 skip_rgb:
 	bmap_1 = devm_kzalloc(&hdev->dev, sizeof(struct oxp_bmap_page_1), GFP_KERNEL);
-	if (!bmap_1)
-		return dev_err_probe(&hdev->dev, -ENOMEM,
+	if (!bmap_1) {
+		ret = dev_err_probe(&hdev->dev, -ENOMEM,
 				     "Unable to allocate button map page 1\n");
+		goto err_quiesce;
+	}
 
 	bmap_2 = devm_kzalloc(&hdev->dev, sizeof(struct oxp_bmap_page_2), GFP_KERNEL);
-	if (!bmap_2)
-		return dev_err_probe(&hdev->dev, -ENOMEM,
+	if (!bmap_2) {
+		ret = dev_err_probe(&hdev->dev, -ENOMEM,
 				     "Unable to allocate button map page 2\n");
+		goto err_quiesce;
+	}
 
 	drvdata.bmap_1 = bmap_1;
 	drvdata.bmap_2 = bmap_2;
@@ -1537,14 +1591,21 @@ skip_rgb:
 	drvdata.rumble_intensity = 5;
 
 	INIT_DELAYED_WORK(&drvdata.oxp_mcu_init, oxp_mcu_init_fn);
+	WRITE_ONCE(drvdata.gen2_work_initialized, true);
 	mod_delayed_work(system_dfl_wq, &drvdata.oxp_mcu_init, msecs_to_jiffies(50));
 
 	ret = devm_device_add_group(&hdev->dev, &oxp_cfg_attrs_group);
-	if (ret)
-		return dev_err_probe(&hdev->dev, ret,
+	if (ret) {
+		dev_err_probe(&hdev->dev, ret,
 				     "Failed to attach configuration attributes\n");
+		goto err_quiesce;
+	}
 
 	return 0;
+
+err_quiesce:
+	oxp_quiesce_work();
+	return ret;
 }
 
 static int oxp_hid_probe(struct hid_device *hdev,
@@ -1587,9 +1648,8 @@ static int oxp_hid_probe(struct hid_device *hdev,
 
 static void oxp_hid_remove(struct hid_device *hdev)
 {
-	cancel_delayed_work(&drvdata.oxp_rgb_queue);
-	cancel_delayed_work(&drvdata.oxp_btn_queue);
-	cancel_delayed_work(&drvdata.oxp_mcu_init);
+	if (hid_get_drvdata(hdev))
+		oxp_quiesce_work();
 	hid_hw_close(hdev);
 	hid_hw_stop(hdev);
 }