* [PATCH] Usb: host - Fix possible NULL derefrence. [not found] <CGME20170130050640epcas2p1f856bc12df5d1ee6b9e0c66cf9dd6339@epcas2p1.samsung.com> @ 2017-01-30 5:06 ` Shailendra Verma 0 siblings, 0 replies; 7+ messages in thread From: Shailendra Verma @ 2017-01-30 5:06 UTC (permalink / raw) To: Mathias Nyman, Greg Kroah-Hartman, Stephen Warren, Thierry Reding, Alexandre Courbot, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, p.shailesh-Sze3O3UU22JBDgjK7y7TUQ, ashish.kalra-Sze3O3UU22JBDgjK7y7TUQ, 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-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> --- drivers/usb/host/xhci-tegra.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c index a59fafb..890c778 100644 --- a/drivers/usb/host/xhci-tegra.c +++ b/drivers/usb/host/xhci-tegra.c @@ -903,6 +903,10 @@ static int tegra_xusb_probe(struct platform_device *pdev) return -ENOMEM; tegra->soc = of_device_get_match_data(&pdev->dev); + if (!tegra->soc) { + dev_err(&pdev->dev, "no device match found\n"); + return -ENODEV; + } mutex_init(&tegra->lock); tegra->dev = &pdev->dev; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] Usb: host - Fix possible NULL derefrence. @ 2017-01-30 5:06 ` Shailendra Verma 0 siblings, 0 replies; 7+ messages in thread From: Shailendra Verma @ 2017-01-30 5:06 UTC (permalink / raw) To: Mathias Nyman, Greg Kroah-Hartman, Stephen Warren, Thierry Reding, Alexandre Courbot, linux-usb, 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/usb/host/xhci-tegra.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c index a59fafb..890c778 100644 --- a/drivers/usb/host/xhci-tegra.c +++ b/drivers/usb/host/xhci-tegra.c @@ -903,6 +903,10 @@ static int tegra_xusb_probe(struct platform_device *pdev) return -ENOMEM; tegra->soc = of_device_get_match_data(&pdev->dev); + if (!tegra->soc) { + dev_err(&pdev->dev, "no device match found\n"); + return -ENODEV; + } mutex_init(&tegra->lock); tegra->dev = &pdev->dev; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
[parent not found: <1485752789-30374-1-git-send-email-shailendra.v-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>]
* Re: [PATCH] Usb: host - Fix possible NULL derefrence. 2017-01-30 5:06 ` Shailendra Verma @ 2017-01-30 6:45 ` Greg Kroah-Hartman -1 siblings, 0 replies; 7+ messages in thread From: Greg Kroah-Hartman @ 2017-01-30 6:45 UTC (permalink / raw) To: Shailendra Verma Cc: Mathias Nyman, Stephen Warren, Thierry Reding, Alexandre Courbot, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, p.shailesh-Sze3O3UU22JBDgjK7y7TUQ, ashish.kalra-Sze3O3UU22JBDgjK7y7TUQ, Shailendra Verma On Mon, Jan 30, 2017 at 10:36:29AM +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-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> > --- > drivers/usb/host/xhci-tegra.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > index a59fafb..890c778 100644 > --- a/drivers/usb/host/xhci-tegra.c > +++ b/drivers/usb/host/xhci-tegra.c > @@ -903,6 +903,10 @@ static int tegra_xusb_probe(struct platform_device *pdev) > return -ENOMEM; > > tegra->soc = of_device_get_match_data(&pdev->dev); > + if (!tegra->soc) { How would the driver be loaded and the probe function called if this returns NULL? Is this ever possible? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Usb: host - Fix possible NULL derefrence. @ 2017-01-30 6:45 ` Greg Kroah-Hartman 0 siblings, 0 replies; 7+ messages in thread From: Greg Kroah-Hartman @ 2017-01-30 6:45 UTC (permalink / raw) To: Shailendra Verma Cc: Mathias Nyman, Stephen Warren, Thierry Reding, Alexandre Courbot, linux-usb, linux-tegra, linux-kernel, p.shailesh, ashish.kalra, Shailendra Verma On Mon, Jan 30, 2017 at 10:36:29AM +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/usb/host/xhci-tegra.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > index a59fafb..890c778 100644 > --- a/drivers/usb/host/xhci-tegra.c > +++ b/drivers/usb/host/xhci-tegra.c > @@ -903,6 +903,10 @@ static int tegra_xusb_probe(struct platform_device *pdev) > return -ENOMEM; > > tegra->soc = of_device_get_match_data(&pdev->dev); > + if (!tegra->soc) { How would the driver be loaded and the probe function called if this returns NULL? Is this ever possible? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20170130064521.GC4324-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] Usb: host - Fix possible NULL derefrence. 2017-01-30 6:45 ` Greg Kroah-Hartman @ 2017-01-30 7:03 ` Thierry Reding -1 siblings, 0 replies; 7+ messages in thread From: Thierry Reding @ 2017-01-30 7:03 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Shailendra Verma, Mathias Nyman, Stephen Warren, Alexandre Courbot, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, p.shailesh-Sze3O3UU22JBDgjK7y7TUQ, ashish.kalra-Sze3O3UU22JBDgjK7y7TUQ, Shailendra Verma [-- Attachment #1: Type: text/plain, Size: 2058 bytes --] On Mon, Jan 30, 2017 at 07:45:21AM +0100, Greg Kroah-Hartman wrote: > On Mon, Jan 30, 2017 at 10:36:29AM +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-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> > > --- > > drivers/usb/host/xhci-tegra.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > > index a59fafb..890c778 100644 > > --- a/drivers/usb/host/xhci-tegra.c > > +++ b/drivers/usb/host/xhci-tegra.c > > @@ -903,6 +903,10 @@ static int tegra_xusb_probe(struct platform_device *pdev) > > return -ENOMEM; > > > > tegra->soc = of_device_get_match_data(&pdev->dev); > > + if (!tegra->soc) { > > How would the driver be loaded and the probe function called if this > returns NULL? > > Is this ever possible? No, it isn't. I've been NAK'ing this kind of patch for a while now. There are two variants of this patch going about: 1) checking the return value of of_match_device() 2) checking the return value of of_device_get_match_data() The same may also apply to of_match_node(), but I haven't seen that used very much lately. For of_match_device() the problem could technically occur if used in non OF setups, because the device could be instantiated by hand in board setup code. Tegra has been OF-only for a couple of years now, so there is no way this can happen today. of_device_get_match_data() is somewhat more complicated because it could still return NULL if the OF table entry had its .data field set to NULL. However in all drivers that I know that would be considered a bug, so might as well let things crash at this point to make it immediately obvious. I had once been tempted to write a checkpatch rule for this, but I'm not sure it's as easy as just warning if there's a check, because there are some legitimate cases, even if they're very rare. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Usb: host - Fix possible NULL derefrence. @ 2017-01-30 7:03 ` Thierry Reding 0 siblings, 0 replies; 7+ messages in thread From: Thierry Reding @ 2017-01-30 7:03 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Shailendra Verma, Mathias Nyman, Stephen Warren, Alexandre Courbot, linux-usb, linux-tegra, linux-kernel, p.shailesh, ashish.kalra, Shailendra Verma [-- Attachment #1: Type: text/plain, Size: 2030 bytes --] On Mon, Jan 30, 2017 at 07:45:21AM +0100, Greg Kroah-Hartman wrote: > On Mon, Jan 30, 2017 at 10:36:29AM +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/usb/host/xhci-tegra.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > > index a59fafb..890c778 100644 > > --- a/drivers/usb/host/xhci-tegra.c > > +++ b/drivers/usb/host/xhci-tegra.c > > @@ -903,6 +903,10 @@ static int tegra_xusb_probe(struct platform_device *pdev) > > return -ENOMEM; > > > > tegra->soc = of_device_get_match_data(&pdev->dev); > > + if (!tegra->soc) { > > How would the driver be loaded and the probe function called if this > returns NULL? > > Is this ever possible? No, it isn't. I've been NAK'ing this kind of patch for a while now. There are two variants of this patch going about: 1) checking the return value of of_match_device() 2) checking the return value of of_device_get_match_data() The same may also apply to of_match_node(), but I haven't seen that used very much lately. For of_match_device() the problem could technically occur if used in non OF setups, because the device could be instantiated by hand in board setup code. Tegra has been OF-only for a couple of years now, so there is no way this can happen today. of_device_get_match_data() is somewhat more complicated because it could still return NULL if the OF table entry had its .data field set to NULL. However in all drivers that I know that would be considered a bug, so might as well let things crash at this point to make it immediately obvious. I had once been tempted to write a checkpatch rule for this, but I'm not sure it's as easy as just warning if there's a check, because there are some legitimate cases, even if they're very rare. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Usb: host - Fix possible NULL derefrence. 2017-01-30 7:03 ` Thierry Reding (?) @ 2017-01-30 19:37 ` Greg Kroah-Hartman -1 siblings, 0 replies; 7+ messages in thread From: Greg Kroah-Hartman @ 2017-01-30 19:37 UTC (permalink / raw) To: Thierry Reding Cc: Shailendra Verma, Mathias Nyman, Stephen Warren, Alexandre Courbot, linux-usb, linux-tegra, linux-kernel, p.shailesh, ashish.kalra, Shailendra Verma On Mon, Jan 30, 2017 at 08:03:23AM +0100, Thierry Reding wrote: > On Mon, Jan 30, 2017 at 07:45:21AM +0100, Greg Kroah-Hartman wrote: > > On Mon, Jan 30, 2017 at 10:36:29AM +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/usb/host/xhci-tegra.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > > > index a59fafb..890c778 100644 > > > --- a/drivers/usb/host/xhci-tegra.c > > > +++ b/drivers/usb/host/xhci-tegra.c > > > @@ -903,6 +903,10 @@ static int tegra_xusb_probe(struct platform_device *pdev) > > > return -ENOMEM; > > > > > > tegra->soc = of_device_get_match_data(&pdev->dev); > > > + if (!tegra->soc) { > > > > How would the driver be loaded and the probe function called if this > > returns NULL? > > > > Is this ever possible? > > No, it isn't. I've been NAK'ing this kind of patch for a while now. > There are two variants of this patch going about: > > 1) checking the return value of of_match_device() > 2) checking the return value of of_device_get_match_data() > > The same may also apply to of_match_node(), but I haven't seen that used > very much lately. > > For of_match_device() the problem could technically occur if used in non > OF setups, because the device could be instantiated by hand in board > setup code. Tegra has been OF-only for a couple of years now, so there > is no way this can happen today. > > of_device_get_match_data() is somewhat more complicated because it could > still return NULL if the OF table entry had its .data field set to NULL. > However in all drivers that I know that would be considered a bug, so > might as well let things crash at this point to make it immediately > obvious. > > I had once been tempted to write a checkpatch rule for this, but I'm not > sure it's as easy as just warning if there's a check, because there are > some legitimate cases, even if they're very rare. Thanks for the info, patch is now dropped. Shailendra, please be more careful. greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-01-30 19:37 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20170130050640epcas2p1f856bc12df5d1ee6b9e0c66cf9dd6339@epcas2p1.samsung.com> 2017-01-30 5:06 ` [PATCH] Usb: host - Fix possible NULL derefrence Shailendra Verma 2017-01-30 5:06 ` Shailendra Verma [not found] ` <1485752789-30374-1-git-send-email-shailendra.v-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> 2017-01-30 6:45 ` Greg Kroah-Hartman 2017-01-30 6:45 ` Greg Kroah-Hartman [not found] ` <20170130064521.GC4324-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> 2017-01-30 7:03 ` Thierry Reding 2017-01-30 7:03 ` Thierry Reding 2017-01-30 19:37 ` Greg Kroah-Hartman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.