linux-hardening - Re: [bug report] printk: ringbuffer: Add KUnit test

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ad19157b-215d-435d-9b60-873d1cfffb04@suswa.mountain>
Date: Wed, 2 Jul 2025 23:33:07 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Nathan Chancellor <nathan@...nel.org>
Cc: Thomas Weißschuh <thomas.weissschuh@...utronix.de>,
	John Ogness <john.ogness@...utronix.de>,
	Kees Cook <kees@...nel.org>, linux-hardening@...r.kernel.org,
	Petr Mladek <pmladek@...e.com>
Subject: Re: [bug report] printk: ringbuffer: Add KUnit test

On Wed, Jul 02, 2025 at 01:22:03PM -0700, Nathan Chancellor wrote:
> On Thu, Jun 26, 2025 at 08:59:52AM +0200, Thomas Weißschuh wrote:
> > On Wed, Jun 25, 2025 at 10:22:19AM -0500, Dan Carpenter wrote:
> > > Hello Thomas Weißschuh,
> > > 
> > > The patch 5ea2bcdfbf46: "printk: ringbuffer: Add KUnit test" from Jun
> > > 12, 2025, leads to the following static checker warning:
> > > 
> > > 	kernel/printk/printk_ringbuffer_kunit_test.c:91 prbtest_check_data()
> > > 	(unpublished script worries this an off by one)
> > > 
> > > kernel/printk/printk_ringbuffer_kunit_test.c
> > >     83 static bool prbtest_check_data(const struct prbtest_rbdata *dat)
> > >     84 {
> > >     85 	unsigned int len;
> > >     86 
> > >     87 	/* Sane length? */
> > >     88 	if (dat->len < 1 || dat->len > MAX_RBDATA_TEXT_SIZE)
> > >     89 		return false;
> > >     90 
> > > --> 91 	if (dat->text[dat->len] != '\0')
> > >     92 		return false;
> > >     93 
> > > 
> > > My question is that the prbtest_rbdata structure is declared like this:
> > > 
> > >     53  /* test data structure */
> > >     54  struct prbtest_rbdata {
> > >     55          unsigned int len;
> > >     56          char text[] __counted_by(len);
> > >     57  };
> > > 
> > > The size of text is not really counted by len, it's "MAX_RBDATA_TEXT_SIZE
> > > + 1".  The condition "if (dat->text[dat->len] != '\0')" is reading one
> > > element beyond the __counted_by() value so something should complain if
> > > we enable all the debugging, right?
> > 
> > You are right, we are reading past the __counted_by().
> > But I don't get any complains with CONFIG_FORTIFY_SOURCE=y and CONFIG_KASAN=y
> > on either clang or gcc.
> 
> You will need CONFIG_UBSAN_BOUNDS to see warnings from __counted_by()
> but they are present if I enable it with this test:
> 
>   [    0.507904] ------------[ cut here ]------------
>   [    0.507904] UBSAN: array-index-out-of-bounds in kernel/printk/printk_ringbuffer_kunit_test.c:132:4

Thanks, Nathan.  You are the best.

regards,
dan carpenter