USB: ehci-platform: fix companion-device leak

PATCH In review Johan Hovold 2 participants 4 messages May 16, 2017 – May 18, 2017

Subsystems (3)

Stable branch USB USB ehci driver

lore

Share & tools

lore

Johan Hovold

author

2017-05-16 14:26 UTC #0 lore raw

PATCH 1/2 USB: ehci-platform: fix companion-device leak

Make sure do drop the reference taken to the companion device during resume.

Fixes d4d75128b8fd ("usb: host: ehci-platform: fix usb 1.1 device is not connected in system resume")

Cc stable <[email protected]> # 4.11

Cc Yoshihiro Shimoda <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/host/ehci-platform.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/usb/host/ehci-platform.c +3 -1

index bc7b9be12f54..f1908ea9fbd8 100644
@@ -384,8 +384,10 @@ static int ehci_platform_resume(struct device *dev)
 	}
 
 	companion_dev = usb_of_get_companion_dev(hcd->self.controller);
-	if (companion_dev)
+	if (companion_dev) {
 		device_pm_wait_for_dev(hcd->self.controller, companion_dev);
+		put_device(companion_dev);
+	}
 
 	ehci_resume(hcd, priv->reset_on_resume);
 	return 0;

Signature · 1 line

2.13.0

I should have added an API like "usb_of_put_companion_dev(dev)" and it calls "of_dev_put(to_platform_device(dev))". And then, ehci_platform_resume() can call the API instead of put_device(). But, adding new API is not good for stable.

So,

Acked-by Yoshihiro Shimoda <[email protected]>

Best regards,
Yoshihiro Shimoda

Quoted 4 lines

drivers/usb/host/ehci-platform.c +0 -1
 	ehci_resume(hcd, priv->reset_on_resume);
 	return 0;
--
2.13.0

Johan Hovold

author

2017-05-17 08:59 UTC #2 lore raw

Quoted 31 lines · Yoshihiro Shimoda

Hi Johan,
Quoted 19 lines
From: Johan Hovold
Sent: Tuesday, May 16, 2017 11:26 PM
Make sure do drop the reference taken to the companion device during resume.
Fixes d4d75128b8fd ("usb: host: ehci-platform: fix usb 1.1 device is not connected in system resume")
Cc stable <[email protected]> # 4.11
Cc Yoshihiro Shimoda <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/host/ehci-platform.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/usb/host/ehci-platform.c +0 -0
index bc7b9be12f54..f1908ea9fbd8 100644
drivers/usb/host/ehci-platform.c static int ehci_platform_resume(struct device *dev) · line 389
Quoted 7 lines
drivers/usb/host/ehci-platform.c +2 -1
@@ -384,8 +384,10 @@ static int ehci_platform_resume(struct device *dev)
 	}
 
 	companion_dev = usb_of_get_companion_dev(hcd->self.controller);
-	if (companion_dev)
+	if (companion_dev) {
 		device_pm_wait_for_dev(hcd->self.controller, companion_dev);
+		put_device(companion_dev);
Thank you for the patch!

Quoted 4 lines

I should have added an API like "usb_of_put_companion_dev(dev)" and it calls "of_dev_put(to_platform_device(dev))". And then, ehci_platform_resume() can call the API instead of put_device(). But, adding new API is not good for stable.

I don't think that's needed or desirable. The helper returns a pointer to a struct device with an incremented refcount, so just using put_device() when done is the right thing to do.

Note that I added a comment about the refcount to the helper in patch 2/2 as a reminder for any future users.

Quoted 3 lines

So,
Acked-by Yoshihiro Shimoda <[email protected]>

Thanks,
Johan

Yoshihiro Shimoda

2017-05-18 01:50 UTC #3 lore raw

Hi Johan,

Quoted 41 lines

-----Original Message-----
From: Johan Hovold
Sent: Wednesday, May 17, 2017 6:00 PM
Quoted 31 lines · Yoshihiro Shimoda
Hi Johan,
Quoted 19 lines
From: Johan Hovold
Sent: Tuesday, May 16, 2017 11:26 PM
Make sure do drop the reference taken to the companion device during resume.
Fixes d4d75128b8fd ("usb: host: ehci-platform: fix usb 1.1 device is not connected in system resume")
Cc stable <[email protected]> # 4.11
Cc Yoshihiro Shimoda <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/host/ehci-platform.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/usb/host/ehci-platform.c +0 -0
index bc7b9be12f54..f1908ea9fbd8 100644
drivers/usb/host/ehci-platform.c static int ehci_platform_resume(struct device *dev) · line 389
Quoted 7 lines
drivers/usb/host/ehci-platform.c +2 -1
@@ -384,8 +384,10 @@ static int ehci_platform_resume(struct device *dev)
 	}
 
 	companion_dev = usb_of_get_companion_dev(hcd->self.controller);
-	if (companion_dev)
+	if (companion_dev) {
 		device_pm_wait_for_dev(hcd->self.controller, companion_dev);
+		put_device(companion_dev);
Thank you for the patch!
Quoted 4 lines
I should have added an API like "usb_of_put_companion_dev(dev)" and it calls "of_dev_put(to_platform_device(dev))". And then, ehci_platform_resume() can call the API instead of put_device(). But, adding new API is not good for stable.

Quoted 6 lines

I don't think that's needed or desirable. The helper returns a pointer to a struct device with an incremented refcount, so just using put_device() when done is the right thing to do.
Note that I added a comment about the refcount to the helper in patch 2/2 as a reminder for any future users.

Thank you for the reply!
I understood it and I agreed with you!

Best regards,
Yoshihiro Shimoda