[PATCH] auxdisplay: linedisp: serialize sysfs message updates

Laxman Acharya Padhya <[email protected]>
Newsgroups org.kernel.vger.stable,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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: [email protected]
Signed-off-by: Laxman Acharya Padhya <[email protected]>
---
 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
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.