Re: [PATCH 08/14] net: ipa: Annotate struct ipa_power with __counted_by

[Kees Cook <keescook@chromium.org>]

On Sat, Sep 23, 2023 at 07:09:19AM -0500, Alex Elder wrote:
> On 9/22/23 12:28 PM, Kees Cook wrote:
> > Prepare for the coming implementation by GCC and Clang of the __counted_by
> > attribute. Flexible array members annotated with __counted_by can have
> > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
> > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> > functions).
> > 
> > As found with Coccinelle[1], add __counted_by for struct ipa_power.
> 
> Looks good, thanks.
> 
> Reviewed-by: Alex Elder <elder@linaro.org>
> 
> Note that there is some interaction between struct ipa_power_data
> and struct ipa_power (the former is used to initialize the latter).
> Both of these contain flexible arrays counted by another field in
> the structure.  It seems possible that the way these are initialized
> might need slight modification to allow the compiler to do its
> enforcement; if that's the case, please reach out to me.

I think it's all okay:

struct ipa_power_data {
        u32 core_clock_rate;
        u32 interconnect_count;         /* # entries in interconnect_data[] */
        const struct ipa_interconnect_data *interconnect_data;
};

"interconnect_data" here is a pointer, not a flexible array. (Yes,
__counted_by is expected to be expanded in the future for pointers,
but not yet.) Looking at initializers, I didn't see any problems with
how struct ipa_power is allocated.

Thanks for the heads-up; I'm sure I'll look at this again when we can
further expand __counted_by to pointers. :)

-Kees

-- 
Kees Cook