linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: Felipe Balbi <balbi@kernel.org>
Cc: Roger Quadros <rogerq@ti.com>, Johan Hovold <johan@kernel.org>,
	Tero Kristo <t-kristo@ti.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alan Stern <stern@rowland.harvard.edu>,
	linux-usb@vger.kernel.org, "Nori, Sekhar" <nsekhar@ti.com>,
	Grygorii Strashko <grygorii.strashko@ti.com>,
	Nishanth Menon <nm@ti.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	linux-pm@vger.kernel.org
Subject: usb: dwc3: of-simple: fix use-after-free on remove
Date: Wed, 20 Jun 2018 02:16:59 -0700	[thread overview]
Message-ID: <20180620091659.GJ26255@atomide.com> (raw)

Hi,

Adding Rafael and linux-pm to Cc as well.

* Felipe Balbi <balbi@kernel.org> [180619 01:23]:
> 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()
> 
> 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?

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" :)

> Anyway, with an assumption like this, after all platform_devices are
> converted over, the assumption can be moved into the bus code and, low
> and behold, to enable runtime pm for your driver, all you have to is
> implement your callbacks and add pm_runtime_put_noidle() to probe and
> pm_runtime_get_noresume() to remove (apart from, of course, making sure
> the device isn't allowed to runtime_suspend when it's actually busy).
> 
> Do you see the end goal?

It certainly would be nice to make runtime PM generic for drivers :)

> (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.

Regards,

Tony
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

             reply	other threads:[~2018-06-20  9:16 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-20  9:16 Tony Lindgren [this message]
  -- strict thread matches above, loose matches on Subject: below --
2018-06-21 15:15 usb: dwc3: of-simple: fix use-after-free on remove Alan Stern
2018-06-21 10:11 Johan Hovold
2018-06-21  9:52 Johan Hovold
2018-06-21  8:27 Johan Hovold
2018-06-21  8:17 Roger Quadros
2018-06-20 22:55 Rafael J. Wysocki
2018-06-20 22:32 Rafael J. Wysocki
2018-06-20 15:46 Johan Hovold
2018-06-20 12:54 Rafael J. Wysocki
2018-06-20 12:23 Johan Hovold
2018-06-20 12:17 Rafael J. Wysocki
2018-06-20 11:05 Felipe Balbi
2018-06-20  9:54 Rafael J. Wysocki
2018-06-20  9:27 Felipe Balbi
2018-06-20  4:34 Tony Lindgren
2018-06-19 12:10 Tero Kristo
2018-06-19  8:18 Felipe Balbi
2018-06-18 14:32 Roger Quadros
2018-06-18 12:21 Felipe Balbi
2018-06-18 11:11 Roger Quadros
2018-06-18  9:51 Felipe Balbi
2018-06-18  9:47 Johan Hovold
2018-06-18  9:33 Felipe Balbi
2018-06-18  8:34 Johan Hovold
2018-06-18  8:15 Felipe Balbi
2018-06-13 10:59 Johan Hovold
2018-06-13  9:39 Felipe Balbi
2018-06-13  8:34 Roger Quadros
2018-06-13  8:05 Felipe Balbi
2018-06-13  7:49 Roger Quadros
2018-05-31 14:58 Johan Hovold
2018-05-31 14:45 Johan Hovold

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180620091659.GJ26255@atomide.com \
    --to=tony@atomide.com \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=grygorii.strashko@ti.com \
    --cc=johan@kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=nsekhar@ti.com \
    --cc=rjw@rjwysocki.net \
    --cc=rogerq@ti.com \
    --cc=stern@rowland.harvard.edu \
    --cc=t-kristo@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).