linux-kernel - Re: [PATCH RFC 0/4] Add firmware signature file check

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: <CACVXFVPJ0TWf=ffFhw+UGoGgsRnrD7o+Ws5rdDxS52jSf4SZ4A@mail.gmail.com>
Date:	Tue, 6 Nov 2012 16:04:50 +0800
From:	Ming Lei <tom.leiming@...il.com>
To:	Takashi Iwai <tiwai@...e.de>
Cc:	Matthew Garrett <mjg59@...f.ucam.org>,
	Alan Cox <alan@...rguk.ukuu.org.uk>, joeyli <jlee@...e.com>,
	Jiri Kosina <jkosina@...e.cz>,
	David Howells <dhowells@...hat.com>,
	Rusty Russell <rusty@...tcorp.com.au>,
	linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org, linux-efi@...r.kernel.org
Subject: Re: [PATCH RFC 0/4] Add firmware signature file check

On Tue, Nov 6, 2012 at 3:32 PM, Takashi Iwai <tiwai@...e.de> wrote:
> At Tue, 6 Nov 2012 15:16:43 +0800,
> Ming Lei wrote:
>>
>> On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai <tiwai@...e.de> wrote:
>> >
>> > Yeah, it's just uncovered in the patch.  As a easy solution, apply the
>> > patch like below to disallow the udev fw loading when signature check
>> > is enforced.
>> >
>> >
>> > thanks,
>> >
>> > Takashi
>> >
>> > ---
>> > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
>> > index 575bc4c..93121c3 100644
>> > --- a/drivers/base/firmware_class.c
>> > +++ b/drivers/base/firmware_class.c
>> > @@ -912,6 +912,13 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
>> >                 goto handle_fw;
>> >         }
>> >
>> > +       /* signature check isn't handled via udev fw loading */
>> > +       if (sig_enforce) {
>> > +               fw_load_abort(fw_priv);
>> > +               direct_load = 1;
>> > +               goto handle_fw;
>> > +       }
>> > +
>>
>> The above might be wrong if the firmware file doesn't exist in default
>> search paths.
>
> Heh, I didn't call it's a perfect patch.  It's just an easy solution,
> as mentioned.
>
>> You should skip loading from user space only if
>> verify_signature() returns false. And the udev loading should be
>> resorted to if there is no such firmware file in default search paths.
>
> ... and the kernel should ask udev again for the corresponding
> signature.

I mean you can't skip user space loading if there is no firmware file
in the default search path.  And you can do it if verify_signature()
returns false. So you needn't have to implement the signature for
user space loading.

> I'm too lazy to implement that just for unknown corner
> cases, so put the patch like above.

There might be some distributions in which the firmwares aren't stored
under the default search path, so your change may cause regression
on these distributions. And, it is a easy change in your patch to make
the situation working.

Also the default search path in firmware_class.c is from built-in path of
udev, and distributions may customize their firmware path by udev
configure option.

>
> Honestly speaking, I have a feeling that we should rather go for
> getting rid of udev fw loading.  The fw loader code is overly complex

Yes, I have the feeling too, but we need to make sure no regressions
introduced.

> just for udev handshaking.
>
> Do you know how many firmwares still rely on udev...?

Do you know how many distributions have switched to 3.7-rcX to
start using direct loading?

Thanks,
-- 
Ming Lei
--
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/