linux-kernel - Re: [PATCH 2/3] firmware: Add CONFIG_BUILTIN

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:	Sun, 25 May 2008 11:30:37 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	Marcel Holtmann <marcel@...tmann.org>
Cc:	David Woodhouse <dwmw2@...radead.org>,
	Sam Ravnborg <sam@...nborg.org>, linux-kernel@...r.kernel.org,
	aoliva@...hat.com, alan@...rguk.ukuu.org.uk,
	Abhay Salunke <Abhay_Salunke@...l.com>, kay.sievers@...y.org,
	Takashi Iwai <tiwai@...e.de>, Michael Buesch <mb@...sch.de>
Subject: Re: [PATCH 2/3] firmware: Add CONFIG_BUILTIN_FIRMWARE option

> so using "/" within the name parameter for request_firmware() is  
> actually forbidden. I know that some driver authers think it is a good  
> idea, but it is not.

Can you explain why it is allowed now? And maybe why the API was
designed in a way that easily allows it?

> I explained this a couple of times. The request_firmware() is an  
> abstract mechanism that can request a firmware file. The location of  
> the firmware file is up to the userspace. The kernel requests a  
> particular file and that is it. All namespacing has to be done by the  
> firmware helper script (nowadays udev). That the current  
> implementation of the firmware helper maps the filename 1:1 to a file  
> under /lib/firmware/ just works, but doesn't have to work all the  
> time. It is not the agreed contract between kernel and userspace.

I don't buy this argument. I could agree if you said that the "agreed
contract" between the kernel and userspace is for the kernel to request
a firmware file /keyed by an arbitrary, null-terminated string/.

The fact that it is usually stored on a filesystem where / means a
directory (and thus grouping) can be seen as a nice convenience of the
filesystem storage, but if firmware was stored elsewhere then you could
degrade to the simple key-based lookup that happens to allow "/" as a
character in the keys.

And because the kernel is nice, it allows userspace to use a filesystem
storage by not using paths like "../../lib/firmware/asdf". But
fundamentally, I don't even see anything wrong with that.

Put another way, you can have pretty arbitrary firmware firmware names
(though since humans need to handle them you want printable characters),
and I don't see why now all the sudden you would treat "/" specially by
*explicitly* disallowing it.

b43 comes with 22 firmware files for a single driver, and groups them
using "b43/<name>". What you're proposing will make firmware fail
*again* for all users, and we got a *LOT* of flak from all kinds of
stakeholders (not just the users) when firmware upgrades were required,
doing it again for such a petty reason is ridiculous.

johannes

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)