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

PATCH In review Gustavo A. R. Silva 4 participants 6 messages Jul 7, 2017 – Jul 10, 2017

Subsystems (4)

HID Tegra architecture support Input drivers Tegra kbc driver

lore

Share & tools

lore

Gustavo A. R. Silva

author

2017-07-07 06:27 UTC #0 lore raw

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

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 <[email protected]>

---
 drivers/input/keyboard/tegra-kbc.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/input/keyboard/tegra-kbc.c +4 -0

index 0c07e10..742c5ac 100644
@@ -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) {

Signature · 1 line

2.5.0

Jon Hunter

maintainer

2017-07-07 07:42 UTC #1 lore raw

Quoted 15 lines · Gustavo A. R. Silva

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-L1vi/[email protected]>
---
 drivers/input/keyboard/tegra-kbc.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/input/keyboard/tegra-kbc.c +0 -0
index 0c07e10..742c5ac 100644

drivers/input/keyboard/tegra-kbc.c static int tegra_kbc_probe(struct platform_device *pdev) · line 623

Quoted 7 lines

drivers/input/keyboard/tegra-kbc.c +4 -0

@@ -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

Signature · 1 line

nvpublic

Thierry Reding

2017-07-07 09:30 UTC #2 lore raw

Quoted 19 lines · Jon Hunter

Quoted 17 lines · Gustavo A. R. Silva
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 <[email protected]>
---
 drivers/input/keyboard/tegra-kbc.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/input/keyboard/tegra-kbc.c +0 -0
index 0c07e10..742c5ac 100644
@@ -617,6 +617,10 @@ static int tegra_kbc_probe(struct platform_device *pdev)
 	const struct of_device_id *match;
 

Quoted 8 lines

drivers/input/keyboard/tegra-kbc.c static int tegra_kbc_probe(struct platform_device *pdev) · line 623
Quoted 5 lines
drivers/input/keyboard/tegra-kbc.c +4 -0
 	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

maintainer

2017-07-10 17:42 UTC #3 lore raw

Quoted 29 lines · Thierry Reding

Quoted 19 lines · Jon Hunter
Quoted 17 lines · Gustavo A. R. Silva
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 <[email protected]>
---
 drivers/input/keyboard/tegra-kbc.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/input/keyboard/tegra-kbc.c +0 -0
index 0c07e10..742c5ac 100644
@@ -617,6 +617,10 @@ static int tegra_kbc_probe(struct platform_device *pdev)
 	const struct of_device_id *match;
 
Quoted 8 lines
drivers/input/keyboard/tegra-kbc.c static int tegra_kbc_probe(struct platform_device *pdev) · line 623
Quoted 5 lines
drivers/input/keyboard/tegra-kbc.c +4 -0
 	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.

Quoted 6 lines

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.

Signature · 1 line

Dmitry

Gustavo A. R. Silva

author

2017-07-10 19:24 UTC #4 lore raw

Hi all,

Quoting Dmitry Torokhov <[email protected]>:

Quoted 32 lines · Thierry Reding

Quoted 31 lines · Jon Hunter
Quoted 13 lines · Gustavo A. R. Silva
Review
Quoted 11 lines
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 <[email protected]>
---
 drivers/input/keyboard/tegra-kbc.c | 4 ++++
 1 file changed, 4 insertions(+)
(quoted diff) +0 -0
b/drivers/input/keyboard/tegra-kbc.c
Quoted 4 lines
drivers/input/keyboard/tegra-kbc.c static int tegra_kbc_probe(struct
Quoted 4 lines
index 0c07e10..742c5ac 100644
drivers/input/keyboard/tegra-kbc.c +0 -0
@@ -617,6 +617,10 @@ static int tegra_kbc_probe(struct  
platform_device *pdev)
Quoted 10 lines
drivers/input/keyboard/tegra-kbc.c static int tegra_kbc_probe(struct platform_device *pdev) · line 623
Quoted 7 lines
drivers/input/keyboard/tegra-kbc.c +4 -0
 	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.

Quoted 9 lines

Quoted 6 lines
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

Gustavo A. R. Silva

author

2017-07-10 19:27 UTC #5 lore raw

In reply to Thierry Reding

Hi all,

Quoting Thierry Reding <[email protected]>:

Quoted 31 lines · Jon Hunter

Quoted 21 lines · Gustavo A. R. Silva
Review
Quoted 11 lines
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 <[email protected]>
---
 drivers/input/keyboard/tegra-kbc.c | 4 ++++
 1 file changed, 4 insertions(+)
(quoted diff) +0 -0
b/drivers/input/keyboard/tegra-kbc.c
drivers/input/keyboard/tegra-kbc.c static int tegra_kbc_probe(struct
Quoted 4 lines
index 0c07e10..742c5ac 100644
drivers/input/keyboard/tegra-kbc.c +0 -0
@@ -617,6 +617,10 @@ static int tegra_kbc_probe(struct  
platform_device *pdev)
Quoted 2 lines
drivers/input/keyboard/tegra-kbc.c +0 -0
 	const struct of_device_id *match;
 
Quoted 8 lines
drivers/input/keyboard/tegra-kbc.c static int tegra_kbc_probe(struct platform_device *pdev) · line 623
Quoted 5 lines
drivers/input/keyboard/tegra-kbc.c +4 -0
 	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.

Quoted 7 lines

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