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 13:54:32 +0200
From:	Marcel Holtmann <marcel@...tmann.org>
To:	Michael Buesch <mb@...sch.de>
Cc:	Johannes Berg <johannes@...solutions.net>,
	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>
Subject: Re: [PATCH 2/3] firmware: Add CONFIG_BUILTIN_FIRMWARE option

Hi Michael,

>>> 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/.
>
> I fully agree with johannes, that using / as a grouping-separator is a
> _nice_ idea and userspace must support it, regardless of the actual
> storage system it is using.
>
> Currently using a single directory for one driver really enhances  
> the way
> firmware can be handled. For example, we can have several different  
> versions
> cleanly installed in /lib/firmware. For example, on my development  
> machine
> I have
> /lib/firmware/b43
> /lib/firmware/b43-old
> /lib/firmware/b43-open
> /lib/firmware/b43legacy
> Putting all these files into plain /lib/firmware would require  
> changing the
> actual filenames and that would be a real pain. (This can be up to  
> about 100 files).

I am okay with userspace supporting namespacing with subdirectories  
and see the point why it helps, but the important here is that  
userspace must support this. It should not be done inside the kernel.

So we have to change udev to look for /lib/firmware/<driver>/ 
<filename> which perfectly fine, but the <driver> part needs to be  
derived from struct device and not from the <filename> part.

>> 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.
>
> Yeah. I'm not going to change that. The users are going to kill us.
> Besides that, there were very good reasons to start grouping the files
> in b43 (bcm43xx didn't do it), as it simply was unmaintainable in  
> bcm43xx.

Again, I agree that we wanna have that, but putting a directory prefix  
into the driver is wrong.

Regards

Marcel

--
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/