drivers/auxdisplay/line-display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
linedisp_display() unconditionally reads msg[count - 1] before
checking whether count is zero, so a write of zero bytes to the
message sysfs attribute hits msg[-1]:
write(fd, "", 0);
-> message_store(..., buf, count=0)
-> linedisp_display(linedisp, buf, count=0)
-> msg[count - 1] == '\n' ; OOB read
The kernfs write buffer for that store is a 1-byte allocation
(kernfs_fop_write_iter() does kmalloc(len + 1) with len == 0),
so msg[-1] is a 1-byte read before the slab object. On a
KASAN-enabled kernel this trips an out-of-bounds report and
panics; on stock kernels it silently reads adjacent slab data
and, if that byte happens to be '\n', the following count--
wraps ssize_t 0 to -1 and is then passed to kmemdup_nul().
linedisp_display() is reached from the message_store() sysfs
callback (drivers/auxdisplay/line-display.c message attribute,
mode 0644) and from the in-tree initial-message setup with
count == -1, so the OOB path is only userspace-triggerable via
zero-byte writes; vfs_write() does not short-circuit on
count == 0 and kernfs_fop_write_iter() dispatches the store
callback regardless.
Guard the trailing-newline trim with a count check. The
existing if (!count) block then takes the clear-display path
unchanged.
Affects every auxdisplay driver that registers via
linedisp_register() / linedisp_attach(): ht16k33, max6959,
img-ascii-lcd, seg-led-gpio.
Fixes: 7e76aece6f03 ("auxdisplay: Extract character line display core support")
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
drivers/auxdisplay/line-display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index fb6d92941..915eb5cd9 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -173,7 +173,7 @@ static int linedisp_display(struct linedisp *linedisp, const char *msg,
count = strlen(msg);
/* if the string ends with a newline, trim it */
- if (msg[count - 1] == '\n')
+ if (count && msg[count - 1] == '\n')
count--;
if (!count) {
--
2.43.0On Thu, May 14, 2026 at 10:43:42PM +0500, Stepan Ionichev wrote: > linedisp_display() unconditionally reads msg[count - 1] before > checking whether count is zero, so a write of zero bytes to the > message sysfs attribute hits msg[-1]: > > write(fd, "", 0); > > -> message_store(..., buf, count=0) > -> linedisp_display(linedisp, buf, count=0) > -> msg[count - 1] == '\n' ; OOB read > > The kernfs write buffer for that store is a 1-byte allocation > (kernfs_fop_write_iter() does kmalloc(len + 1) with len == 0), > so msg[-1] is a 1-byte read before the slab object. On a > KASAN-enabled kernel this trips an out-of-bounds report and > panics; on stock kernels it silently reads adjacent slab data > and, if that byte happens to be '\n', the following count-- > wraps ssize_t 0 to -1 and is then passed to kmemdup_nul(). > > linedisp_display() is reached from the message_store() sysfs > callback (drivers/auxdisplay/line-display.c message attribute, > mode 0644) and from the in-tree initial-message setup with > count == -1, so the OOB path is only userspace-triggerable via > zero-byte writes; vfs_write() does not short-circuit on > count == 0 and kernfs_fop_write_iter() dispatches the store > callback regardless. > > Guard the trailing-newline trim with a count check. The > existing if (!count) block then takes the clear-display path > unchanged. > > Affects every auxdisplay driver that registers via > linedisp_register() / linedisp_attach(): ht16k33, max6959, > img-ascii-lcd, seg-led-gpio. Pushed to my review and testing queue, thanks! -- With Best Regards, Andy Shevchenko
On Thu, May 14, 2026 at 8:44 PM Stepan Ionichev <sozdayvek@gmail.com> wrote: > > linedisp_display() unconditionally reads msg[count - 1] before > checking whether count is zero, so a write of zero bytes to the > message sysfs attribute hits msg[-1]: > > write(fd, "", 0); > > -> message_store(..., buf, count=0) > -> linedisp_display(linedisp, buf, count=0) > -> msg[count - 1] == '\n' ; OOB read > > The kernfs write buffer for that store is a 1-byte allocation > (kernfs_fop_write_iter() does kmalloc(len + 1) with len == 0), > so msg[-1] is a 1-byte read before the slab object. On a > KASAN-enabled kernel this trips an out-of-bounds report and > panics; on stock kernels it silently reads adjacent slab data > and, if that byte happens to be '\n', the following count-- > wraps ssize_t 0 to -1 and is then passed to kmemdup_nul(). > > linedisp_display() is reached from the message_store() sysfs > callback (drivers/auxdisplay/line-display.c message attribute, > mode 0644) and from the in-tree initial-message setup with > count == -1, so the OOB path is only userspace-triggerable via > zero-byte writes; Isn't it also triggerable when PANEL_BOOT_MESSAGE is left default with PANEL_CHANGE_MESSAGE="y"? (However these double quotes makes me wonder if this even works, as usually we compare symbols against plain 'n'. 'm', or 'y' (without any quotes). > vfs_write() does not short-circuit on > count == 0 and kernfs_fop_write_iter() dispatches the store > callback regardless. > > Guard the trailing-newline trim with a count check. The > existing if (!count) block then takes the clear-display path > unchanged. > > Affects every auxdisplay driver that registers via > linedisp_register() / linedisp_attach(): ht16k33, max6959, > img-ascii-lcd, seg-led-gpio. In any case this seems a legit report, I will take the change. -- With Best Regards, Andy Shevchenko
Hi Andy,
On Fri, 15 May 2026 at 09:13, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Thu, May 14, 2026 at 8:44 PM Stepan Ionichev <sozdayvek@gmail.com> wrote:
> > linedisp_display() unconditionally reads msg[count - 1] before
> > checking whether count is zero, so a write of zero bytes to the
> > message sysfs attribute hits msg[-1]:
> >
> > write(fd, "", 0);
> >
> > -> message_store(..., buf, count=0)
> > -> linedisp_display(linedisp, buf, count=0)
> > -> msg[count - 1] == '\n' ; OOB read
> >
> > The kernfs write buffer for that store is a 1-byte allocation
> > (kernfs_fop_write_iter() does kmalloc(len + 1) with len == 0),
> > so msg[-1] is a 1-byte read before the slab object. On a
> > KASAN-enabled kernel this trips an out-of-bounds report and
> > panics; on stock kernels it silently reads adjacent slab data
> > and, if that byte happens to be '\n', the following count--
> > wraps ssize_t 0 to -1 and is then passed to kmemdup_nul().
> >
> > linedisp_display() is reached from the message_store() sysfs
> > callback (drivers/auxdisplay/line-display.c message attribute,
> > mode 0644) and from the in-tree initial-message setup with
> > count == -1, so the OOB path is only userspace-triggerable via
> > zero-byte writes;
>
> Isn't it also triggerable when PANEL_BOOT_MESSAGE is left default
> with PANEL_CHANGE_MESSAGE="y"? (However these double quotes makes me
> wonder if this even works, as usually we compare symbols against plain
> 'n'. 'm', or 'y' (without any quotes).
>
> > vfs_write() does not short-circuit on
> > count == 0 and kernfs_fop_write_iter() dispatches the store
> > callback regardless.
I think PANEL_BOOT_MESSAGE is the only way to trigger this, as
writing an empty string to a device attribute is a no-op according
to commit afcb5a811ff3ab39 ("auxdisplay: img-ascii-lcd: Fix lock-up
when displaying empty string")? If that is still true, the issue
was introduced by commit c8ffef985af564c1 ("auxdisplay: linedisp:
Support configuring the boot message")?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Mon, May 18, 2026 at 10:08:07AM +0200, Geert Uytterhoeven wrote:
> On Fri, 15 May 2026 at 09:13, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Thu, May 14, 2026 at 8:44 PM Stepan Ionichev <sozdayvek@gmail.com> wrote:
> > > linedisp_display() unconditionally reads msg[count - 1] before
> > > checking whether count is zero, so a write of zero bytes to the
> > > message sysfs attribute hits msg[-1]:
> > >
> > > write(fd, "", 0);
> > >
> > > -> message_store(..., buf, count=0)
> > > -> linedisp_display(linedisp, buf, count=0)
> > > -> msg[count - 1] == '\n' ; OOB read
> > >
> > > The kernfs write buffer for that store is a 1-byte allocation
> > > (kernfs_fop_write_iter() does kmalloc(len + 1) with len == 0),
> > > so msg[-1] is a 1-byte read before the slab object. On a
> > > KASAN-enabled kernel this trips an out-of-bounds report and
> > > panics; on stock kernels it silently reads adjacent slab data
> > > and, if that byte happens to be '\n', the following count--
> > > wraps ssize_t 0 to -1 and is then passed to kmemdup_nul().
> > >
> > > linedisp_display() is reached from the message_store() sysfs
> > > callback (drivers/auxdisplay/line-display.c message attribute,
> > > mode 0644) and from the in-tree initial-message setup with
> > > count == -1, so the OOB path is only userspace-triggerable via
> > > zero-byte writes;
> >
> > Isn't it also triggerable when PANEL_BOOT_MESSAGE is left default
> > with PANEL_CHANGE_MESSAGE="y"? (However these double quotes makes me
> > wonder if this even works, as usually we compare symbols against plain
> > 'n'. 'm', or 'y' (without any quotes).
> >
> > > vfs_write() does not short-circuit on
> > > count == 0 and kernfs_fop_write_iter() dispatches the store
> > > callback regardless.
>
> I think PANEL_BOOT_MESSAGE is the only way to trigger this, as
> writing an empty string to a device attribute is a no-op according
> to commit afcb5a811ff3ab39 ("auxdisplay: img-ascii-lcd: Fix lock-up
> when displaying empty string")? If that is still true, the issue
> was introduced by commit c8ffef985af564c1 ("auxdisplay: linedisp:
> Support configuring the boot message")?
Good points. Should I drop the patch and ask for a new commit message
(and Fixes tag)?
--
With Best Regards,
Andy Shevchenko
On Mon, May 18, 2026 at 11:15:00AM +0300, Andy Shevchenko wrote: > Good points. Should I drop the patch and ask for a new commit message > (and Fixes tag)? The current line-display.c message_store() calls linedisp_display(linedisp, buf, count) unconditionally, with no count == 0 short-circuit, so write(fd, "", 0) still reaches msg[-1]. The afcb5a811ff3a fix Geert mentions was on img-ascii-lcd's own message_store before the shared code was extracted; when 7e76aece6f03 pulled linedisp_display into line-display.c, the empty-write guard didn't come with it. So both paths trigger the same dereference: zero-byte sysfs writes and PANEL_BOOT_MESSAGE="" via linedisp_attach(). The underlying bug sits in 7e76aece6f03 either way, so I think the existing Fixes is right and no respin is needed. Happy to send v2 with both commits mentioned in the log if you'd prefer that. Stepan
On Fri, May 15, 2026 at 10:12:26AM +0300, Andy Shevchenko wrote:
> Isn't it also triggerable when PANEL_BOOT_MESSAGE is left default
> with PANEL_CHANGE_MESSAGE="y"? (However these double quotes makes me
> wonder if this even works, as usually we compare symbols against plain
> 'n'. 'm', or 'y' (without any quotes).
Yes -- the same count guard also covers the init path: when
PANEL_BOOT_MESSAGE="" and PANEL_CHANGE_MESSAGE=y, linedisp_attach()
calls linedisp_display(linedisp, "", -1), so count = strlen("") = 0
and msg[-1] reads .rodata before the empty string literal. KASAN
catches it at boot. The patch covers both paths in one guard.
Re: depends on PANEL_CHANGE_MESSAGE="y" -- agreed, that looks odd.
Normally we'd just write "depends on PANEL_CHANGE_MESSAGE". I can
send a separate Kconfig patch if you'd like.
> In any case this seems a legit report, I will take the change.
Thanks for taking it.
Stepan
© 2016 - 2026 Red Hat, Inc.