[PATCH v2 1/2] EDAC/device_sysfs: Reject poll_msec value 0

[PATCH v2 1/2] EDAC/device_sysfs: Reject poll_msec value 0

[Jad Keskes]

The poll_msec store allows writing 0, which sets the workqueue delay
to 0 jiffies. This causes the poll work to spin without any delay,
consuming 100% CPU on the polling kworker.

The comment has said "must be at least one millisecond" since the file
was first added, but no validation was ever enforced. Add a check for
value < 1 and return -EINVAL.

Fixes: e27e3dac6517 ("drivers/edac: add edac_device class")
Signed-off-by: Jad Keskes <inasj268@gmail.com>
---
 drivers/edac/edac_device_sysfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/edac/edac_device_sysfs.c b/drivers/edac/edac_device_sysfs.c
index b1c2717cd023..89adfd3091e0 100644
--- a/drivers/edac/edac_device_sysfs.c
+++ b/drivers/edac/edac_device_sysfs.c
@@ -96,6 +96,9 @@ static ssize_t edac_device_ctl_poll_msec_store(struct edac_device_ctl_info
 	 * and set a new one.
 	 */
 	value = simple_strtoul(data, NULL, 0);
+	if (value < 1)
+		return -EINVAL;
+
 	edac_device_reset_delay_period(ctl_info, value);
 
 	return count;
-- 
2.55.0

[PATCH v2 2/2] EDAC/device: Don't re-arm workqueue during teardown

[Jad Keskes]

edac_device_del_device() stops the workqueue then removes the sysfs
attributes.  However, writing to poll_msec triggers
edac_device_reset_delay_period(), which unconditionally re-arms the
workqueue.  If writes occur during that window, the re-armed work
runs after edac_dev has been freed.

Fix this by checking op_state in reset_delay_period().  del_device()
already sets it to OP_OFFLINE before tearing down the workqueue, so
the new guard causes reset_delay_period() to skip mod_work() when
the device is being removed.

Signed-off-by: Jad Keskes <inasj268@gmail.com>
---
 drivers/edac/edac_device.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index cf0d3c2dfc04..50ae7f1e5152 100644
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -354,6 +354,10 @@ void edac_device_reset_delay_period(struct edac_device_ctl_info *edac_dev,
 	edac_dev->poll_msec = msec;
 	edac_dev->delay	    = msecs_to_jiffies(msec);
 
+	/* Don't re-arm the workqueue if the device is being torn down */
+	if (edac_dev->op_state == OP_OFFLINE)
+		return;
+
 	/* See comment in edac_device_workq_setup() above */
 	if (edac_dev->poll_msec == DEFAULT_POLL_INTERVAL)
 		edac_mod_work(&edac_dev->work, round_jiffies_relative(edac_dev->delay));
-- 
2.55.0

Re: [PATCH v2 1/2] EDAC/device_sysfs: Reject poll_msec value 0

[Borislav Petkov]

On Fri, Jul 10, 2026 at 12:35:47PM +0100, Jad Keskes wrote:
> The poll_msec store allows writing 0, which sets the workqueue delay
> to 0 jiffies. This causes the poll work to spin without any delay,
> consuming 100% CPU on the polling kworker.
> 
> The comment has said "must be at least one millisecond" since the file
> was first added, but no validation was ever enforced. Add a check for
> value < 1 and return -EINVAL.
> 
> Fixes: e27e3dac6517 ("drivers/edac: add edac_device class")
> Signed-off-by: Jad Keskes <inasj268@gmail.com>
> ---
>  drivers/edac/edac_device_sysfs.c | 3 +++
>  1 file changed, 3 insertions(+)

Thanks for those, definitely better.

However, Sashiko found more crud:

https://sashiko.dev/#/patchset/20260710113548.46266-1-inasj268%40gmail.com

That thing in edac_device_ctl_poll_msec_store() should not do unsigned longs
and then stuff them into a uint. I guess we didn't have the kstrtox-etc
helpers back then...

I think you should convert it to kstrtouint() directly and then deal with
the value in the proper range.

And then the other reported issues would be good to address too.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette