diff mbox series

[v1] pinctrl: mediatek: fix global-out-of-bounds issue

Message ID 20211104112329.8446-2-zhiyong.tao@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Mediatek pinctrl patch | expand

Commit Message

zhiyong.tao Nov. 4, 2021, 11:23 a.m. UTC
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@mediatek.com>
Signed-off-by: Guodong Liu <guodong.liu@mediatek.corp-partner.google.com>
---
 drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Chen-Yu Tsai Nov. 8, 2021, 8:32 a.m. UTC | #1
On Thu, Nov 4, 2021 at 7:24 PM Zhiyong Tao <zhiyong.tao@mediatek.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@mediatek.com>
> Signed-off-by: Guodong Liu <guodong.liu@mediatek.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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
zhiyong.tao Nov. 9, 2021, 1:43 a.m. UTC | #2
On Mon, 2021-11-08 at 16:32 +0800, Chen-Yu Tsai wrote:
> On Thu, Nov 4, 2021 at 7:24 PM Zhiyong Tao <zhiyong.tao@mediatek.com>
> wrote:
> 
> This should be tagged as v2. Your first submission, not tagged with
> any
> version or RFC/RFT, is v1.

Hi Chen-Yu,

Thanks for your suggestion, so we will send next patch version as V3.

> 
> > 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@mediatek.com>
> > Signed-off-by: Guodong Liu <
> > guodong.liu@mediatek.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.
> 

yes, it is from the original patch "https://crrev.com/c/3233623".
we will preserve the authorship and append my Signed-off-by
in next version.

> > ---
> >  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.
> 
we keep it as 'hw->soc->npins > eint_n', so that is can be consistent
with 'desc[eint_n].eint.eint_n == eint_n'. 

Thanks

> 
> 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@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek
diff mbox series

Patch

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 &&
+	    desc[eint_n].eint.eint_n == eint_n)
 		*gpio_n = eint_n;
 	else
 		*gpio_n = mtk_xt_find_eint_num(hw, eint_n);