diff mbox

input: tegra-kbc: add NULL check on of_match_device() return value

Message ID 20170707062758.GA17584@embeddedgus (mailing list archive)
State New, archived
Headers show

Commit Message

Gustavo A. R. Silva July 7, 2017, 6:27 a.m. UTC
Check return value from call to of_match_device()
in order to prevent a NULL pointer dereference.

In case of NULL print error message and return -ENODEV

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/input/keyboard/tegra-kbc.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Jon Hunter July 7, 2017, 7:42 a.m. UTC | #1
On 07/07/17 07:27, Gustavo A. R. Silva wrote:
> Check return value from call to of_match_device()
> in order to prevent a NULL pointer dereference.
> 
> In case of NULL print error message and return -ENODEV
> 
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> ---
>  drivers/input/keyboard/tegra-kbc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c
> index 0c07e10..742c5ac 100644
> --- a/drivers/input/keyboard/tegra-kbc.c
> +++ b/drivers/input/keyboard/tegra-kbc.c
> @@ -617,6 +617,10 @@ static int tegra_kbc_probe(struct platform_device *pdev)
>  	const struct of_device_id *match;
>  
>  	match = of_match_device(tegra_kbc_of_match, &pdev->dev);
> +	if (!match) {
> +		dev_err(&pdev->dev, "failed to match device\n");
> +		return -ENODEV;
> +	}

Given that Tegra always uses device-tree, I believe that this cannot
happen and so this additional check is not needed.

Jon
Thierry Reding July 7, 2017, 9:30 a.m. UTC | #2
On Fri, Jul 07, 2017 at 08:42:32AM +0100, Jon Hunter wrote:
> 
> On 07/07/17 07:27, Gustavo A. R. Silva wrote:
> > Check return value from call to of_match_device()
> > in order to prevent a NULL pointer dereference.
> > 
> > In case of NULL print error message and return -ENODEV
> > 
> > Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> > ---
> >  drivers/input/keyboard/tegra-kbc.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c
> > index 0c07e10..742c5ac 100644
> > --- a/drivers/input/keyboard/tegra-kbc.c
> > +++ b/drivers/input/keyboard/tegra-kbc.c
> > @@ -617,6 +617,10 @@ static int tegra_kbc_probe(struct platform_device *pdev)
> >  	const struct of_device_id *match;
> >  
> >  	match = of_match_device(tegra_kbc_of_match, &pdev->dev);
> > +	if (!match) {
> > +		dev_err(&pdev->dev, "failed to match device\n");
> > +		return -ENODEV;
> > +	}
> 
> Given that Tegra always uses device-tree, I believe that this cannot
> happen and so this additional check is not needed.

I think you can make it happen if you manually create the platform
device with a name matching that of the driver. But you really shouldn't
be doing that, so might as well let it crash and burn so that people
realize their mistake early.

Errors are easily overlooked, crashes are not.

Thierry
Dmitry Torokhov July 10, 2017, 5:42 p.m. UTC | #3
On Fri, Jul 07, 2017 at 11:30:41AM +0200, Thierry Reding wrote:
> On Fri, Jul 07, 2017 at 08:42:32AM +0100, Jon Hunter wrote:
> > 
> > On 07/07/17 07:27, Gustavo A. R. Silva wrote:
> > > Check return value from call to of_match_device()
> > > in order to prevent a NULL pointer dereference.
> > > 
> > > In case of NULL print error message and return -ENODEV
> > > 
> > > Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> > > ---
> > >  drivers/input/keyboard/tegra-kbc.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c
> > > index 0c07e10..742c5ac 100644
> > > --- a/drivers/input/keyboard/tegra-kbc.c
> > > +++ b/drivers/input/keyboard/tegra-kbc.c
> > > @@ -617,6 +617,10 @@ static int tegra_kbc_probe(struct platform_device *pdev)
> > >  	const struct of_device_id *match;
> > >  
> > >  	match = of_match_device(tegra_kbc_of_match, &pdev->dev);
> > > +	if (!match) {
> > > +		dev_err(&pdev->dev, "failed to match device\n");
> > > +		return -ENODEV;
> > > +	}
> > 
> > Given that Tegra always uses device-tree, I believe that this cannot
> > happen and so this additional check is not needed.
> 
> I think you can make it happen if you manually create the platform
> device with a name matching that of the driver. But you really shouldn't
> be doing that, so might as well let it crash and burn so that people
> realize their mistake early.
> 
> Errors are easily overlooked, crashes are not.

Agree.

Thanks.
Gustavo A. R. Silva July 10, 2017, 7:24 p.m. UTC | #4
Hi all,

