linux-kernel - Re: A workaround for request_firmware() stuck in module

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]

Message-ID: <s5htxvcieg0.wl%tiwai@suse.de>
Date:	Wed, 05 Sep 2012 19:09:51 +0200
From:	Takashi Iwai <tiwai@...e.de>
To:	Lucas De Marchi <lucas.de.marchi@...il.com>
Cc:	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Ming Lei <ming.lei@...onical.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>, Kay Sievers <kay@...y.org>,
	linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: A workaround for request_firmware() stuck in module_init

At Wed, 5 Sep 2012 13:59:56 -0300,
Lucas De Marchi wrote:
> 
> On Wed, Sep 5, 2012 at 10:03 AM, Alan Cox <alan@...rguk.ukuu.org.uk> wrote:
> >> If the driver is built in kernel, the request_firmware in .probe() may
> >> prolong kernel init, and it might be a problem. But looks it is not a
> >> big deal since most of drivers are built as module.
> >
> > Doing it by deferring the load also fixes that. The built in ones will
> > defer their final probe until the firmware appears and all will be well.
> >
> > If your rootfs needs firmware not in your initrd you already broke it and
> > there is a certain level beyond which you just have to give up trying to
> > save people from themselves.
> >
> > It may actually make sense to push more of it into the core driver layer
> > and take some of the ability to make mistakes away from driver authors.
> > For the general case of "load firmware if we see one" there isn't really
> > any reason we can't have a firmware_name entry in the probe table
> > entries themselves. If that was present the core bus probe would kick a
> > firmware load off and only when the firmware had loaded would it call
> > ->probe with dev->firmware pointing at a refcounted firmware struct.
> >
> > At that point it should be much faster to fix existing drivers and much
> > harder for a random device driver to get it wrong. We can even add
> > helpers which manage dev->firmware, and free the relevant objects when
> > needed, plus doing automatic ref/deref on probe/remove so that for a
> > typical driver the author only has to do
> >
> >         {PCI_blah , ... .firmware_name="wibble500.xcr", }
> >
> > and all the loading, unloading, not loading twice happens by "magic" for
> > the driver author.
> >
> > Add a dev_discard_firmware() for drivers that do this and know they can
> > then dump the file and all is good 8)
> 
> 
> It seems like a good plan. So drivers that call request_module()
> inside init_module() can be easily converted to this new scheme.
> 
> For those drivers that load the firmware upon open() syscal can be
> left as is, right?
> 
> Then we can write the rule in stone: *don't call request_firmware from
> init_module, instead give the name of the firmware*.

And we can even add a WARNING() if the call still happens, once when
the new implementation is done.


Takashi

> I even see
> drivers whose only purpose is to load the firmware and change the PID
> so it's handled by another module (like drivers/bluetooth/bcm203x.c)
> to be simplified by some extent.
> 
> 
> Lucas De Marchi
> 
--
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/