Re: [PATCH] gpiolib: annotate struct acpi_gpio_mapping with __counted_by_ptr

["Ard Biesheuvel" <ardb@kernel.org>]

On Sat, 3 Oct 2026, at 10:54, Bill Wendling wrote:
> On Fri, Oct 2, 2026 at 6:52 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>> On Fri, 2 Oct 2026, at 14:44, Bill Wendling wrote:
>> > On Fri, Oct 2, 2026 at 12:38 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>> >> On Thu, 1 Oct 2026, at 23:24, Linus Walleij wrote:
>> >> > On Thu, Oct 1, 2026 at 10:05 PM Bill Wendling <morbo@google.com> wrote:
>> >> >> On Thu, Oct 1, 2026 at 12:56 PM Linus Walleij <linusw@kernel.org> wrote:
>> >> >> > On Mon, Sep 28, 2026 at 9:22 AM Bill Wendling <morbo@google.com> wrote:
>> >> >> > > On Sun, Sep 27, 2026 at 11:38 PM Bill Wendling <morbo@google.com> wrote:
>> >> >> > > >
>> >> >> > > > The 'data' pointer field in 'struct acpi_gpio_mapping' is associated
>> >> >> > > > with the 'size' field, which represents the number of elements of
>> >> >> > > > type 'struct acpi_gpio_params' allocated for 'data'.
>> >> >> > > >
>> >> >> > > > To improve bounds checking via CONFIG_UBSAN_BOUNDS and
>> >> >> > > > CONFIG_FORTIFY_SOURCE, annotate 'data' with the __counted_by_ptr
>> >> >> > > > attribute.
>> >> >> > > >
>> >> >> > > > Analysis of allocation, assignment, and access points shows that the
>> >> >> > > > pointer is never accessed before the count is set, which guarantees that
>> >> >> > > > this annotation is safe and will not cause runtime panics or
>> >> >> > > > false-positive bounds checks.
>> >> >> > > >
>> >> >> > > > Cc: codemender-patching+linux@google.com
>> >> >> > > > Assisted-by: LLM
>> >> >> > > > Signed-off-by: Bill Wendling <morbo@google.com>
>> >> >> > > > ---
>> >> >> > > >  include/linux/gpio/consumer.h | 2 +-
>> >> >> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >> > > >
>> >> >> > > > diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
>> >> >> > > > index fceeefd5f893..2b80cf7aa7e7 100644
>> >> >> > > > --- a/include/linux/gpio/consumer.h
>> >> >> > > > +++ b/include/linux/gpio/consumer.h
>> >> >> > > > @@ -667,7 +667,7 @@ struct acpi_gpio_params {
>> >> >> > > >
>> >> >> > > >  struct acpi_gpio_mapping {
>> >> >> > > >         const char *name;
>> >> >> > > > -       const struct acpi_gpio_params *data;
>> >> >> > > > +       const struct acpi_gpio_params *data __counted_by_ptr(size);
>> >> >> > > >         unsigned int size;
>> >> >> > > >
>> >> >> > > >  /* Ignore IoRestriction field */
>> >> >> > >
>> >> >> > > There's a problem with 'drivers/firmware/efi/libstub/Makefile'. Clang
>> >> >> > > needs a compiler flag to support the "__counted_by_ptr" attribute
>> >> >> > > referencing a field *after* the pointer, like in this patch. However,
>> >> >> > > the Makefile blasts the flag away for x86 platforms. Below is a hack
>> >> >> > > that copies the part of the top-level Makefile that adds the flag. I
>> >> >> > > don't think that's a good solution. The comment in the driver's
>> >> >> > > Makefile says that the stub code executes before the kernel does,
>> >> >> > > which I assume is why a lot of the flags are blown away... In any
>> >> >> > > event, I'm not sure how best to address this.
>> >> >> >
>> >> >> > But is this a problem with the current patch?
>> >> >> >
>> >> >> > Does libefistub use <linux/gpio/consumer.h> in any way, shape
>> >> >> > or form?
>> >> >> >
>> >> >> It's being #included transitively:
>> >> >>
>> >> >> In file included from drivers/firmware/efi/libstub/efi-stub-helper.c:12:
>> >> >> In file included from ./include/linux/efi.h:20:
>> >> >> In file included from ./include/linux/rtc.h:18:
>> >> >> In file included from ./include/linux/nvmem-provider.h:16:
>> >> >> ./include/linux/gpio/consumer.h:670:55: error: use of undeclared
>> >> >> identifier 'size'; did you
>> >> >>       mean 'ksize'?
>> >> >>   670 |         const struct acpi_gpio_params *data __counted_by_ptr(size);
>> >> >>       |                                                              ^~~~
>> >> >>       |                                                              ksize
>> >> >> ././include/linux/compiler_types.h:392:64: note: expanded from macro
>> >> >> '__counted_by_ptr'
>> >> >>   392 | #define __counted_by_ptr(member)
>> >> >> __attribute__((__counted_by__(member)))
>> >> >>       |
>> >> >>        ^~~~~~
>> >> >> ./include/linux/slab.h:602:8: note: 'ksize' declared here
>> >> >>   602 | size_t ksize(const void *objp);
>> >> >>       |        ^
>> >> >
>> >> > Hm I see.
>> >> >
>> >> > Certainly Jeremy or Ard will have an idea about how to solve this,
>> >> > so paging them in.
>> >> >
>> >>
>> >> libstub code never executes in the context of the kernel, but only in
>> >> the context of the boot firmware. Generally, we disable instrumentation
>> >> there that has a significant runtime component, basically because we
>> >> cannot crash or panic the kernel before we have even booted it.
>> >>
>> >> Can we just #define __counted_by_ptr(...) to nothing when building
>> >> from that Makefile?
>> >>
>> > Doing it in the Makefile is tricky, because of how the "c_flags"
>> > variable is defined and used. I couldn't find a good way to do it.
>> > Instead, I inserted "#undef __counted_by{_ptr}" at the top of the
>> > affected files. It's gross. If there's a way I'm missing, please let
>> > me know.
>> >
>>
>> Does that even build?
>
> It did for me...
>
> Let me look into Sashiko's comments.
>

When you #undef __counted_by(), it is passed straight to the compiler
rather than being turned into whichever __attribute__(()) it is supposed
to resolve to by the preprocessor.

Hence my surprise that it actually builds.

In any case, this should be done in the Makefile or in a header, not in
each individual C source file.