[PATCH] EDAC/device: Serialize poll_msec updates against device teardown

[Jad Keskes]

The poll_msec attribute was writable on interrupt-driven controllers,
where the value is meaningless, and a write racing with device removal
could re-arm the polling workqueue after it had been stopped and the
ctl_info freed -- use-after-free.

Restrict the attribute to controllers that are actually polled: _show()
and _store() now return -EPERM unless the controller is in the
OP_RUNNING_POLL state.

That check alone is not sufficient against the teardown race: _store()
can pass it and then be preempted by a concurrent del_device() that sets
OP_OFFLINE, stops the workqueue and frees the ctl_info once the store
completes.  So re-check the state inside reset_delay_period()
under device_ctls_mutex -- the same lock under which del_device()
updates op_state -- making the check atomic with the re-arm.  The mutex
is taken only around the check and re-arm, never across the stop/teardown
path, so it cannot deadlock against other pollers queued on the shared
EDAC workqueue.

Reported-by: Borislav Petkov <bp@xxxxxxxxx>
Signed-off-by: Jad Keskes <inasj268@xxxxxxxxx>
---
 drivers/edac/edac_device.c       | 11 +++++++++++
 drivers/edac/edac_device_sysfs.c |  8 ++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index 361dc985497b..519d1c6d0930 100644
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -392,10 +392,21 @@ static void edac_device_workq_teardown(struct edac_device_ctl_info *edac_dev)
  */
 void edac_device_reset_delay_period(struct edac_device_ctl_info *edac_dev, unsigned int msec)
 {
+	mutex_lock(&device_ctls_mutex);
+
+	/* Don't re-arm the workqueue once teardown has begun or when the
+	 * controller isn't polled; this check must be atomic with the
+	 * re-arm below so it can't race a concurrent del_device().
+	 */
+	if (edac_dev->op_state != OP_RUNNING_POLL)
+		goto out;
+
 	edac_dev->poll_msec = msec;
 	edac_dev->delay     = msecs_to_jiffies(msec);
 
 	edac_mod_work(&edac_dev->work, edac_dev->delay);
+out:
+	mutex_unlock(&device_ctls_mutex);
 }
 
 int edac_device_alloc_index(void)
diff --git a/drivers/edac/edac_device_sysfs.c b/drivers/edac/edac_device_sysfs.c
index e12122b2f42e..821a80806086 100644
--- a/drivers/edac/edac_device_sysfs.c
+++ b/drivers/edac/edac_device_sysfs.c
@@ -83,6 +83,10 @@ static ssize_t edac_device_ctl_panic_on_ue_store(struct edac_device_ctl_info
 static ssize_t edac_device_ctl_poll_msec_show(struct edac_device_ctl_info
 					*ctl_info, char *data)
 {
+	/* Interval is only meaningful while running under polling */
+	if (ctl_info->op_state != OP_RUNNING_POLL)
+		return -EPERM;
+
 	return sprintf(data, "%u\n", ctl_info->poll_msec);
 }
 
@@ -93,6 +97,10 @@ static ssize_t edac_device_ctl_poll_msec_store(struct edac_device_ctl_info
 	unsigned int value;
 	int ret;
 
+	/* Only meaningful when the device is running under polling */
+	if (ctl_info->op_state != OP_RUNNING_POLL)
+		return -EPERM;
+
 	/*
 	 * Get the value, make sure it is non-zero, must be at least one
 	 * millisecond for the delay period between scans.
-- 
2.55.0