Re: [PATCH] fortify: add KUnit tests for __counted_by and __counted_by_ptr

[Bill Wendling]

Hi Thomas,

On Mon, Sep 28, 2026 at 11:23 PM Thomas Weißschuh
<thomas.weissschuh@xxxxxxxxxxxxx> wrote:
>
> Hi Bill,
>
> this looks much better. Thanks for the rework.
>
> On Tue, Sep 29, 2026 at 01:00:31AM +0000, Bill Wendling wrote:
> > The '__counted_by' and '__counted_by_ptr' attributes associate a
> > flexible array member or pointer member with a struct field that holds
> > its element count. Supporting compilers use these annotations to
> > compute dynamic object sizes via '__builtin_dynamic_object_size()' for
> > runtime bounds checking with CONFIG_FORTIFY_SOURCE.
> >
> > Add KUnit tests ('fortify_test_counted_by_flex' and
> > 'fortify_test_counted_by_ptr', guarded by CONFIG_CC_HAS_COUNTED_BY and
> > CONFIG_CC_HAS_COUNTED_BY_PTR respectively) to verify that:
> >
> >  - '__builtin_dynamic_object_size()' (both types 0 and 1) returns the
> >    expected logical byte size for annotated flexible array and pointer
> >    members.
> >  - Fortified operations ('memset()' and 'memchr()') succeed within the
> >    logical bounds and detect out-of-bounds read and write accesses
> >    beyond the annotated count.
> >
> > Allocate the test buffers with extra physical capacity (2 * size) in
> > 'noinline' helpers and hide the returned pointers with
> > OPTIMIZER_HIDE_VAR() so allocation-size attributes, physical slab
> > bounds, and compiler optimizations do not mask the '__counted_by' and
> > '__counted_by_ptr' checks.
> >
> > Signed-off-by: Bill Wendling <morbo@xxxxxxxxxx>
> > ---
> > v2: Move tests to the 'fortify' KUnit tests. It uses UBSAN, which is
> >     what gets triggered by 'counted_by'.
>
> v2 is missing in subject.
>
Doh!

> > ---
> >  lib/tests/fortify_kunit.c | 119 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 119 insertions(+)
> >
> > diff --git a/lib/tests/fortify_kunit.c b/lib/tests/fortify_kunit.c
> > index 413cdbf3dc0d..2335171f84d8 100644
> > --- a/lib/tests/fortify_kunit.c
> > +++ b/lib/tests/fortify_kunit.c
> > @@ -1011,6 +1011,119 @@ static void fortify_test_kmemdup(struct kunit *test)
> >       kfree(copy);
> >  }
> >
> > +#ifdef CONFIG_CC_HAS_COUNTED_BY
>
> The ugly ifdeffery can be replaced by IS_ENABLED():
>
> if (!IS_ENABLED(CONFIG_FOO))
>         kunit_skip(test, "Not built with CONFIG_FOO=y");
>
> It makes the code cleaner and gives some useful feedback at runtime.
>
Ah yes! This is much nicer. It'll also fix up the #ifdef stuff at the
end as well.

> > +struct counted_by_flex_struct {
> > +     size_t size;
> > +     int array[] __counted_by(size);
> > +};
> > +
> > +/*
> > + * Allocate the struct out-of-line with extra physical capacity so that
> > + * __alloc_size() and physical slab bounds do not mask the __counted_by()
> > + * logical bounds check.
> > + */
> > +static noinline struct counted_by_flex_struct *
> > +alloc_counted_by_flex_struct(struct kunit *test, size_t size)
> > +{
> > +     struct counted_by_flex_struct *s;
> > +
> > +     s = kzalloc(sizeof(*s) + 2 * size * sizeof(s->array[0]), GFP_KERNEL);
>
> kunit_kzalloc() to automatically free the allocation again.
> struct_size() for the size calculation.
>
Oh cool! done.

> > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, s);
> > +
> > +     s->size = size;
> > +     return s;
> > +}
> > +
> > +static void fortify_test_counted_by_flex(struct kunit *test)
> > +{
> > +     size_t size = 128;
> > +     struct counted_by_flex_struct *s;
> > +     size_t elem_bytes = size * sizeof(s->array[0]);
> > +
> > +     s = alloc_counted_by_flex_struct(test, size);
> > +
> > +     OPTIMIZER_HIDE_VAR(s);
> > +     OPTIMIZER_HIDE_VAR(elem_bytes);
> > +
> > +     /* __builtin_dynamic_object_size() should return the logical length. */
> > +     KUNIT_EXPECT_EQ(test, elem_bytes,
> > +                     __builtin_dynamic_object_size(s->array, 0));
> > +     KUNIT_EXPECT_EQ(test, elem_bytes,
> > +                     __builtin_dynamic_object_size(s->array, 1));
> > +
> > +     /* Within-bounds write and read succeed. */
> > +     memset(s->array, 0x42, elem_bytes);
> > +     KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
> > +     KUNIT_EXPECT_NOT_NULL(test, memchr(s->array, 0x42, elem_bytes));
> > +     KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
> > +
> > +     /* Out-of-bounds write and read past logical size are caught. */
> > +     memset(s->array, 0x42, elem_bytes + 1);
> > +     KUNIT_EXPECT_EQ(test, fortify_write_overflows, 1);
> > +     KUNIT_EXPECT_NULL(test, memchr(s->array, 0x42, elem_bytes + 1));
> > +     KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
> > +
> > +     kfree(s);
> > +}
> > +
> > +#ifdef CONFIG_CC_HAS_COUNTED_BY_PTR
> > +struct counted_by_ptr_struct {
> > +     char *ptr __counted_by_ptr(size);
> > +     size_t size;
> > +};
>
> In the other structure the arguments where swapped, intentional?
>
Yes. It's a minor test to make sure that the attribute can refer to a
field that's defined after the pointer.

