* [PATCH] I2c: busses - Fix possible NULL derefrence.
[not found] <CGME20170130050324epcas3p28c9adcffd7bc40e1733cdeef1e5240d5@epcas3p2.samsung.com>
@ 2017-01-30 5:03 ` Shailendra Verma
2017-01-30 7:12 ` Thierry Reding
2017-02-09 16:37 ` Wolfram Sang
0 siblings, 2 replies; 7+ messages in thread
From: Shailendra Verma @ 2017-01-30 5:03 UTC (permalink / raw)
To: Laxman Dewangan, Wolfram Sang, Stephen Warren, Thierry Reding,
Alexandre Courbot, linux-i2c, linux-tegra, linux-kernel,
p.shailesh, ashish.kalra, Shailendra Verma, Shailendra Verma
of_device_get_match_data could return NULL, and so can cause
a NULL pointer dereference later.
Signed-off-by: Shailendra Verma <shailendra.v@samsung.com>
---
drivers/i2c/busses/i2c-tegra.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 4af9bba..93ac1e1 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -920,6 +920,10 @@ static int tegra_i2c_probe(struct platform_device *pdev)
tegra_i2c_parse_dt(i2c_dev);
i2c_dev->hw = of_device_get_match_data(&pdev->dev);
+ if (!i2c_dev->hw) {
+ dev_err(&pdev->dev, "no device match found\n");
+ return -ENODEV;
+ }
i2c_dev->is_dvc = of_device_is_compatible(pdev->dev.of_node,
"nvidia,tegra20-i2c-dvc");
init_completion(&i2c_dev->msg_complete);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] I2c: busses - Fix possible NULL derefrence.
2017-01-30 5:03 ` [PATCH] I2c: busses - Fix possible NULL derefrence Shailendra Verma
@ 2017-01-30 7:12 ` Thierry Reding
2017-01-30 8:07 ` Uwe Kleine-König
2017-02-09 16:37 ` Wolfram Sang
1 sibling, 1 reply; 7+ messages in thread
From: Thierry Reding @ 2017-01-30 7:12 UTC (permalink / raw)
To: Shailendra Verma
Cc: Laxman Dewangan, Wolfram Sang, Stephen Warren, Alexandre Courbot,
linux-i2c, linux-tegra, linux-kernel, p.shailesh, ashish.kalra,
Shailendra Verma
[-- Attachment #1: Type: text/plain, Size: 475 bytes --]
On Mon, Jan 30, 2017 at 10:33:07AM +0530, Shailendra Verma wrote:
> of_device_get_match_data could return NULL, and so can cause
> a NULL pointer dereference later.
>
> Signed-off-by: Shailendra Verma <shailendra.v@samsung.com>
> ---
> drivers/i2c/busses/i2c-tegra.c | 4 ++++
> 1 file changed, 4 insertions(+)
This will never happen. Any match in the OF table that would cause the
->probe() to occur has a valid .data pointer associated with it.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] I2c: busses - Fix possible NULL derefrence.
2017-01-30 7:12 ` Thierry Reding
@ 2017-01-30 8:07 ` Uwe Kleine-König
2017-01-30 8:54 ` Thierry Reding
0 siblings, 1 reply; 7+ messages in thread
From: Uwe Kleine-König @ 2017-01-30 8:07 UTC (permalink / raw)
To: Thierry Reding
Cc: Shailendra Verma, Laxman Dewangan, Wolfram Sang, Stephen Warren,
Alexandre Courbot, linux-i2c, linux-tegra, linux-kernel,
p.shailesh, ashish.kalra, Shailendra Verma
Hello,
On Mon, Jan 30, 2017 at 08:12:17AM +0100, Thierry Reding wrote:
> On Mon, Jan 30, 2017 at 10:33:07AM +0530, Shailendra Verma wrote:
> > of_device_get_match_data could return NULL, and so can cause
> > a NULL pointer dereference later.
> >
> > Signed-off-by: Shailendra Verma <shailendra.v@samsung.com>
> > ---
> > drivers/i2c/busses/i2c-tegra.c | 4 ++++
> > 1 file changed, 4 insertions(+)
>
> This will never happen. Any match in the OF table that would cause the
> ->probe() to occur has a valid .data pointer associated with it.
Theoretically you could (I think) bind that driver to a node with
compatible = "tegra-i2c";
Anyhow, even if today there was no possibility this could happen, that's
something that might easily be changed by a future change. So I doubt
"this will never happen" stays true for sure and being defensive is a
good idea. And even a BUG would be better than a silent NULL pointer
dereference.
Just my € 0.02
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] I2c: busses - Fix possible NULL derefrence.
2017-01-30 8:07 ` Uwe Kleine-König
@ 2017-01-30 8:54 ` Thierry Reding
2017-01-30 11:15 ` Uwe Kleine-König
0 siblings, 1 reply; 7+ messages in thread
From: Thierry Reding @ 2017-01-30 8:54 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Shailendra Verma, Laxman Dewangan, Wolfram Sang, Stephen Warren,
Alexandre Courbot, linux-i2c, linux-tegra, linux-kernel,
p.shailesh, ashish.kalra, Shailendra Verma
[-- Attachment #1: Type: text/plain, Size: 1654 bytes --]
On Mon, Jan 30, 2017 at 09:07:15AM +0100, Uwe Kleine-König wrote:
> Hello,
>
> On Mon, Jan 30, 2017 at 08:12:17AM +0100, Thierry Reding wrote:
> > On Mon, Jan 30, 2017 at 10:33:07AM +0530, Shailendra Verma wrote:
> > > of_device_get_match_data could return NULL, and so can cause
> > > a NULL pointer dereference later.
> > >
> > > Signed-off-by: Shailendra Verma <shailendra.v@samsung.com>
> > > ---
> > > drivers/i2c/busses/i2c-tegra.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> >
> > This will never happen. Any match in the OF table that would cause the
> > ->probe() to occur has a valid .data pointer associated with it.
>
> Theoretically you could (I think) bind that driver to a node with
>
> compatible = "tegra-i2c";
That's not a valid compatible string and I don't think this could end up
anywhere that would make the driver bind. Even if it did I think it'd be
good to crash rather than error out to make it very obvious that you've
made a mistake that needs to be immediately fixed.
If you error out it's much more likely that people won't notice.
> Anyhow, even if today there was no possibility this could happen, that's
> something that might easily be changed by a future change. So I doubt
> "this will never happen" stays true for sure and being defensive is a
> good idea.
Let's revisit this again *if* this ever becomes a real issue. There's no
use in adding dead code to the kernel to handle hypothetical use-cases.
> And even a BUG would be better than a silent NULL pointer dereference.
I've never encountered a NULL pointer dereference that was silent. =)
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] I2c: busses - Fix possible NULL derefrence.
2017-01-30 8:54 ` Thierry Reding
@ 2017-01-30 11:15 ` Uwe Kleine-König
2017-01-30 11:24 ` Thierry Reding
0 siblings, 1 reply; 7+ messages in thread
From: Uwe Kleine-König @ 2017-01-30 11:15 UTC (permalink / raw)
To: Thierry Reding
Cc: Shailendra Verma, Laxman Dewangan, Wolfram Sang, Stephen Warren,
Alexandre Courbot, linux-i2c, linux-tegra, linux-kernel,
p.shailesh, ashish.kalra, Shailendra Verma
On Mon, Jan 30, 2017 at 09:54:55AM +0100, Thierry Reding wrote:
> On Mon, Jan 30, 2017 at 09:07:15AM +0100, Uwe Kleine-König wrote:
> > Hello,
> >
> > On Mon, Jan 30, 2017 at 08:12:17AM +0100, Thierry Reding wrote:
> > > On Mon, Jan 30, 2017 at 10:33:07AM +0530, Shailendra Verma wrote:
> > > > of_device_get_match_data could return NULL, and so can cause
> > > > a NULL pointer dereference later.
> > > >
> > > > Signed-off-by: Shailendra Verma <shailendra.v@samsung.com>
> > > > ---
> > > > drivers/i2c/busses/i2c-tegra.c | 4 ++++
> > > > 1 file changed, 4 insertions(+)
> > >
> > > This will never happen. Any match in the OF table that would cause the
> > > ->probe() to occur has a valid .data pointer associated with it.
> >
> > Theoretically you could (I think) bind that driver to a node with
> >
> > compatible = "tegra-i2c";
>
> That's not a valid compatible string and I don't think this could end up
> anywhere that would make the driver bind. Even if it did I think it'd be
Look at platform_match() in drivers/base/platform.c. If
of_driver_match_device fails it might still match based on
strcmp(pdev->name, drv->name).
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] I2c: busses - Fix possible NULL derefrence.
2017-01-30 11:15 ` Uwe Kleine-König
@ 2017-01-30 11:24 ` Thierry Reding
0 siblings, 0 replies; 7+ messages in thread
From: Thierry Reding @ 2017-01-30 11:24 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Shailendra Verma, Laxman Dewangan, Wolfram Sang, Stephen Warren,
Alexandre Courbot, linux-i2c, linux-tegra, linux-kernel,
p.shailesh, ashish.kalra, Shailendra Verma
[-- Attachment #1: Type: text/plain, Size: 1628 bytes --]
On Mon, Jan 30, 2017 at 12:15:53PM +0100, Uwe Kleine-König wrote:
> On Mon, Jan 30, 2017 at 09:54:55AM +0100, Thierry Reding wrote:
> > On Mon, Jan 30, 2017 at 09:07:15AM +0100, Uwe Kleine-König wrote:
> > > Hello,
> > >
> > > On Mon, Jan 30, 2017 at 08:12:17AM +0100, Thierry Reding wrote:
> > > > On Mon, Jan 30, 2017 at 10:33:07AM +0530, Shailendra Verma wrote:
> > > > > of_device_get_match_data could return NULL, and so can cause
> > > > > a NULL pointer dereference later.
> > > > >
> > > > > Signed-off-by: Shailendra Verma <shailendra.v@samsung.com>
> > > > > ---
> > > > > drivers/i2c/busses/i2c-tegra.c | 4 ++++
> > > > > 1 file changed, 4 insertions(+)
> > > >
> > > > This will never happen. Any match in the OF table that would cause the
> > > > ->probe() to occur has a valid .data pointer associated with it.
> > >
> > > Theoretically you could (I think) bind that driver to a node with
> > >
> > > compatible = "tegra-i2c";
> >
> > That's not a valid compatible string and I don't think this could end up
> > anywhere that would make the driver bind. Even if it did I think it'd be
>
> Look at platform_match() in drivers/base/platform.c. If
> of_driver_match_device fails it might still match based on
> strcmp(pdev->name, drv->name).
pdev->name is never influenced by the compatible string. The only way
you could create a device that would match this driver is if you were to
manually create it using of_platform_device_create() or similar,
something which we can easily prevent (or revert should anyone ever get
such code into the kernel again).
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] I2c: busses - Fix possible NULL derefrence.
2017-01-30 5:03 ` [PATCH] I2c: busses - Fix possible NULL derefrence Shailendra Verma
2017-01-30 7:12 ` Thierry Reding
@ 2017-02-09 16:37 ` Wolfram Sang
1 sibling, 0 replies; 7+ messages in thread
From: Wolfram Sang @ 2017-02-09 16:37 UTC (permalink / raw)
To: Shailendra Verma
Cc: Laxman Dewangan, Stephen Warren, Thierry Reding,
Alexandre Courbot, linux-i2c, linux-tegra, linux-kernel,
p.shailesh, ashish.kalra, Shailendra Verma
[-- Attachment #1: Type: text/plain, Size: 343 bytes --]
On Mon, Jan 30, 2017 at 10:33:07AM +0530, Shailendra Verma wrote:
> of_device_get_match_data could return NULL, and so can cause
> a NULL pointer dereference later.
>
> Signed-off-by: Shailendra Verma <shailendra.v@samsung.com>
I don't mind either way, but since Thierry is the maintainer of this
driver, I respect his preference.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-02-09 16:41 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20170130050324epcas3p28c9adcffd7bc40e1733cdeef1e5240d5@epcas3p2.samsung.com>
2017-01-30 5:03 ` [PATCH] I2c: busses - Fix possible NULL derefrence Shailendra Verma
2017-01-30 7:12 ` Thierry Reding
2017-01-30 8:07 ` Uwe Kleine-König
2017-01-30 8:54 ` Thierry Reding
2017-01-30 11:15 ` Uwe Kleine-König
2017-01-30 11:24 ` Thierry Reding
2017-02-09 16:37 ` Wolfram Sang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).