Re: [PATCH 1/2] gpio: realtek: Add GPIO support for RTD SoC variants

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Thu, Nov 2, 2023 at 1:40 PM TY_Chang[張子逸] <tychang@xxxxxxxxxxx> wrote:

> >On Wed, Nov 1, 2023 at 3:58 AM Tzuyi Chang <tychang@xxxxxxxxxxx> wrote:
> >> +static unsigned int rtd_gpio_deb_offset(struct rtd_gpio *data,
> >> +unsigned int offset) {
> >> +       return data->info->deb_offset[offset / 8]; }
> >
> >So this is clearly counted by the GPIO number offset and the GPIO number
> >determines how far into the array we can index.
> >
> >It looks a bit dangerous, it it possible to encode the array lengths better?
>
> I think I can add array size members for each offset array within the
> rtd_gpio_info structure and utilize them to prevent accessing elements outside the array.

I don't know about that for constant arrays, if you look at recent commits
from Kees Cook you will find examples of how to use the compiler helpers
for dynamic arrays, e.g.

git log -p --author=Kees

will find you things like this:

@@ -60,7 +60,7 @@ struct reset_control {
 struct reset_control_array {
        struct reset_control base;
        unsigned int num_rstcs;
-       struct reset_control *rstc[];
+       struct reset_control *rstc[] __counted_by(num_rstcs);
 };

So the compiler instruction __counted_by() is used pointing back to
the variable above to avoid outofbounds accesses.

BUT: those are *dynamic* arrays, placed *last* in the struct.

You have several *constant* arrays, in the same struct (and
C allows this).

I don't really know what is the best practice for constants, maybe Kees
has some pointers?

Maybe what you have is the best we can do.

Yours,
Linus Walleij