linux-kernel - Re: [PATCH v1] pinctrl: mediatek: fix global-out-of-bounds issue

Open Source and information security mailing list archives

Message-ID: <CAGXv+5EWCkcC51nNF4k1FgEsVhuiQADCiOcdSLtU9yMb-8Q0GA@mail.gmail.com>
Date:   Mon, 8 Nov 2021 16:32:59 +0800
From:   Chen-Yu Tsai <wenst@...omium.org>
To:     Zhiyong Tao <zhiyong.tao@...iatek.com>
Cc:     robh+dt@...nel.org, linus.walleij@...aro.org, mark.rutland@....com,
        matthias.bgg@...il.com, sean.wang@...nel.org,
        srv_heupstream@...iatek.com, hui.liu@...iatek.com,
        light.hsieh@...iatek.com, sean.wang@...iatek.com,
        seiya.wang@...iatek.com, rex-bc.chen@...iatek.com,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org, linux-gpio@...r.kernel.org,
        Guodong Liu <guodong.liu@...iatek.corp-partner.google.com>
Subject: Re: [PATCH v1] pinctrl: mediatek: fix global-out-of-bounds issue

On Thu, Nov 4, 2021 at 7:24 PM Zhiyong Tao <zhiyong.tao@...iatek.com> wrote:

This should be tagged as v2. Your first submission, not tagged with any
version or RFC/RFT, is v1.

> When eint virtual eint number is greater than gpio number,
> it maybe produce 'desc[eint_n]' size globle-out-of-bounds issue.
>
> Signed-off-by: Zhiyong Tao <zhiyong.tao@...iatek.com>
> Signed-off-by: Guodong Liu <guodong.liu@...iatek.corp-partner.google.com>

Guodong's Signed-off-by here is confusing, as they are not the author nor
the submitter, and as far as the mailing list is concerned they have not
handled this patch either.

Looks like the original patch is from https://crrev.com/c/3233623 ?

It's customary to preserve the authorship and append your Signed-off-by
when submitting patches written by others.

> ---
>  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> index 45ebdeba985a..12163d3c4bcb 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> @@ -285,8 +285,12 @@ static int mtk_xt_get_gpio_n(void *data, unsigned long eint_n,
>         desc = (const struct mtk_pin_desc *)hw->soc->pins;
>         *gpio_chip = &hw->chip;
>
> -       /* Be greedy to guess first gpio_n is equal to eint_n */
> -       if (desc[eint_n].eint.eint_n == eint_n)
> +       /*
> +        * Be greedy to guess first gpio_n is equal to eint_n.
> +        * Only eint virtual eint number is greater than gpio number.
> +        */
> +       if (hw->soc->npins > eint_n &&

Nit: I believe it's more common to have the variable on the left, and the
invariable on the right, but maybe it's just me:

    if (eint_n < hw->soc->npins && ...

Either way is OK I guess.

ChenYu

> +           desc[eint_n].eint.eint_n == eint_n)
>                 *gpio_n = eint_n;
>         else
>                 *gpio_n = mtk_xt_find_eint_num(hw, eint_n);
> --
> 2.25.1
>
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives