Re: [PATCH 1/2] EDAC/device_sysfs: Use kstrtouint for poll_msec to prevent truncation

Borislav Petkov <[email protected]>
Newsgroups org.kernel.vger.linux-edac,org.kernel.vger.linux-kernel
Message-ID <20260808010138.GAanZ_8rVhhc4HbtyJ@fat_crate.local>
On Thu, Jul 30, 2026 at 03:55:48PM +0100, Jad Keskes wrote:
> The poll_msec sysfs store uses simple_strtoul() which accepts an
> unsigned long, but the target field (poll_msec) is unsigned int. On
> 64-bit systems, a value > UINT_MAX is silently truncated when stored.
> 
> Fix the mismatch by using kstrtouint() instead. This rejects values
> > UINT_MAX at parse time, making truncation impossible. Also add a
> check for value < 1 to reject the 0-delay case, which would cause the
> poll work to spin without delay and consume 100% CPU.
> 
> Fixes: e27e3dac6517 ("drivers/edac: add edac_device class")
> Signed-off-by: Jad Keskes <[email protected]>
> ---
>  drivers/edac/edac_device_sysfs.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)

Applied, thanks.

Also, I did some more trivial cleanups ontop:

Author: Borislav Petkov (AMD) <[email protected]>
Date:   Fri Aug 7 14:16:17 2026 -0700

    EDAC/device_sysfs: Cleanup around edac_device_ctl_poll_msec_store()
    
    - Align function args
    - Fix comment style
    - Fixup formatting around edac_device_reset_delay_period() too
    
    The not-too-trivial change is converting the
    edac_device_reset_delay_period() msec argument to unsigned int as that
    is what the rest of the code expects.
    
    Signed-off-by: Borislav Petkov (AMD) <[email protected]>

diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index cf0d3c2dfc04..638be1f47c59 100644
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -342,14 +342,10 @@ static void edac_device_workq_teardown(struct edac_device_ctl_info *edac_dev)
 }
 
 /*
- * edac_device_reset_delay_period
- *
- *	need to stop any outstanding workq queued up at this time
- *	because we will be resetting the sleep time.
- *	Then restart the workq on the new delay
+ * Stop any outstanding workq queued up at this time because sleep time will
+ * be reset. Then restart the workq on the new delay.
  */
-void edac_device_reset_delay_period(struct edac_device_ctl_info *edac_dev,
-				    unsigned long msec)
+void edac_device_reset_delay_period(struct edac_device_ctl_info *edac_dev, unsigned int msec)
 {
 	edac_dev->poll_msec = msec;
 	edac_dev->delay	    = msecs_to_jiffies(msec);
diff --git a/drivers/edac/edac_device_sysfs.c b/drivers/edac/edac_device_sysfs.c
index 6995ce039db9..6359007701ba 100644
--- a/drivers/edac/edac_device_sysfs.c
+++ b/drivers/edac/edac_device_sysfs.c
@@ -84,17 +84,15 @@ static ssize_t edac_device_ctl_poll_msec_show(struct edac_device_ctl_info
 	return sprintf(data, "%u\n", ctl_info->poll_msec);
 }
 
-static ssize_t edac_device_ctl_poll_msec_store(struct edac_device_ctl_info
-					*ctl_info, const char *data,
-					size_t count)
+static ssize_t edac_device_ctl_poll_msec_store(struct edac_device_ctl_info *ctl_info,
+					       const char *data, size_t count)
 {
 	unsigned int value;
 	int ret;
 
-	/* get the value and enforce that it is non-zero, must be at least
-	 * one millisecond for the delay period, between scans
-	 * Then cancel last outstanding delay for the work request
-	 * and set a new one.
+	/*
+	 * Get the value, make sure it is non-zero, must be at least one millisecond
+	 * for the delay period between scans.
 	 */
 	ret = kstrtouint(data, 0, &value);
 	if (ret < 0)
diff --git a/drivers/edac/edac_module.h b/drivers/edac/edac_module.h
index 47593afdc234..eceef5539186 100644
--- a/drivers/edac/edac_module.h
+++ b/drivers/edac/edac_module.h
@@ -52,8 +52,7 @@ bool edac_queue_work(struct delayed_work *work, unsigned long delay);
 bool edac_stop_work(struct delayed_work *work);
 bool edac_mod_work(struct delayed_work *work, unsigned long delay);
 
-extern void edac_device_reset_delay_period(struct edac_device_ctl_info
-					   *edac_dev, unsigned long msec);
+extern void edac_device_reset_delay_period(struct edac_device_ctl_info *edac_dev, unsigned int msec);
 extern void edac_mc_reset_delay_period(unsigned long value);
 
 /*

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.