Skip to contentSkip to conversation

usb: dwc3: of-simple: fix use-after-free on remove

DISCUSSION Felipe Balbi 1 participant 1 message Jun 20, 2018 USB

lore
Share & tools
lore
#0 lore raw

Hi,

Quoted 24 lines · Tony Lindgren
Quoted 14 lines · Felipe Balbi

This is a direct consequence of not paying attention to the order of things. If driver were to assume that pm_domain->activate() would do the right thing for the device -- meaning that probe would run with an active device --, then we wouldn't need that pm_runtime_get() call on probe at all. Rather we would follow the sequence:

	pm_runtime_forbid()
	pm_runtime_set_active()
	pm_runtime_enable()

	/* do your probe routine */

	pm_runtime_put_noidle()
Quoted 2 lines

Then you remove you would need to call pm_runtime_get_noresume() to balance out the pm_runtime_put_noidle() there.

How about let's create some prettier interface for the above runtime PM trickery?

How about something like pm_runtime_init_enabled() for the above sequence?

Quoted 6 lines

It might be then able to do the trick even if activate is not implemented..

Right now it has the feeling of "oh well we can't get runtime PM to work so let's bypass it with activate call and then trick runtime PM to start in enabled mode" :)

no strong feelings about that either way. It's only 3 lines of code anyway. I feel like that's a minor cosmetic change.

Quoted 8 lines
Quoted 4 lines

(If you need to know why the pm_runtime_put_noidle(), remember that pm_runtime_set_active() increments the usage counter, so pm_runtime_put_noidle is basically allowing pm_runtime to happen as soon as userspace writes "auto" to /sys/..../power/control)

I wonder if we could also remove the need for drivers to call pm_runtime_putnoidle() at the end of the probe? If we had pm_runtime_init_enabled() implemented.

probably not. We want to block runtime pm during probe, until the device is fully initialized, the only way to do that is to increment rpm usage counter.