> > +
> > +/*
> > + * Allocate the struct out-of-line with extra physical capacity so that
> > + * __alloc_size() and physical slab bounds do not mask the __counted_by_ptr()
> > + * logical bounds check.
> > + */
> > +static noinline struct counted_by_ptr_struct *
> > +alloc_counted_by_ptr_struct(struct kunit *test, size_t size)
> > +{
> > +     struct counted_by_ptr_struct *s;
> > +
> > +     s = kmalloc_obj(struct counted_by_ptr_struct);
>
> We should probably also get kunit_kmalloc_obj() at some point.
>
> > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, s);
> > +
> > +     s->size = size;
> > +     s->ptr = kzalloc(2 * size, GFP_KERNEL);
> > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, s->ptr);
> > +
> > +     return s;
> > +}
> > +
> > +static void fortify_test_counted_by_ptr(struct kunit *test)
> > +{
> > +     size_t size = 128;
> > +     struct counted_by_ptr_struct *s;
> > +
> > +     s = alloc_counted_by_ptr_struct(test, size);
> > +
> > +     OPTIMIZER_HIDE_VAR(s);
> > +     OPTIMIZER_HIDE_VAR(size);
> > +
> > +     /* __builtin_dynamic_object_size() should return the logical length. */
> > +     KUNIT_EXPECT_EQ(test, size, __builtin_dynamic_object_size(s->ptr, 0));
> > +     KUNIT_EXPECT_EQ(test, size, __builtin_dynamic_object_size(s->ptr, 1));
>
> Is this builtin guaranteed to be available?
> We have a wrapper KUNIT_EXPECT_BDOS() above.
>
I suppose not. I'll use the macros instead.

> > +
> > +     /* Within-bounds write and read succeed. */
> > +     memset(s->ptr, 0x42, size);
> > +     KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
> > +     KUNIT_EXPECT_NOT_NULL(test, memchr(s->ptr, 0x42, size));
> > +     KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
> > +
> > +     /* Out-of-bounds write and read past logical size are caught. */
> > +     memset(s->ptr, 0x42, size + 1);
> > +     KUNIT_EXPECT_EQ(test, fortify_write_overflows, 1);
> > +     KUNIT_EXPECT_NULL(test, memchr(s->ptr, 0x42, size + 1));
> > +     KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
> > +
> > +     kfree(s->ptr);
> > +     kfree(s);
> > +}
> > +#endif /* CONFIG_CC_HAS_COUNTED_BY_PTR */
> > +#endif /* CONFIG_CC_HAS_COUNTED_BY */
> > +
> >  static int fortify_test_init(struct kunit *test)
> >  {
> >       if (!IS_ENABLED(CONFIG_FORTIFY_SOURCE))
> > @@ -1054,6 +1167,12 @@ static struct kunit_case fortify_test_cases[] = {
> >       KUNIT_CASE(fortify_test_memchr_inv),
> >       KUNIT_CASE(fortify_test_memcmp),
> >       KUNIT_CASE(fortify_test_kmemdup),
> > +#ifdef CONFIG_CC_HAS_COUNTED_BY
> > +     KUNIT_CASE(fortify_test_counted_by_flex),
> > +#ifdef CONFIG_CC_HAS_COUNTED_BY_PTR
>
> The nesting of these conditionals looks unnecessary.
>

-bw

> > +     KUNIT_CASE(fortify_test_counted_by_ptr),
> > +#endif /* CONFIG_CC_HAS_COUNTED_BY_PTR */
> > +#endif /* CONFIG_CC_HAS_COUNTED_BY */
> >       {}
> >  };
> >
> > --
> > 2.56.0.rc1.315.gc6ed9934b7-goog
> >