linux-kernel - Re: [RFC] firmware load: defer request_firmware during early boot and resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 21 Jul 2012 19:35:43 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Ming Lei <tom.leiming@...il.com>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"linux-usb" <linux-usb@...r.kernel.org>,
	Alan Stern <stern@...land.harvard.edu>,
	"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
	Oliver Neukum <oneukum@...e.de>, Dave Jones <davej@...hat.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Matthew Garrett <mjg@...hat.com>,
	Jack Stone <jwjstone@...tmail.fm>,
	Larry Finger <Larry.Finger@...inger.net>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Linux PM list <linux-pm@...r.kernel.org>
Subject: Re: [RFC] firmware load: defer request_firmware during early boot and resume

On Saturday, July 21, 2012, Ming Lei wrote:
> On Sat, Jul 21, 2012 at 5:56 PM, Rafael J. Wysocki <rjw@...k.pl> wrote:
> > On Saturday, July 21, 2012, Ming Lei wrote:
> >> CC guys who discussed the problem in the below link in Jan. :
> >>
> >>       http://marc.info/?t=132528956000002&r=10&w=2
> >>
> >> On Fri, Jul 20, 2012 at 8:33 PM, Ming Lei <tom.leiming@...il.com> wrote:
> >> > The RFC patch is just for discussing if the idea of deferring
> >> > request_firmware is doable for addressing the issue of
> >> > request_firmware in resume path, which is caused by driver
> >> > unbind/rebind during resume.
> >> >
> >> > At least usb bus is involved in such things, one driver may be
> >> > unbound and rebound in resume path at several situations, and
> >> > request_firmware is often called inside probe().
> >> >
> >> > Also the idea should be helpful for other hotplug buses too,
> >> > at least there was the similar problem report on pcmcia bus.
> >>
> >> Looks it works well in my two test cases: one is to call request_firmware
> >> in early boot(initcall), another one is to call request_firmware in probe()
> >> of resume path(caused by unbind & rebind).  And without the patch, both
> >> two request_firmware return failure and can't complete the loading.
> >
> > And that's precisely why you're not supposed to use request_firmware() in
> > those two situations.
> 
> So you mean we should do as below?
> 
> For the early boot situation, the driver which calls request_firmware in probe()
> can't be built in kernel image.
> 
> For the second situation, some drivers can't be allowed to call
> request_firmware()
> in its probe() because these drivers may be unbound and rebound inside resume()
> or complete_resume(),  for example, see usb_resume_complete().
> 
> IMO, the 1st one is very unfriendly and the 2nd one can't be avoided for some
> hotplug buses.

I'm not sure if it really can't be avoided.

> That is just the motivation of this patch to make request_firmware() workable in
> both the two above situations.
> 
> >
> > My opinion is that, at least for the suspend/hibernate case, the firmware
> > data should be loaded into memory before suspend (e.g. using a PM notifier)
> > and released only after the corresponding resume (or suspend failure), so
> > that it's present in memory during the entire suspend-resume cycle.
> 
> The patch isn't to replace caching firmware data during suspend-resume cycle,
> and just a supplement for it.
> 
> It is not easy to cache firmware data during suspend-resume cycle for
> the above 2nd situation because of the lifetime problem of firmware data:
> the driver may be unbound and rebound inside resume path, even the
> device may vanish and appear again.
> 
> Also, Matthew had a below case[1] which can't be solved with caching
> firmware data at all, not to mention consuming much memory by caching
> firmware:
> 
>        1) user boots from cold. Device comes up with generic USB ID.
>        2) isight_firmware loads and binds. Firmware is loaded. Device
>            disconnects and reconnects with an ID that's bound by the UVC
>            driver.
>        3) user reboots. Device comes up with UVC ID. isight_firmware
>            doesn't bind.
>        4) user suspends.
>        5) user resumes. isight_firmware binds and attempts to load firmware.
> 
> But it can be dealt with easily by the simple patch.
> 
> Finally, suppose caching firmware may work well for the 2nd situation, we still
> have to cache all the firmwares of all hotplug devices(in one system) which
> need firmware before suspending, because these devices may be unplugged
> and plugged again during suspend-resume cycle or be powered off by system.

OK, I give up.  This may not be too ugly to live, after all.

I'll post some comments in a reply to the message with the patch.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/