[PATCH] HID: sensor-custom: Synchronize misc-device teardown with file lifetime

[PATCH] HID: sensor-custom: Synchronize misc-device teardown with file lifetime

[Srinivas Pandruvada]

The HID sensor custom misc device frees its per-sensor state in
remove() while userspace may still hold an open file descriptor. A
reader blocked in read() can remain queued on sensor_inst->wait after the
object has been freed, and a later wakeup races with teardown and
touches stale memory.

Synchronize the misc-device teardown with file lifetime before freeing
the per-sensor state. Set a removal flag, reject new opens, wake
blocked waiters, and wait for the last open file to close before
deregistering the misc device and freeing the FIFO. Fail in-flight read()
and poll() operations cleanly once removal is in progress so userspace
cannot access freed memory after the device disappears.

This avoids use-after-free on read(), poll(), release(), and open()
after the HID sensor custom character device has been removed

Reported-by: Семён Марьясин <simeon@maryasin.name>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/hid-sensor-custom.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
index c2b425afd951..c9338ae47050 100644
--- a/drivers/hid/hid-sensor-custom.c
+++ b/drivers/hid/hid-sensor-custom.c
@@ -53,6 +53,14 @@ struct hid_sensor_custom {
 	struct kfifo data_fifo;
 	unsigned long misc_opened;
 	wait_queue_head_t wait;
+	wait_queue_head_t remove_wait;
+	/*
+	 * Shared with the file operations; use READ_ONCE()/WRITE_ONCE() for the
+	 * teardown state. The actual lifetime guarantee comes from misc_opened:
+	 * removal waits for the last fd to close before deregistering the misc
+	 * device and freeing the FIFO.
+	 */
+	bool removing;
 	struct platform_device *custom_pdev;
 };
 
@@ -653,6 +661,9 @@ static ssize_t hid_sensor_custom_read(struct file *file, char __user *buf,
 	sensor_inst = container_of(file->private_data,
 				   struct hid_sensor_custom, custom_dev);
 
+	if (READ_ONCE(sensor_inst->removing))
+		return -ENODEV;
+
 	if (count < sizeof(struct hid_sensor_sample))
 		return -EINVAL;
 
@@ -662,9 +673,12 @@ static ssize_t hid_sensor_custom_read(struct file *file, char __user *buf,
 				return -EAGAIN;
 
 			ret = wait_event_interruptible(sensor_inst->wait,
+				READ_ONCE(sensor_inst->removing) ||
 				!kfifo_is_empty(&sensor_inst->data_fifo));
 			if (ret)
 				return ret;
+			if (READ_ONCE(sensor_inst->removing))
+				return -ENODEV;
 		}
 		ret = kfifo_to_user(&sensor_inst->data_fifo, buf, count,
 				    &copied);
@@ -684,6 +698,7 @@ static int hid_sensor_custom_release(struct inode *inode, struct file *file)
 				   struct hid_sensor_custom, custom_dev);
 
 	clear_bit(0, &sensor_inst->misc_opened);
+	wake_up_all(&sensor_inst->remove_wait);
 
 	return 0;
 }
@@ -694,6 +709,8 @@ static int hid_sensor_custom_open(struct inode *inode, struct file *file)
 
 	sensor_inst = container_of(file->private_data,
 				   struct hid_sensor_custom, custom_dev);
+	if (READ_ONCE(sensor_inst->removing))
+		return -ENODEV;
 	/* We essentially have single reader and writer */
 	if (test_and_set_bit(0, &sensor_inst->misc_opened))
 		return -EBUSY;
@@ -712,6 +729,9 @@ static __poll_t hid_sensor_custom_poll(struct file *file,
 
 	poll_wait(file, &sensor_inst->wait, wait);
 
+	if (READ_ONCE(sensor_inst->removing))
+		return EPOLLERR;
+
 	if (!kfifo_is_empty(&sensor_inst->data_fifo))
 		mask = EPOLLIN | EPOLLRDNORM;
 
@@ -736,6 +756,7 @@ static int hid_sensor_custom_dev_if_add(struct hid_sensor_custom *sensor_inst)
 		return ret;
 
 	init_waitqueue_head(&sensor_inst->wait);
+	init_waitqueue_head(&sensor_inst->remove_wait);
 
 	sensor_inst->custom_dev.minor = MISC_DYNAMIC_MINOR;
 	sensor_inst->custom_dev.name = dev_name(&sensor_inst->pdev->dev);
@@ -751,10 +772,12 @@ static int hid_sensor_custom_dev_if_add(struct hid_sensor_custom *sensor_inst)
 static void hid_sensor_custom_dev_if_remove(struct hid_sensor_custom
 								*sensor_inst)
 {
-	wake_up(&sensor_inst->wait);
+	WRITE_ONCE(sensor_inst->removing, true);
+	wake_up_all(&sensor_inst->wait);
+	wait_event(sensor_inst->remove_wait,
+		   !test_bit(0, &sensor_inst->misc_opened));
 	misc_deregister(&sensor_inst->custom_dev);
 	kfifo_free(&sensor_inst->data_fifo);
-
 }
 
 /*
-- 
2.56.0