* [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).