Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
MIME-Version: 1.0
References: <20231101025802.3744-1-tychang@realtek.com> <20231101025802.3744-2-tychang@realtek.com>
 <CACRpkdZiKCiVsmkPnJRW1c5SBfzNmBqCH9bS8XsgUszD6H=vPg@mail.gmail.com> <12ada22542cf4b2991da2e8c2858c5d7@realtek.com>
In-Reply-To: <12ada22542cf4b2991da2e8c2858c5d7@realtek.com>
From:   Linus Walleij <linus.walleij@linaro.org>
Date:   Thu, 2 Nov 2023 15:52:16 +0100
Message-ID: <CACRpkdYcK+sr5Ohh5su1cYv8R6BtKSxUUS-rAZP_iWmXbkL-Ug@mail.gmail.com>
Subject: Re: [PATCH 1/2] gpio: realtek: Add GPIO support for RTD SoC variants
To:     =?UTF-8?B?VFlfQ2hhbmdb5by15a2Q6YC4XQ==?= <tychang@realtek.com>
Cc:     Bartosz Golaszewski <brgl@bgdev.pl>,
        Andy Shevchenko <andy@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Kees Cook <keescook@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Nov 2, 2023 at 1:40=E2=80=AFPM TY_Chang[=E5=BC=B5=E5=AD=90=E9=80=B8=
] <tychang@realtek.com> wrote:

> >On Wed, Nov 1, 2023 at 3:58=E2=80=AFAM Tzuyi Chang <tychang@realtek.com>=
 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 bet=
ter?
>
> 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 ou=
tside 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=3DKees

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