| Message ID | 20190220134841.7642-1-thierry.reding@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xhci: tegra: Prevent error pointer dereference | expand |
On 20/02/2019 13:48, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > During initialization, the host and super-speed power domains will > contain an ERR_PTR() encoded error code rather than being NULL. To > avoid a crash, use a !IS_ERR_OR_NULL() condition during cleanup. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/usb/host/xhci-tegra.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > index 938ff06c0349..efb0cad8710e 100644 > --- a/drivers/usb/host/xhci-tegra.c > +++ b/drivers/usb/host/xhci-tegra.c > @@ -941,9 +941,9 @@ static void tegra_xusb_powerdomain_remove(struct device *dev, > device_link_del(tegra->genpd_dl_ss); > if (tegra->genpd_dl_host) > device_link_del(tegra->genpd_dl_host); > - if (tegra->genpd_dev_ss) > + if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss)) > dev_pm_domain_detach(tegra->genpd_dev_ss, true); > - if (tegra->genpd_dev_host) > + if (!IS_ERR_OR_NULL(tegra->genpd_dev_host)) > dev_pm_domain_detach(tegra->genpd_dev_host, true); > } Reviewed-by: Jon Hunter <jonathanh@nvidia.com> Thanks for fixing! Jon
On Wed, Feb 20, 2019 at 02:48:41PM +0100, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > During initialization, the host and super-speed power domains will > contain an ERR_PTR() encoded error code rather than being NULL. To > avoid a crash, use a !IS_ERR_OR_NULL() condition during cleanup. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/usb/host/xhci-tegra.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > index 938ff06c0349..efb0cad8710e 100644 > --- a/drivers/usb/host/xhci-tegra.c > +++ b/drivers/usb/host/xhci-tegra.c > @@ -941,9 +941,9 @@ static void tegra_xusb_powerdomain_remove(struct device *dev, > device_link_del(tegra->genpd_dl_ss); > if (tegra->genpd_dl_host) > device_link_del(tegra->genpd_dl_host); > - if (tegra->genpd_dev_ss) > + if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss)) > dev_pm_domain_detach(tegra->genpd_dev_ss, true); > - if (tegra->genpd_dev_host) > + if (!IS_ERR_OR_NULL(tegra->genpd_dev_host)) > dev_pm_domain_detach(tegra->genpd_dev_host, true); > } Should this go to older kernels? If so, any hint as to what commit this "fixes:"? thanks, greg k-h
On Wed, Feb 20, 2019 at 05:16:12PM +0100, Greg Kroah-Hartman wrote: > On Wed, Feb 20, 2019 at 02:48:41PM +0100, Thierry Reding wrote: > > From: Thierry Reding <treding@nvidia.com> > > > > During initialization, the host and super-speed power domains will > > contain an ERR_PTR() encoded error code rather than being NULL. To > > avoid a crash, use a !IS_ERR_OR_NULL() condition during cleanup. > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > drivers/usb/host/xhci-tegra.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > > index 938ff06c0349..efb0cad8710e 100644 > > --- a/drivers/usb/host/xhci-tegra.c > > +++ b/drivers/usb/host/xhci-tegra.c > > @@ -941,9 +941,9 @@ static void tegra_xusb_powerdomain_remove(struct device *dev, > > device_link_del(tegra->genpd_dl_ss); > > if (tegra->genpd_dl_host) > > device_link_del(tegra->genpd_dl_host); > > - if (tegra->genpd_dev_ss) > > + if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss)) > > dev_pm_domain_detach(tegra->genpd_dev_ss, true); > > - if (tegra->genpd_dev_host) > > + if (!IS_ERR_OR_NULL(tegra->genpd_dev_host)) > > dev_pm_domain_detach(tegra->genpd_dev_host, true); > > } > > Should this go to older kernels? If so, any hint as to what commit this > "fixes:"? Technically this: Fixes: 6494a9ad86de ("usb: xhci: tegra: Add genpd support") which was merged into v4.20. However, I have never seen this actually crash on current kernels and only came across it while working on a change to reset controls which can cause dev_pm_domain_attach_by_name() to fail. That function can't fail in current kernels, so I don't think we need to fix this in old kernels. So if we can get this into v5.0, that'd be good enough for me. Thierry
On Wed, Feb 20, 2019 at 05:27:02PM +0100, Thierry Reding wrote: > On Wed, Feb 20, 2019 at 05:16:12PM +0100, Greg Kroah-Hartman wrote: > > On Wed, Feb 20, 2019 at 02:48:41PM +0100, Thierry Reding wrote: > > > From: Thierry Reding <treding@nvidia.com> > > > > > > During initialization, the host and super-speed power domains will > > > contain an ERR_PTR() encoded error code rather than being NULL. To > > > avoid a crash, use a !IS_ERR_OR_NULL() condition during cleanup. > > > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > > --- > > > drivers/usb/host/xhci-tegra.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > > > index 938ff06c0349..efb0cad8710e 100644 > > > --- a/drivers/usb/host/xhci-tegra.c > > > +++ b/drivers/usb/host/xhci-tegra.c > > > @@ -941,9 +941,9 @@ static void tegra_xusb_powerdomain_remove(struct device *dev, > > > device_link_del(tegra->genpd_dl_ss); > > > if (tegra->genpd_dl_host) > > > device_link_del(tegra->genpd_dl_host); > > > - if (tegra->genpd_dev_ss) > > > + if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss)) > > > dev_pm_domain_detach(tegra->genpd_dev_ss, true); > > > - if (tegra->genpd_dev_host) > > > + if (!IS_ERR_OR_NULL(tegra->genpd_dev_host)) > > > dev_pm_domain_detach(tegra->genpd_dev_host, true); > > > } > > > > Should this go to older kernels? If so, any hint as to what commit this > > "fixes:"? > > Technically this: > > Fixes: 6494a9ad86de ("usb: xhci: tegra: Add genpd support") > > which was merged into v4.20. > > However, I have never seen this actually crash on current kernels and > only came across it while working on a change to reset controls which > can cause dev_pm_domain_attach_by_name() to fail. That function can't > fail in current kernels, so I don't think we need to fix this in old > kernels. So if we can get this into v5.0, that'd be good enough for > me. 5.0-final is too late, but 5.0.y is doable. Mathias, want me to take this directly? thanks, greg k-h
On 20.2.2019 18.31, Greg Kroah-Hartman wrote: > On Wed, Feb 20, 2019 at 05:27:02PM +0100, Thierry Reding wrote: >> On Wed, Feb 20, 2019 at 05:16:12PM +0100, Greg Kroah-Hartman wrote: >>> On Wed, Feb 20, 2019 at 02:48:41PM +0100, Thierry Reding wrote: >>>> From: Thierry Reding <treding@nvidia.com> >>>> >>>> During initialization, the host and super-speed power domains will >>>> contain an ERR_PTR() encoded error code rather than being NULL. To >>>> avoid a crash, use a !IS_ERR_OR_NULL() condition during cleanup. >>>> >>>> Signed-off-by: Thierry Reding <treding@nvidia.com> >>>> --- >>>> drivers/usb/host/xhci-tegra.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c >>>> index 938ff06c0349..efb0cad8710e 100644 >>>> --- a/drivers/usb/host/xhci-tegra.c >>>> +++ b/drivers/usb/host/xhci-tegra.c >>>> @@ -941,9 +941,9 @@ static void tegra_xusb_powerdomain_remove(struct device *dev, >>>> device_link_del(tegra->genpd_dl_ss); >>>> if (tegra->genpd_dl_host) >>>> device_link_del(tegra->genpd_dl_host); >>>> - if (tegra->genpd_dev_ss) >>>> + if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss)) >>>> dev_pm_domain_detach(tegra->genpd_dev_ss, true); >>>> - if (tegra->genpd_dev_host) >>>> + if (!IS_ERR_OR_NULL(tegra->genpd_dev_host)) >>>> dev_pm_domain_detach(tegra->genpd_dev_host, true); >>>> } >>> >>> Should this go to older kernels? If so, any hint as to what commit this >>> "fixes:"? >> >> Technically this: >> >> Fixes: 6494a9ad86de ("usb: xhci: tegra: Add genpd support") >> >> which was merged into v4.20. >> >> However, I have never seen this actually crash on current kernels and >> only came across it while working on a change to reset controls which >> can cause dev_pm_domain_attach_by_name() to fail. That function can't >> fail in current kernels, so I don't think we need to fix this in old >> kernels. So if we can get this into v5.0, that'd be good enough for >> me. > > 5.0-final is too late, but 5.0.y is doable. > > Mathias, want me to take this directly? > Yes please Acked-by: Mathias Nyman <mathias.nyman@linux.intel.com>
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c index 938ff06c0349..efb0cad8710e 100644 --- a/drivers/usb/host/xhci-tegra.c +++ b/drivers/usb/host/xhci-tegra.c @@ -941,9 +941,9 @@ static void tegra_xusb_powerdomain_remove(struct device *dev, device_link_del(tegra->genpd_dl_ss); if (tegra->genpd_dl_host) device_link_del(tegra->genpd_dl_host); - if (tegra->genpd_dev_ss) + if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss)) dev_pm_domain_detach(tegra->genpd_dev_ss, true); - if (tegra->genpd_dev_host) + if (!IS_ERR_OR_NULL(tegra->genpd_dev_host)) dev_pm_domain_detach(tegra->genpd_dev_host, true); }