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