Quoting Dmitry Torokhov <dmitry.torokhov@gmail.com>:

> On Fri, Jul 07, 2017 at 11:30:41AM +0200, Thierry Reding wrote:
>> On Fri, Jul 07, 2017 at 08:42:32AM +0100, Jon Hunter wrote:
>> >
>> > On 07/07/17 07:27, Gustavo A. R. Silva wrote:
>> > > Check return value from call to of_match_device()
>> > > in order to prevent a NULL pointer dereference.
>> > >
>> > > In case of NULL print error message and return -ENODEV
>> > >
>> > > Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> > > ---
>> > >  drivers/input/keyboard/tegra-kbc.c | 4 ++++
>> > >  1 file changed, 4 insertions(+)
>> > >
>> > > diff --git a/drivers/input/keyboard/tegra-kbc.c  
>> b/drivers/input/keyboard/tegra-kbc.c
>> > > index 0c07e10..742c5ac 100644
>> > > --- a/drivers/input/keyboard/tegra-kbc.c
>> > > +++ b/drivers/input/keyboard/tegra-kbc.c
>> > > @@ -617,6 +617,10 @@ static int tegra_kbc_probe(struct  
>> platform_device *pdev)
>> > >  	const struct of_device_id *match;
>> > >
>> > >  	match = of_match_device(tegra_kbc_of_match, &pdev->dev);
>> > > +	if (!match) {
>> > > +		dev_err(&pdev->dev, "failed to match device\n");
>> > > +		return -ENODEV;
>> > > +	}
>> >
>> > Given that Tegra always uses device-tree, I believe that this cannot
>> > happen and so this additional check is not needed.
>>
>> I think you can make it happen if you manually create the platform
>> device with a name matching that of the driver. But you really shouldn't
>> be doing that, so might as well let it crash and burn so that people
>> realize their mistake early.
>>
>> Errors are easily overlooked, crashes are not.
>
> Agree.
>

I get it.

Thanks for clarifying.
--
Gustavo A. R. Silva





--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gustavo A. R. Silva July 10, 2017, 7:27 p.m. UTC | #5
Hi all,

Quoting Thierry Reding <thierry.reding@gmail.com>:

> On Fri, Jul 07, 2017 at 08:42:32AM +0100, Jon Hunter wrote:
>>
>> On 07/07/17 07:27, Gustavo A. R. Silva wrote:
>> > Check return value from call to of_match_device()
>> > in order to prevent a NULL pointer dereference.
>> >
>> > In case of NULL print error message and return -ENODEV
>> >
>> > Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> > ---
>> >  drivers/input/keyboard/tegra-kbc.c | 4 ++++
>> >  1 file changed, 4 insertions(+)
>> >
>> > diff --git a/drivers/input/keyboard/tegra-kbc.c  
>> b/drivers/input/keyboard/tegra-kbc.c
>> > index 0c07e10..742c5ac 100644
>> > --- a/drivers/input/keyboard/tegra-kbc.c
>> > +++ b/drivers/input/keyboard/tegra-kbc.c
>> > @@ -617,6 +617,10 @@ static int tegra_kbc_probe(struct  
>> platform_device *pdev)
>> >  	const struct of_device_id *match;
>> >
>> >  	match = of_match_device(tegra_kbc_of_match, &pdev->dev);
>> > +	if (!match) {
>> > +		dev_err(&pdev->dev, "failed to match device\n");
>> > +		return -ENODEV;
>> > +	}
>>
>> Given that Tegra always uses device-tree, I believe that this cannot
>> happen and so this additional check is not needed.
>
> I think you can make it happen if you manually create the platform
> device with a name matching that of the driver. But you really shouldn't
> be doing that, so might as well let it crash and burn so that people
> realize their mistake early.
>
> Errors are easily overlooked, crashes are not.
>

I get it.

Thanks for clarifying.
--
Gustavo A. R. Silva






--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c
index 0c07e10..742c5ac 100644
--- a/drivers/input/keyboard/tegra-kbc.c
+++ b/drivers/input/keyboard/tegra-kbc.c
@@ -617,6 +617,10 @@  static int tegra_kbc_probe(struct platform_device *pdev)
 	const struct of_device_id *match;
 
 	match = of_match_device(tegra_kbc_of_match, &pdev->dev);
+	if (!match) {
+		dev_err(&pdev->dev, "failed to match device\n");
+		return -ENODEV;
+	}
 
 	kbc = devm_kzalloc(&pdev->dev, sizeof(*kbc), GFP_KERNEL);
 	if (!kbc) {