| Message ID | 20180627055959.21001-1-niklas.soderlund+renesas@ragnatech.se |
|---|---|
| State | New |
| Headers | show |
| Series | pinctrl: sh-pfc: fix a null pointer dereference of drive strength information | expand |
Hi Niklas, On Wed, Jun 27, 2018 at 8:01 AM Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote: > Not all SoCs describes the drive strength registers. When reading the > sysfs pinconf-pins file on such a SoC this results in a null pointer > dereference. Protect against this dereference and allow reading of the > pinconf-pins by adding a check if the drive strength registers are > described or not. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Thanks for your patch! > This was found on the Eagle board and is based on the latest > renesas/devel branch. I think the real issue is pfc-r8a77990.c setting SH_PFC_PIN_CFG_DRIVE_STRENGTH without providing sh_pfc.drive_regs[]. Without that flag set, sh_pfc_pinconf_validate(..., PIN_CONFIG_DRIVE_STRENGTH) would cause an earlier failure. > --- a/drivers/pinctrl/sh-pfc/pinctrl.c > +++ b/drivers/pinctrl/sh-pfc/pinctrl.c > @@ -483,6 +483,9 @@ static u32 sh_pfc_pinconf_find_drive_strength_reg(struct sh_pfc *pfc, > const struct pinmux_drive_reg *reg; > unsigned int i; > > + if (!pfc->info->drive_regs) > + return 0; > + > for (reg = pfc->info->drive_regs; reg->reg; ++reg) { > for (i = 0; i < ARRAY_SIZE(reg->fields); ++i) { > field = ®->fields[i]; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert, On 2018-06-27 10:27:54 +0200, Geert Uytterhoeven wrote: > Hi Niklas, > > On Wed, Jun 27, 2018 at 8:01 AM Niklas Söderlund > <niklas.soderlund+renesas@ragnatech.se> wrote: > > Not all SoCs describes the drive strength registers. When reading the > > sysfs pinconf-pins file on such a SoC this results in a null pointer > > dereference. Protect against this dereference and allow reading of the > > pinconf-pins by adding a check if the drive strength registers are > > described or not. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > Thanks for your patch! > > > This was found on the Eagle board and is based on the latest > > renesas/devel branch. > > I think the real issue is pfc-r8a77990.c setting SH_PFC_PIN_CFG_DRIVE_STRENGTH > without providing sh_pfc.drive_regs[]. > Without that flag set, sh_pfc_pinconf_validate(..., PIN_CONFIG_DRIVE_STRENGTH) > would cause an earlier failure. Ahh I see, thanks for the pointer. I will explore this option as it seems like a nicer solution, thanks! > > > --- a/drivers/pinctrl/sh-pfc/pinctrl.c > > +++ b/drivers/pinctrl/sh-pfc/pinctrl.c > > @@ -483,6 +483,9 @@ static u32 sh_pfc_pinconf_find_drive_strength_reg(struct sh_pfc *pfc, > > const struct pinmux_drive_reg *reg; > > unsigned int i; > > > > + if (!pfc->info->drive_regs) > > + return 0; > > + > > for (reg = pfc->info->drive_regs; reg->reg; ++reg) { > > for (i = 0; i < ARRAY_SIZE(reg->fields); ++i) { > > field = ®->fields[i]; > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Niklas, On Thu, Jun 28, 2018 at 2:41 AM Niklas Söderlund <niklas.soderlund@ragnatech.se> wrote: > On 2018-06-27 10:27:54 +0200, Geert Uytterhoeven wrote: > > On Wed, Jun 27, 2018 at 8:01 AM Niklas Söderlund > > <niklas.soderlund+renesas@ragnatech.se> wrote: > > > Not all SoCs describes the drive strength registers. When reading the > > > sysfs pinconf-pins file on such a SoC this results in a null pointer > > > dereference. Protect against this dereference and allow reading of the > > > pinconf-pins by adding a check if the drive strength registers are > > > described or not. > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > > > Thanks for your patch! > > > > > This was found on the Eagle board and is based on the latest > > > renesas/devel branch. > > > > I think the real issue is pfc-r8a77990.c setting SH_PFC_PIN_CFG_DRIVE_STRENGTH > > without providing sh_pfc.drive_regs[]. > > Without that flag set, sh_pfc_pinconf_validate(..., PIN_CONFIG_DRIVE_STRENGTH) > > would cause an earlier failure. > > Ahh I see, thanks for the pointer. I will explore this option as it > seems like a nicer solution, thanks! To be 100% clear: the proper solution is to add the missing drive_regs[], not to remove the flags ;-) Gr{oetje,eeting}s, Geert
Hi Geert, On 2018-06-28 08:57:40 +0200, Geert Uytterhoeven wrote: > Hi Niklas, > > On Thu, Jun 28, 2018 at 2:41 AM Niklas Söderlund > <niklas.soderlund@ragnatech.se> wrote: > > On 2018-06-27 10:27:54 +0200, Geert Uytterhoeven wrote: > > > On Wed, Jun 27, 2018 at 8:01 AM Niklas Söderlund > > > <niklas.soderlund+renesas@ragnatech.se> wrote: > > > > Not all SoCs describes the drive strength registers. When reading the > > > > sysfs pinconf-pins file on such a SoC this results in a null pointer > > > > dereference. Protect against this dereference and allow reading of the > > > > pinconf-pins by adding a check if the drive strength registers are > > > > described or not. > > > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > > > > > Thanks for your patch! > > > > > > > This was found on the Eagle board and is based on the latest > > > > renesas/devel branch. > > > > > > I think the real issue is pfc-r8a77990.c setting SH_PFC_PIN_CFG_DRIVE_STRENGTH > > > without providing sh_pfc.drive_regs[]. > > > Without that flag set, sh_pfc_pinconf_validate(..., PIN_CONFIG_DRIVE_STRENGTH) > > > would cause an earlier failure. > > > > Ahh I see, thanks for the pointer. I will explore this option as it > > seems like a nicer solution, thanks! > > To be 100% clear: the proper solution is to add the missing drive_regs[], > not to remove the flags ;-) Normally I would agree with you, but V3M have no drive strength registered described in the datasheet so in this instance I do think the correct fix is to remove the flags :-) > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Niklas, On Mon, Jul 2, 2018 at 9:48 AM Niklas Söderlund <niklas.soderlund@ragnatech.se> wrote: > On 2018-06-28 08:57:40 +0200, Geert Uytterhoeven wrote: > > On Thu, Jun 28, 2018 at 2:41 AM Niklas Söderlund > > <niklas.soderlund@ragnatech.se> wrote: > > > On 2018-06-27 10:27:54 +0200, Geert Uytterhoeven wrote: > > > > On Wed, Jun 27, 2018 at 8:01 AM Niklas Söderlund > > > > <niklas.soderlund+renesas@ragnatech.se> wrote: > > > > > Not all SoCs describes the drive strength registers. When reading the > > > > > sysfs pinconf-pins file on such a SoC this results in a null pointer > > > > > dereference. Protect against this dereference and allow reading of the > > > > > pinconf-pins by adding a check if the drive strength registers are > > > > > described or not. > > > > > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > > > > > > > Thanks for your patch! > > > > > > > > > This was found on the Eagle board and is based on the latest > > > > > renesas/devel branch. > > > > > > > > I think the real issue is pfc-r8a77990.c setting SH_PFC_PIN_CFG_DRIVE_STRENGTH > > > > without providing sh_pfc.drive_regs[]. > > > > Without that flag set, sh_pfc_pinconf_validate(..., PIN_CONFIG_DRIVE_STRENGTH) > > > > would cause an earlier failure. > > > > > > Ahh I see, thanks for the pointer. I will explore this option as it > > > seems like a nicer solution, thanks! > > > > To be 100% clear: the proper solution is to add the missing drive_regs[], > > not to remove the flags ;-) > > Normally I would agree with you, but V3M have no drive strength > registered described in the datasheet so in this instance I do think the > correct fix is to remove the flags :-) Thanks, hard evidence is a good way to convince me ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c index 654dc20e171b9363..ef837676d0312e8d 100644 --- a/drivers/pinctrl/sh-pfc/pinctrl.c +++ b/drivers/pinctrl/sh-pfc/pinctrl.c @@ -483,6 +483,9 @@ static u32 sh_pfc_pinconf_find_drive_strength_reg(struct sh_pfc *pfc, const struct pinmux_drive_reg *reg; unsigned int i; + if (!pfc->info->drive_regs) + return 0; + for (reg = pfc->info->drive_regs; reg->reg; ++reg) { for (i = 0; i < ARRAY_SIZE(reg->fields); ++i) { field = ®->fields[i];
Not all SoCs describes the drive strength registers. When reading the sysfs pinconf-pins file on such a SoC this results in a null pointer dereference. Protect against this dereference and allow reading of the pinconf-pins by adding a check if the drive strength registers are described or not. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/pinctrl/sh-pfc/pinctrl.c | 3 +++ 1 file changed, 3 insertions(+) --- Hi Geert, This was found on the Eagle board and is based on the latest renesas/devel branch. Regards, Niklas