2012-11-15 04:51:48

by Axel Lin

[permalink] [raw]
Subject: [PATCH 1/4] pinctrl: dove: Prevent NULL dereference if of_match_device returns NULL

of_match_device() may return NULL.

Signed-off-by: Axel Lin <[email protected]>
---
drivers/pinctrl/mvebu/pinctrl-dove.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-dove.c b/drivers/pinctrl/mvebu/pinctrl-dove.c
index ffe74b2..a8b9b42 100644
--- a/drivers/pinctrl/mvebu/pinctrl-dove.c
+++ b/drivers/pinctrl/mvebu/pinctrl-dove.c
@@ -578,8 +578,12 @@ static struct of_device_id dove_pinctrl_of_match[] __devinitdata = {

static int __devinit dove_pinctrl_probe(struct platform_device *pdev)
{
- const struct of_device_id *match =
- of_match_device(dove_pinctrl_of_match, &pdev->dev);
+ const struct of_device_id *match;
+
+ match = of_match_device(dove_pinctrl_of_match, &pdev->dev);
+ if (!match)
+ return -ENODEV;
+
pdev->dev.platform_data = match->data;

/*
--
1.7.9.5



2012-11-15 04:52:47

by Axel Lin

[permalink] [raw]
Subject: [PATCH 2/4] pinctrl: kirkwood: Prevent NULL dereference if of_match_device returns NULL

of_match_device() may return NULL.

Signed-off-by: Axel Lin <[email protected]>
---
drivers/pinctrl/mvebu/pinctrl-kirkwood.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-kirkwood.c b/drivers/pinctrl/mvebu/pinctrl-kirkwood.c
index 9a74ef6..fe885ca 100644
--- a/drivers/pinctrl/mvebu/pinctrl-kirkwood.c
+++ b/drivers/pinctrl/mvebu/pinctrl-kirkwood.c
@@ -444,8 +444,11 @@ static struct of_device_id kirkwood_pinctrl_of_match[] __devinitdata = {

static int __devinit kirkwood_pinctrl_probe(struct platform_device *pdev)
{
- const struct of_device_id *match =
- of_match_device(kirkwood_pinctrl_of_match, &pdev->dev);
+ const struct of_device_id *match;
+
+ match = of_match_device(kirkwood_pinctrl_of_match, &pdev->dev);
+ if (!match)
+ return -ENODEV;
pdev->dev.platform_data = match->data;
return mvebu_pinctrl_probe(pdev);
}
--
1.7.9.5


2012-11-15 04:54:36

by Axel Lin

[permalink] [raw]
Subject: [PATCH 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

of_match_device() may return NULL.

Signed-off-by: Axel Lin <[email protected]>
---
drivers/pinctrl/pinctrl-at91.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 8490a55..32006c8 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
uint32_t *tmp;
struct device_node *np = pdev->dev.of_node;
struct device_node *child;
+ const struct of_device_id *match;

if (!np)
return -ENODEV;

+ match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+ if (!match)
+ return -ENODEV;
+
info->dev = &pdev->dev;
- info->ops = (struct at91_pinctrl_mux_ops*)
- of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+ info->ops = (struct at91_pinctrl_mux_ops *) match->data;
+
at91_pinctrl_child_count(info, np);

if (info->nbanks < 1) {
@@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
static int __devinit at91_gpio_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
+ const struct of_device_id *match;
struct resource *res;
struct at91_gpio_chip *at91_chip = NULL;
struct gpio_chip *chip;
@@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
goto err;
}

- at91_chip->ops = (struct at91_pinctrl_mux_ops*)
- of_match_device(at91_gpio_of_match, &pdev->dev)->data;
+ match = of_match_device(at91_gpio_of_match, &pdev->dev);
+ if (!match)
+ return -ENODEV;
+
+ at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
at91_chip->pioc_virq = irq;
at91_chip->pioc_idx = alias_idx;

--
1.7.9.5


2012-11-15 04:56:14

by Axel Lin

[permalink] [raw]
Subject: [PATCH 4/4] pinctrl: nomadik: Prevent NULL dereference if of_match_device returns NULL

of_match_device() may return NULL.

Signed-off-by: Axel Lin <[email protected]>
---
drivers/pinctrl/pinctrl-nomadik.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
index 238060e..40bd1b3 100644
--- a/drivers/pinctrl/pinctrl-nomadik.c
+++ b/drivers/pinctrl/pinctrl-nomadik.c
@@ -1863,9 +1863,14 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)

if (platid)
version = platid->driver_data;
- else if (np)
- version = (unsigned int)
- of_match_device(nmk_pinctrl_match, &pdev->dev)->data;
+ else if (np) {
+ const struct of_device_id *match;
+
+ match = of_match_device(nmk_pinctrl_match, &pdev->dev);
+ if (!match)
+ return -ENODEV;
+ version = (unsigned int) match->data;
+ }

/* Poke in other ASIC variants here */
if (version == PINCTRL_NMK_STN8815)
--
1.7.9.5


2012-11-15 04:58:09

by Axel Lin

[permalink] [raw]
Subject: [PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

of_match_device() may return NULL.

Signed-off-by: Axel Lin <[email protected]>
---
This resend CC Jean-Christophe.

drivers/pinctrl/pinctrl-at91.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 8490a55..32006c8 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
uint32_t *tmp;
struct device_node *np = pdev->dev.of_node;
struct device_node *child;
+ const struct of_device_id *match;

if (!np)
return -ENODEV;

+ match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+ if (!match)
+ return -ENODEV;
+
info->dev = &pdev->dev;
- info->ops = (struct at91_pinctrl_mux_ops*)
- of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+ info->ops = (struct at91_pinctrl_mux_ops *) match->data;
+
at91_pinctrl_child_count(info, np);

if (info->nbanks < 1) {
@@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
static int __devinit at91_gpio_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
+ const struct of_device_id *match;
struct resource *res;
struct at91_gpio_chip *at91_chip = NULL;
struct gpio_chip *chip;
@@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
goto err;
}

- at91_chip->ops = (struct at91_pinctrl_mux_ops*)
- of_match_device(at91_gpio_of_match, &pdev->dev)->data;
+ match = of_match_device(at91_gpio_of_match, &pdev->dev);
+ if (!match)
+ return -ENODEV;
+
+ at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
at91_chip->pioc_virq = irq;
at91_chip->pioc_idx = alias_idx;

--
1.7.9.5


2012-11-15 07:35:22

by Thomas Petazzoni

[permalink] [raw]
Subject: Re: [PATCH 1/4] pinctrl: dove: Prevent NULL dereference if of_match_device returns NULL

Dear Axel Lin,

On Thu, 15 Nov 2012 12:51:36 +0800, Axel Lin wrote:
> of_match_device() may return NULL.
>
> Signed-off-by: Axel Lin <[email protected]>

Could you detail under what conditions of_match_device() may return
NULL in the specific case of this driver? This of_match_device() call
is using the same dove_pinctrl_of_match array that is used to ->probe()
this driver. So I don't see how you can get into ->probe() without
having a matching entry.

Am I missing something?

Best regards,

Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

2012-11-15 08:23:28

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

On 11/15/2012 05:58 AM, Axel Lin :
> of_match_device() may return NULL.
>
> Signed-off-by: Axel Lin <[email protected]>

Seems sensible,

Acked-by: Nicolas Ferre <[email protected]>

> ---
> This resend CC Jean-Christophe.
>
> drivers/pinctrl/pinctrl-at91.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 8490a55..32006c8 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
> uint32_t *tmp;
> struct device_node *np = pdev->dev.of_node;
> struct device_node *child;
> + const struct of_device_id *match;
>
> if (!np)
> return -ENODEV;
>
> + match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> + if (!match)
> + return -ENODEV;
> +
> info->dev = &pdev->dev;
> - info->ops = (struct at91_pinctrl_mux_ops*)
> - of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> + info->ops = (struct at91_pinctrl_mux_ops *) match->data;
> +
> at91_pinctrl_child_count(info, np);
>
> if (info->nbanks < 1) {
> @@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
> static int __devinit at91_gpio_probe(struct platform_device *pdev)
> {
> struct device_node *np = pdev->dev.of_node;
> + const struct of_device_id *match;
> struct resource *res;
> struct at91_gpio_chip *at91_chip = NULL;
> struct gpio_chip *chip;
> @@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
> goto err;
> }
>
> - at91_chip->ops = (struct at91_pinctrl_mux_ops*)
> - of_match_device(at91_gpio_of_match, &pdev->dev)->data;
> + match = of_match_device(at91_gpio_of_match, &pdev->dev);
> + if (!match)
> + return -ENODEV;
> +
> + at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
> at91_chip->pioc_virq = irq;
> at91_chip->pioc_idx = alias_idx;
>
>


--
Nicolas Ferre

Subject: Re: [PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

On 12:58 Thu 15 Nov , Axel Lin wrote:
> of_match_device() may return NULL.
this is not possible on at91

and I do a oups here as if we have a NULL pointer which means the driver is
wrong

Best Regards,
J.
>
> Signed-off-by: Axel Lin <[email protected]>
> ---
> This resend CC Jean-Christophe.
>
> drivers/pinctrl/pinctrl-at91.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 8490a55..32006c8 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
> uint32_t *tmp;
> struct device_node *np = pdev->dev.of_node;
> struct device_node *child;
> + const struct of_device_id *match;
>
> if (!np)
> return -ENODEV;
>
> + match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> + if (!match)
> + return -ENODEV;
> +
> info->dev = &pdev->dev;
> - info->ops = (struct at91_pinctrl_mux_ops*)
> - of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> + info->ops = (struct at91_pinctrl_mux_ops *) match->data;
> +
> at91_pinctrl_child_count(info, np);
>
> if (info->nbanks < 1) {
> @@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
> static int __devinit at91_gpio_probe(struct platform_device *pdev)
> {
> struct device_node *np = pdev->dev.of_node;
> + const struct of_device_id *match;
> struct resource *res;
> struct at91_gpio_chip *at91_chip = NULL;
> struct gpio_chip *chip;
> @@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
> goto err;
> }
>
> - at91_chip->ops = (struct at91_pinctrl_mux_ops*)
> - of_match_device(at91_gpio_of_match, &pdev->dev)->data;
> + match = of_match_device(at91_gpio_of_match, &pdev->dev);
> + if (!match)
> + return -ENODEV;
> +
> + at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
> at91_chip->pioc_virq = irq;
> at91_chip->pioc_idx = alias_idx;
>
> --
> 1.7.9.5
>
>
>

2012-11-15 13:56:53

by Thomas Petazzoni

[permalink] [raw]
Subject: Re: [PATCH 1/4] pinctrl: dove: Prevent NULL dereference if of_match_device returns NULL

Dear Axel Lin,

On Thu, 15 Nov 2012 21:44:07 +0800, Axel Lin wrote:

> BTW, I found there is no way to compile the dove and kirkwood pinctrl
> drivers.
> I need add below patch to compile these two drivers, how do you think about
> below patch?
> ( In dove, the USE_OF is optional, it is selected only when MACH_DOVE_DT is
> enabled )
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 14f8160..dda6785 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -531,6 +531,8 @@ config ARCH_DOVE
> select CPU_V7
> select GENERIC_CLOCKEVENTS
> select MIGHT_HAVE_PCI
> + select PINCTRL
> + select PINCTRL_DOVE if USE_OF
> select PLAT_ORION_LEGACY
> select USB_ARCH_HAS_EHCI
> help
> @@ -542,6 +544,8 @@ config ARCH_KIRKWOOD
> select CPU_FEROCEON
> select GENERIC_CLOCKEVENTS
> select PCI
> + select PINCTRL
> + select PINCTRL_KIRKWOOD
> select PLAT_ORION_LEGACY
> help
> Support for the following Marvell Kirkwood series SoCs:

See "[PATCH 1/5] ARM: Kirkwood: Allow use of pinctrl" and "[PATCH 4/5]
ARM: Dove: Make use of pinctrl driver" which have been posted by Andrew
Lunn on October, 24th.

Best regards,

Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

2012-11-15 14:42:58

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 4/4] pinctrl: nomadik: Prevent NULL dereference if of_match_device returns NULL

On Thu, Nov 15, 2012 at 5:56 AM, Axel Lin <[email protected]> wrote:

> of_match_device() may return NULL.
>
> Signed-off-by: Axel Lin <[email protected]>
> ---
> drivers/pinctrl/pinctrl-nomadik.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
> index 238060e..40bd1b3 100644
> --- a/drivers/pinctrl/pinctrl-nomadik.c
> +++ b/drivers/pinctrl/pinctrl-nomadik.c
> @@ -1863,9 +1863,14 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
>
> if (platid)
> version = platid->driver_data;
> - else if (np)
> - version = (unsigned int)
> - of_match_device(nmk_pinctrl_match, &pdev->dev)->data;
> + else if (np) {
> + const struct of_device_id *match;
> +
> + match = of_match_device(nmk_pinctrl_match, &pdev->dev);
> + if (!match)
> + return -ENODEV;
> + version = (unsigned int) match->data;
> + }

AFAICT this can actually happen so patch applied, unless Lee
speaks against it.

Patches 1-3/4 are dropped though, as the maintainers didn't seem
to like them.

Yours,
Linus Walleij

2012-11-15 15:15:47

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 4/4] pinctrl: nomadik: Prevent NULL dereference if of_match_device returns NULL

On Thu, 15 Nov 2012, Linus Walleij wrote:

> On Thu, Nov 15, 2012 at 5:56 AM, Axel Lin <[email protected]> wrote:
>
> > of_match_device() may return NULL.
> >
> > Signed-off-by: Axel Lin <[email protected]>
> > ---
> > drivers/pinctrl/pinctrl-nomadik.c | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
> > index 238060e..40bd1b3 100644
> > --- a/drivers/pinctrl/pinctrl-nomadik.c
> > +++ b/drivers/pinctrl/pinctrl-nomadik.c
> > @@ -1863,9 +1863,14 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
> >
> > if (platid)
> > version = platid->driver_data;
> > - else if (np)
> > - version = (unsigned int)
> > - of_match_device(nmk_pinctrl_match, &pdev->dev)->data;
> > + else if (np) {
> > + const struct of_device_id *match;
> > +
> > + match = of_match_device(nmk_pinctrl_match, &pdev->dev);
> > + if (!match)
> > + return -ENODEV;
> > + version = (unsigned int) match->data;
> > + }
>
> AFAICT this can actually happen so patch applied, unless Lee
> speaks against it.

At the very least it make the code easy to read.

Acked-by: Lee Jones <[email protected]>

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2012-11-15 19:18:45

by Thomas Petazzoni

[permalink] [raw]
Subject: Re: [PATCH 2/4] pinctrl: kirkwood: Prevent NULL dereference if of_match_device returns NULL

Dear Axel Lin,

On Thu, 15 Nov 2012 12:52:39 +0800, Axel Lin wrote:
> of_match_device() may return NULL.
>
> Signed-off-by: Axel Lin <[email protected]>

Why? We already discussed together that this situation could not
happen: if you are in the probe() function, then there *must* be a
match, because the probe() function precisely gets called when there is
a match.

So unless you give more details about what the problem is, I don't
think this patch and the similar one you sent for Dove are useful.

Best regards,

Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

2012-11-16 08:36:50

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

On 11/15/2012 11:00 AM, Jean-Christophe PLAGNIOL-VILLARD :
> On 12:58 Thu 15 Nov , Axel Lin wrote:
>> of_match_device() may return NULL.
> this is not possible on at91
>
> and I do a oups here as if we have a NULL pointer which means the driver is
> wrong

Well, okay, but it does not prevent from adding a supplementary check to
mimic every other pinctrl driver and use a common return path to the
of_match_device() function.

So I am not completely against this patch (that I why I added my Acked-by).


>> Signed-off-by: Axel Lin <[email protected]>
>> ---
>> This resend CC Jean-Christophe.
>>
>> drivers/pinctrl/pinctrl-at91.c | 17 +++++++++++++----
>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
>> index 8490a55..32006c8 100644
>> --- a/drivers/pinctrl/pinctrl-at91.c
>> +++ b/drivers/pinctrl/pinctrl-at91.c
>> @@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
>> uint32_t *tmp;
>> struct device_node *np = pdev->dev.of_node;
>> struct device_node *child;
>> + const struct of_device_id *match;
>>
>> if (!np)
>> return -ENODEV;
>>
>> + match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
>> + if (!match)
>> + return -ENODEV;
>> +
>> info->dev = &pdev->dev;
>> - info->ops = (struct at91_pinctrl_mux_ops*)
>> - of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
>> + info->ops = (struct at91_pinctrl_mux_ops *) match->data;
>> +
>> at91_pinctrl_child_count(info, np);
>>
>> if (info->nbanks < 1) {
>> @@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
>> static int __devinit at91_gpio_probe(struct platform_device *pdev)
>> {
>> struct device_node *np = pdev->dev.of_node;
>> + const struct of_device_id *match;
>> struct resource *res;
>> struct at91_gpio_chip *at91_chip = NULL;
>> struct gpio_chip *chip;
>> @@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
>> goto err;
>> }
>>
>> - at91_chip->ops = (struct at91_pinctrl_mux_ops*)
>> - of_match_device(at91_gpio_of_match, &pdev->dev)->data;
>> + match = of_match_device(at91_gpio_of_match, &pdev->dev);
>> + if (!match)
>> + return -ENODEV;
>> +
>> + at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
>> at91_chip->pioc_virq = irq;
>> at91_chip->pioc_idx = alias_idx;
>>
>> --
>> 1.7.9.5
>>
>>
>>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>


--
Nicolas Ferre

Subject: Re: [PATCH RESEND 3/4] pinctrl: at91: Prevent NULL dereference if of_match_device returns NULL

On 09:36 Fri 16 Nov , Nicolas Ferre wrote:
> On 11/15/2012 11:00 AM, Jean-Christophe PLAGNIOL-VILLARD :
> > On 12:58 Thu 15 Nov , Axel Lin wrote:
> >> of_match_device() may return NULL.
> > this is not possible on at91
> >
> > and I do a oups here as if we have a NULL pointer which means the driver is
> > wrong
>
> Well, okay, but it does not prevent from adding a supplementary check to
> mimic every other pinctrl driver and use a common return path to the
> of_match_device() function.
>
> So I am not completely against this patch (that I why I added my Acked-by).
here it's a silent warning a oops it clear we have a bug

Best Regards,
J.
>
>
> >> Signed-off-by: Axel Lin <[email protected]>
> >> ---
> >> This resend CC Jean-Christophe.
> >>
> >> drivers/pinctrl/pinctrl-at91.c | 17 +++++++++++++----
> >> 1 file changed, 13 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> >> index 8490a55..32006c8 100644
> >> --- a/drivers/pinctrl/pinctrl-at91.c
> >> +++ b/drivers/pinctrl/pinctrl-at91.c
> >> @@ -829,13 +829,18 @@ static int __devinit at91_pinctrl_probe_dt(struct platform_device *pdev,
> >> uint32_t *tmp;
> >> struct device_node *np = pdev->dev.of_node;
> >> struct device_node *child;
> >> + const struct of_device_id *match;
> >>
> >> if (!np)
> >> return -ENODEV;
> >>
> >> + match = of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> >> + if (!match)
> >> + return -ENODEV;
> >> +
> >> info->dev = &pdev->dev;
> >> - info->ops = (struct at91_pinctrl_mux_ops*)
> >> - of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
> >> + info->ops = (struct at91_pinctrl_mux_ops *) match->data;
> >> +
> >> at91_pinctrl_child_count(info, np);
> >>
> >> if (info->nbanks < 1) {
> >> @@ -1359,6 +1364,7 @@ static struct of_device_id at91_gpio_of_match[] __devinitdata = {
> >> static int __devinit at91_gpio_probe(struct platform_device *pdev)
> >> {
> >> struct device_node *np = pdev->dev.of_node;
> >> + const struct of_device_id *match;
> >> struct resource *res;
> >> struct at91_gpio_chip *at91_chip = NULL;
> >> struct gpio_chip *chip;
> >> @@ -1399,8 +1405,11 @@ static int __devinit at91_gpio_probe(struct platform_device *pdev)
> >> goto err;
> >> }
> >>
> >> - at91_chip->ops = (struct at91_pinctrl_mux_ops*)
> >> - of_match_device(at91_gpio_of_match, &pdev->dev)->data;
> >> + match = of_match_device(at91_gpio_of_match, &pdev->dev);
> >> + if (!match)
> >> + return -ENODEV;
> >> +
> >> + at91_chip->ops = (struct at91_pinctrl_mux_ops *) match->data;
> >> at91_chip->pioc_virq = irq;
> >> at91_chip->pioc_idx = alias_idx;
> >>
> >> --
> >> 1.7.9.5
> >>
> >>
> >>
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
> >
>
>
> --
> Nicolas Ferre