[PATCH] auxdisplay: linedisp: serialize sysfs message updates

Laxman Acharya Padhya posted 1 patch 40 minutes ago
drivers/auxdisplay/line-display.c | 7 +++++++
drivers/auxdisplay/line-display.h | 3 +++
2 files changed, 10 insertions(+)
[PATCH] auxdisplay: linedisp: serialize sysfs message updates
Posted by Laxman Acharya Padhya 40 minutes ago
message_store() frees and replaces linedisp->message, restarts the
scroll timer, and may immediately read the new buffer again to update the
display. message_show() and scroll_step_ms_store() can run concurrently
through separate sysfs opens, because kernfs only serializes operations per
open file.

That leaves the display state vulnerable to concurrent readers and writers.
A racing write can free a message buffer that another sysfs write is still
copying from, and a sysfs read can format a message pointer while another
thread frees and replaces it.

Serialize sysfs access to the display state with a mutex. Keep the timer
callback lockless, and use timer_delete_sync() under the mutex so sysfs
writers wait for any in-flight scroll callback before changing the
message, scroll position, or scroll rate.

Fixes: 7e76aece6f03 ("auxdisplay: Extract character line display core support")
Cc: stable@vger.kernel.org
Signed-off-by: Laxman Acharya Padhya <acharyalaxman8848@gmail.com>
---
 drivers/auxdisplay/line-display.c | 7 +++++++
 drivers/auxdisplay/line-display.h | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index 915eb5cd9..fbac10668 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -164,6 +164,7 @@ static void linedisp_scroll(struct timer_list *t)
 static int linedisp_display(struct linedisp *linedisp, const char *msg,
 			    ssize_t count)
 {
+	guard(mutex)(&linedisp->lock);
 	char *new_msg;
 
 	/* stop the scroll timer */
@@ -226,6 +227,7 @@ static ssize_t message_show(struct device *dev, struct device_attribute *attr,
 {
 	struct linedisp *linedisp = to_linedisp(dev);
 
+	guard(mutex)(&linedisp->lock);
 	return sysfs_emit(buf, "%s\n", linedisp->message);
 }
 
@@ -267,6 +269,7 @@ static ssize_t scroll_step_ms_show(struct device *dev,
 {
 	struct linedisp *linedisp = to_linedisp(dev);
 
+	guard(mutex)(&linedisp->lock);
 	return sysfs_emit(buf, "%u\n", jiffies_to_msecs(linedisp->scroll_rate));
 }
 
@@ -282,6 +285,8 @@ static ssize_t scroll_step_ms_store(struct device *dev,
 	if (err)
 		return err;
 
+	guard(mutex)(&linedisp->lock);
+
 	timer_delete_sync(&linedisp->timer);
 
 	linedisp->scroll_rate = msecs_to_jiffies(ms);
@@ -444,6 +449,7 @@ int linedisp_attach(struct linedisp *linedisp, struct device *dev,
 	linedisp->ops = ops;
 	linedisp->num_chars = num_chars;
 	linedisp->scroll_rate = DEFAULT_SCROLL_RATE;
+	mutex_init(&linedisp->lock);
 
 	linedisp->buf = kzalloc(linedisp->num_chars, GFP_KERNEL);
 	if (!linedisp->buf)
@@ -531,6 +537,7 @@ int linedisp_register(struct linedisp *linedisp, struct device *parent,
 	linedisp->ops = ops;
 	linedisp->num_chars = num_chars;
 	linedisp->scroll_rate = DEFAULT_SCROLL_RATE;
+	mutex_init(&linedisp->lock);
 
 	err = ida_alloc(&linedisp_id, GFP_KERNEL);
 	if (err < 0)
diff --git a/drivers/auxdisplay/line-display.h b/drivers/auxdisplay/line-display.h
index 36853b639..13b56fe3b 100644
--- a/drivers/auxdisplay/line-display.h
+++ b/drivers/auxdisplay/line-display.h
@@ -13,6 +13,7 @@
 #define _LINEDISP_H
 
 #include <linux/device.h>
+#include <linux/mutex.h>
 #include <linux/timer_types.h>
 
 #include <linux/map_to_7segment.h>
@@ -58,6 +59,7 @@ struct linedisp_ops {
 /**
  * struct linedisp - character line display private data structure
  * @dev: the line display device
+ * @lock: serializes sysfs access to the display state
  * @timer: timer used to implement scrolling
  * @ops: character line display operations
  * @buf: pointer to the buffer for the string currently displayed
@@ -70,6 +72,7 @@ struct linedisp_ops {
  */
 struct linedisp {
 	struct device dev;
+	struct mutex lock; /* serializes sysfs access to the display state */
 	struct timer_list timer;
 	const struct linedisp_ops *ops;
 	struct linedisp_map *map;
-- 
2.51.2