| Message ID | 20230815075602.10473-3-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Fix NULL pointer dereference in RZ/{G2L,V2M,A2} pinctrl driver | expand |
Hi Biju, On Tue, Aug 15, 2023 at 9:56 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > Fix the below random NULL pointer crash during boot by serializing > groups and functions creation in rzv2m_dt_subnode_to_map() with > mutex lock. > > Crash logs: > pc : __pi_strcmp+0x20/0x140 > lr : pinmux_func_name_to_selector+0x68/0xa4 > Call trace: > __pi_strcmp+0x20/0x140 > pinmux_generic_add_function+0x34/0xcc > rzv2m_dt_subnode_to_map+0x2e4/0x418 > rzv2m_dt_node_to_map+0x15c/0x18c > pinctrl_dt_to_map+0x218/0x37c > create_pinctrl+0x70/0x3d8 > > Fixes: 92a9b8252576 ("pinctrl: renesas: Add RZ/V2M pin and gpio controller driver") > Cc: stable@kernel.org > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v1->v2: > * Updated crash logs as per submitting patches doc. Thanks for your patch! > --- a/drivers/pinctrl/renesas/pinctrl-rzv2m.c > +++ b/drivers/pinctrl/renesas/pinctrl-rzv2m.c > @@ -14,6 +14,7 @@ > #include <linux/gpio/driver.h> > #include <linux/io.h> > #include <linux/module.h> > +#include <linux/mutex.h> > #include <linux/of.h> > #include <linux/platform_device.h> > #include <linux/spinlock.h> > @@ -124,6 +125,7 @@ struct rzv2m_pinctrl { > struct pinctrl_gpio_range gpio_range; > > spinlock_t lock; > + struct mutex mutex; /* serialize adding groups and functions */ I guess it's not too late to add a comment to the other lock? > }; > > static const unsigned int drv_1_8V_group2_uA[] = { 1800, 3800, 7800, 11000 }; > @@ -322,10 +324,12 @@ static int rzv2m_dt_subnode_to_map(struct pinctrl_dev *pctldev, > name = np->name; > } > > + mutex_lock(&pctrl->mutex); I think this is too late, as krealloc_array() above has already released the old map. > /* Register a single pin group listing all the pins we read from DT */ > gsel = pinctrl_generic_add_group(pctldev, name, pins, num_pinmux, NULL); > if (gsel < 0) { > ret = gsel; > + mutex_unlock(&pctrl->mutex); Please add a new label below, to avoid adding a call to mutex_unlock() here. > goto done; > } > > @@ -340,6 +344,8 @@ static int rzv2m_dt_subnode_to_map(struct pinctrl_dev *pctldev, > goto remove_group; > } > > + mutex_unlock(&pctrl->mutex); > + > maps[idx].type = PIN_MAP_TYPE_MUX_GROUP; > maps[idx].data.mux.group = name; > maps[idx].data.mux.function = name; > @@ -351,6 +357,7 @@ static int rzv2m_dt_subnode_to_map(struct pinctrl_dev *pctldev, > > remove_group: > pinctrl_generic_remove_group(pctldev, gsel); ^^ new label here. > + mutex_unlock(&pctrl->mutex); > done: > *index = idx; > kfree(configs); Gr{oetje,eeting}s, Geert
diff --git a/drivers/pinctrl/renesas/pinctrl-rzv2m.c b/drivers/pinctrl/renesas/pinctrl-rzv2m.c index c73784b8b4ba..65718c9bfc0c 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzv2m.c +++ b/drivers/pinctrl/renesas/pinctrl-rzv2m.c @@ -14,6 +14,7 @@ #include <linux/gpio/driver.h> #include <linux/io.h> #include <linux/module.h> +#include <linux/mutex.h> #include <linux/of.h> #include <linux/platform_device.h> #include <linux/spinlock.h> @@ -124,6 +125,7 @@ struct rzv2m_pinctrl { struct pinctrl_gpio_range gpio_range; spinlock_t lock; + struct mutex mutex; /* serialize adding groups and functions */ }; static const unsigned int drv_1_8V_group2_uA[] = { 1800, 3800, 7800, 11000 }; @@ -322,10 +324,12 @@ static int rzv2m_dt_subnode_to_map(struct pinctrl_dev *pctldev, name = np->name; } + mutex_lock(&pctrl->mutex); /* Register a single pin group listing all the pins we read from DT */ gsel = pinctrl_generic_add_group(pctldev, name, pins, num_pinmux, NULL); if (gsel < 0) { ret = gsel; + mutex_unlock(&pctrl->mutex); goto done; } @@ -340,6 +344,8 @@ static int rzv2m_dt_subnode_to_map(struct pinctrl_dev *pctldev, goto remove_group; } + mutex_unlock(&pctrl->mutex); + maps[idx].type = PIN_MAP_TYPE_MUX_GROUP; maps[idx].data.mux.group = name; maps[idx].data.mux.function = name; @@ -351,6 +357,7 @@ static int rzv2m_dt_subnode_to_map(struct pinctrl_dev *pctldev, remove_group: pinctrl_generic_remove_group(pctldev, gsel); + mutex_unlock(&pctrl->mutex); done: *index = idx; kfree(configs); @@ -1065,6 +1072,7 @@ static int rzv2m_pinctrl_probe(struct platform_device *pdev) "failed to enable GPIO clk\n"); spin_lock_init(&pctrl->lock); + mutex_init(&pctrl->mutex); platform_set_drvdata(pdev, pctrl);
Fix the below random NULL pointer crash during boot by serializing groups and functions creation in rzv2m_dt_subnode_to_map() with mutex lock. Crash logs: pc : __pi_strcmp+0x20/0x140 lr : pinmux_func_name_to_selector+0x68/0xa4 Call trace: __pi_strcmp+0x20/0x140 pinmux_generic_add_function+0x34/0xcc rzv2m_dt_subnode_to_map+0x2e4/0x418 rzv2m_dt_node_to_map+0x15c/0x18c pinctrl_dt_to_map+0x218/0x37c create_pinctrl+0x70/0x3d8 Fixes: 92a9b8252576 ("pinctrl: renesas: Add RZ/V2M pin and gpio controller driver") Cc: stable@kernel.org Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v1->v2: * Updated crash logs as per submitting patches doc. --- drivers/pinctrl/renesas/pinctrl-rzv2m.c | 8 ++++++++ 1 file changed, 8 insertions(+)