Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754765Ab2KFISO (ORCPT ); Tue, 6 Nov 2012 03:18:14 -0500 Received: from cantor2.suse.de ([195.135.220.15]:34076 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754336Ab2KFISM (ORCPT ); Tue, 6 Nov 2012 03:18:12 -0500 Date: Tue, 06 Nov 2012 09:18:10 +0100 Message-ID: From: Takashi Iwai To: Ming Lei Cc: Matthew Garrett , Alan Cox , joeyli , Jiri Kosina , David Howells , Rusty Russell , linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, linux-efi@vger.kernel.org Subject: Re: [PATCH RFC 0/4] Add firmware signature file check In-Reply-To: References: <1348152065-31353-1-git-send-email-mjg@redhat.com> <20121029174131.GC7580@srcf.ucam.org> <20121031173728.GA18615@srcf.ucam.org> <1351743715.21227.95.camel@linux-s257.site> <20121101131849.752df6fd@pyramind.ukuu.org.uk> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.2 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3849 Lines: 104 At Tue, 6 Nov 2012 16:04:50 +0800, Ming Lei wrote: > > On Tue, Nov 6, 2012 at 3:32 PM, Takashi Iwai wrote: > > At Tue, 6 Nov 2012 15:16:43 +0800, > > Ming Lei wrote: > >> > >> On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai 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. Right, and it's intentionally dropped so. For the non-default fw path, it can be added via proc dynamically or via kconfig statically. If the firmware is generated via udev, then it doesn't make sense to check a static signature file. > > 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. A "regression" can't happen in this case because the secure boot is a completely new stuff :) For normal boot, sig_enforce is false, so no behavior change here (well my patch still applies the signature check for direct fw loading, but it won't regress at least). > 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. Well, the default paths in kernel can be changed to follow that as well, no? > > 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. Right. And I guess the exceptional firmware case is better found by checking udev. But it's a bit off topic from secure boot. > > 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? Obviously no distro releases using 3.7-rc since it's still rc. But what's your point? Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/