[PATCH v1] auxdisplay: linedisp: prepare message before registration

[PATCH v1] auxdisplay: linedisp: prepare message before registration

[Yibo Tan]

Since the line-display attachment support was added, linedisp_register()
has displayed its initial message before calling device_add().  The
HT16K33, MAX6959 and GPIO segment drivers implement display updates with
delayed work, so that display operation can publish driver-owned work
before the final fallible registration step.

If device_add() fails, the registration error path releases the display
buffers and the caller's probe failure releases its managed private data.
The admitted worker can then access both freed objects.  In an HT16K33
KASAN reproducer, a real device_add() -EEXIST failure produced a
slab-use-after-free first in __pwq_activate_work() and then in
ht16k33_seg7_update().

The vulnerable kernel queued and executed the freed HT16K33 work in three
of three runs.  With this change, the same three runs reached the same
device_add() failure without queuing the update and produced no KASAN,
WARNING, Oops or panic.

Separate message preparation from the hardware update.  Prepare the
initial message before registering the device or attribute groups, and
only invoke the non-failing update after registration succeeds.  Apply the
same ordering to linedisp_attach(), which has the same static pattern but
currently has no in-tree caller.

Fixes: 3ba5c78fe7c5 ("auxdisplay: linedisp: support attribute attachment to auxdisplay devices")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Yibo Tan <lhfff@tju.edu.cn>
---
 drivers/auxdisplay/line-display.c | 63 +++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index 915eb5cd96b2..d5854a9ac9b0 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -149,20 +149,8 @@ static void linedisp_scroll(struct timer_list *t)
 	mod_timer(&linedisp->timer, jiffies + linedisp->scroll_rate);
 }
 
-/**
- * linedisp_display() - set the message to be displayed
- * @linedisp: pointer to the private data structure
- * @msg: the message to display
- * @count: length of msg, or -1
- *
- * Display a new message @msg on the display. @msg can be longer than the
- * number of characters the display can display, in which case it will begin
- * scrolling across the display.
- *
- * Return: 0 on success, -ENOMEM on memory allocation failure
- */
-static int linedisp_display(struct linedisp *linedisp, const char *msg,
-			    ssize_t count)
+static int linedisp_set_message(struct linedisp *linedisp, const char *msg,
+				ssize_t count)
 {
 	char *new_msg;
 
@@ -181,8 +169,6 @@ static int linedisp_display(struct linedisp *linedisp, const char *msg,
 		kfree(linedisp->message);
 		linedisp->message = NULL;
 		linedisp->message_len = 0;
-		memset(linedisp->buf, ' ', linedisp->num_chars);
-		linedisp->ops->update(linedisp);
 		return 0;
 	}
 
@@ -195,6 +181,16 @@ static int linedisp_display(struct linedisp *linedisp, const char *msg,
 	linedisp->message = new_msg;
 	linedisp->message_len = count;
 	linedisp->scroll_pos = 0;
+	return 0;
+}
+
+static void linedisp_update(struct linedisp *linedisp)
+{
+	if (!linedisp->message_len) {
+		memset(linedisp->buf, ' ', linedisp->num_chars);
+		linedisp->ops->update(linedisp);
+		return;
+	}
 
 	if (should_scroll(linedisp)) {
 		/* display scrolling message */
@@ -206,7 +202,30 @@ static int linedisp_display(struct linedisp *linedisp, const char *msg,
 		       umin(linedisp->num_chars, linedisp->message_len));
 		linedisp->ops->update(linedisp);
 	}
+}
 
+/**
+ * linedisp_display() - set the message to be displayed
+ * @linedisp: pointer to the private data structure
+ * @msg: the message to display
+ * @count: length of msg, or -1
+ *
+ * Display a new message @msg on the display. @msg can be longer than the
+ * number of characters the display can display, in which case it will begin
+ * scrolling across the display.
+ *
+ * Return: 0 on success, -ENOMEM on memory allocation failure
+ */
+static int linedisp_display(struct linedisp *linedisp, const char *msg,
+			    ssize_t count)
+{
+	int err;
+
+	err = linedisp_set_message(linedisp, msg, count);
+	if (err)
+		return err;
+
+	linedisp_update(linedisp);
 	return 0;
 }
 
@@ -461,8 +480,7 @@ int linedisp_attach(struct linedisp *linedisp, struct device *dev,
 	if (err)
 		goto out_del_timer;
 
-	/* display a default message */
-	err = linedisp_display(linedisp, LINEDISP_INIT_TEXT, -1);
+	err = linedisp_set_message(linedisp, LINEDISP_INIT_TEXT, -1);
 	if (err)
 		goto out_del_attach;
 
@@ -471,6 +489,9 @@ int linedisp_attach(struct linedisp *linedisp, struct device *dev,
 	if (err)
 		goto out_del_attach;
 
+	/* display the prepared default message */
+	linedisp_update(linedisp);
+
 	return 0;
 
 out_del_attach:
@@ -557,8 +578,7 @@ int linedisp_register(struct linedisp *linedisp, struct device *parent,
 	if (err)
 		goto out_del_timer;
 
-	/* display a default message */
-	err = linedisp_display(linedisp, LINEDISP_INIT_TEXT, -1);
+	err = linedisp_set_message(linedisp, LINEDISP_INIT_TEXT, -1);
 	if (err)
 		goto out_del_attach;
 
@@ -566,6 +586,9 @@ int linedisp_register(struct linedisp *linedisp, struct device *parent,
 	if (err)
 		goto out_del_attach;
 
+	/* display the prepared default message */
+	linedisp_update(linedisp);
+
 	return 0;
 
 out_del_attach:
-- 
2.39.5

Re: [PATCH v1] auxdisplay: linedisp: prepare message before registration

[Andy Shevchenko]

On Sun, Sep 13, 2026 at 12:11:52AM +0800, Yibo Tan wrote:
> Since the line-display attachment support was added, linedisp_register()
> has displayed its initial message before calling device_add().  The
> HT16K33, MAX6959 and GPIO segment drivers implement display updates with
> delayed work, so that display operation can publish driver-owned work
> before the final fallible registration step.
> 
> If device_add() fails, the registration error path releases the display
> buffers and the caller's probe failure releases its managed private data.
> The admitted worker can then access both freed objects.  In an HT16K33
> KASAN reproducer, a real device_add() -EEXIST failure produced a
> slab-use-after-free first in __pwq_activate_work() and then in
> ht16k33_seg7_update().
> 
> The vulnerable kernel queued and executed the freed HT16K33 work in three
> of three runs.  With this change, the same three runs reached the same
> device_add() failure without queuing the update and produced no KASAN,
> WARNING, Oops or panic.
> 
> Separate message preparation from the hardware update.  Prepare the
> initial message before registering the device or attribute groups, and
> only invoke the non-failing update after registration succeeds.  Apply the
> same ordering to linedisp_attach(), which has the same static pattern but
> currently has no in-tree caller.

Yeah, but this will fix only work queueing and not the whole issue as was
earlier noted (about allocation of the private data in the driver and using
it inside the auxdisplay core. Can we rather redo the _register() / _unregister()
APIs to follow the design that is used, for instance, in IIO subsystem?

-- 
With Best Regards,
Andy Shevchenko