[PATCH] auxdisplay: line-display: fix OOB read on zero-length message_store()

[PATCH] auxdisplay: line-display: fix OOB read on zero-length message_store()

[Stepan Ionichev]

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.0

Re: [PATCH] auxdisplay: line-display: fix OOB read on zero-length message_store()

[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

Re: [PATCH] auxdisplay: line-display: fix OOB read on zero-length message_store()

[Stepan Ionichev]

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

Re: [PATCH] auxdisplay: line-display: fix OOB read on zero-length message_store()

[Geert Uytterhoeven]

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

Re: [PATCH] auxdisplay: line-display: fix OOB read on zero-length message_store()

[Andy Shevchenko]

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

Re: [PATCH] auxdisplay: line-display: fix OOB read on zero-length message_store()

[Stepan Ionichev]

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

Re: [PATCH] auxdisplay: line-display: fix OOB read on zero-length message_store()

[Andy Shevchenko]

On 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