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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250702202203.GA3347226@ax162>
Date: Wed, 2 Jul 2025 13:22:03 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
Cc: Dan Carpenter <dan.carpenter@...aro.org>,
	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 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
  [    0.507907] index 108 is out of range for type 'char[] __counted_by(len)' (aka 'char[]')
  [    0.507909] CPU: 3 UID: 0 PID: 392 Comm: prbtest writer  Tainted: G                 N  6.16.0-rc4-next-20250702 #1 PREEMPT(voluntary)
  [    0.507912] Tainted: [N]=TEST
  [    0.507913] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
  [    0.507914] Call Trace:
  [    0.507916]  <TASK>
  [    0.507918]  dump_stack_lvl+0x84/0xc0
  [    0.507922]  ubsan_epilogue+0x5/0x30
  [    0.507925]  __ubsan_handle_out_of_bounds+0xa2/0xc0
  [    0.507928]  prbtest_writer+0x16a/0x210
  [    0.507932]  ? __pfx_prbtest_writer+0x10/0x10
  [    0.507934]  kthread+0x21a/0x260
  [    0.507937]  ? __pfx_kthread+0x10/0x10
  [    0.507938]  ret_from_fork+0x77/0xd0
  [    0.507940]  ? __pfx_kthread+0x10/0x10
  [    0.507941]  ret_from_fork_asm+0x1a/0x30
  [    0.507945]  </TASK>
  [    0.507945] ---[ end trace ]---

  [    0.516113] ------------[ cut here ]------------
  [    0.516115] UBSAN: array-index-out-of-bounds in kernel/printk/printk_ringbuffer_kunit_test.c:91:6
  [    0.516117]     # test_readerwriter: lib/ubsan.c:228: array-index-out-of-bounds in kernel/printk/printk_ringbuffer_kunit_test.c
  [    0.516119] index 126 is out of range for type 'char const[] __counted_by(len)' (aka 'const char[]')
  [    0.516122] CPU: 0 UID: 0 PID: 389 Comm: kunit_try_catch Tainted: G                 N  6.16.0-rc4-next-20250702 #1 PREEMPT(voluntary)
  [    0.516125] Tainted: [N]=TEST
  [    0.516126] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
  [    0.516127] Call Trace:
  [    0.516129]  <TASK>
  [    0.516131]  dump_stack_lvl+0x84/0xc0
  [    0.516136]  ubsan_epilogue+0x5/0x30
  [    0.516138]  __ubsan_handle_out_of_bounds+0xa2/0xc0
  [    0.516142]  prbtest_reader+0x2ff/0x480
  [    0.516145]  ? srso_alias_return_thunk+0x5/0xfbef5
  [    0.516147]  ? set_next_entity+0x5e/0x140
  [    0.516150]  ? __pfx_prbtest_wakeup_callback+0x10/0x10
  [    0.516155]  ? set_cpus_allowed_ptr+0x83/0xb0
  [    0.516158]  test_readerwriter+0x297/0x3c0
  [    0.516161]  kunit_try_run_case+0x8c/0x190
  [    0.516164]  kunit_generic_run_threadfn_adapter+0x1a/0x40
  [    0.516166]  ? __pfx_kunit_generic_run_threadfn_adapter+0x10/0x10
  [    0.516167]  kthread+0x21a/0x260
  [    0.516169]  ? __pfx_kthread+0x10/0x10
  [    0.516170]  ret_from_fork+0x77/0xd0
  [    0.516173]  ? __pfx_kthread+0x10/0x10
  [    0.516174]  ret_from_fork_asm+0x1a/0x30
  [    0.516177]  </TASK>
  [    0.516178] ---[ end trace ]---

I see Petr sent an updated patch for this, which does resolve the
warning in my testing.

https://lore.kernel.org/20250702095157.110916-4-pmladek@suse.com/

Cheers,
Nathan