linux-kernel - Re: [PATCH 6/7] firmware_class: add "fd" input file

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:	Mon, 21 Jul 2014 09:42:00 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Kees Cook <keescook@...omium.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"Luis R. Rodriguez" <mcgrof@...e.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	James Morris <james.l.morris@...cle.com>,
	David Howells <dhowells@...hat.com>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	linux-security-module <linux-security-module@...r.kernel.org>,
	linux-firmware@...nel.org,
	linux-wireless <linux-wireless@...r.kernel.org>
Subject: Re: [PATCH 6/7] firmware_class: add "fd" input file

On Mon, Jul 21, 2014 at 1:43 AM, Kees Cook <keescook@...omium.org> wrote:
> On Sat, Jul 19, 2014 at 8:04 PM, Ming Lei <ming.lei@...onical.com> wrote:
>> On Tue, Jul 15, 2014 at 5:38 AM, Kees Cook <keescook@...omium.org> wrote:
>>> As an alternative to loading bytes from the "data" blob when reading
>>> firmware, let kernel read from an fd, so that the LSM can reason about
>>> the origin of firmware contents during userspace on-demand loading.
>>
>> From user space view, maybe it is better to keep previous usage and just
>> check if loading is from 'data' blob or fd in 'echo 0 > loading' of
>> firmware_loading_store(), then the 'fd' usage becomes very similar with
>> before.
>
> I don't think this is a good idea because otherwise there isn't a good
> way to have an "atomic" check of the firmware contents. What does it

Could you share why 'atomic' check is necessary? As we know, it isn't
real atomic, :-).

> means to write to "fd" several times, then write "data" a little,
> before writing "loading", etc? I originally wrote the patch series

That depends how firmware loader supports these cases, and won't
be difficult to handle them.

For non-fd userspace interface, it is very flexible to be capable of
supporting to load firmware data from multiple images, or in flight.
With single 'fd' interface, it won't be possible any more.

> requiring the "loading" piece, and it ended up being very complicated
> due to needing to switch the memory buffer logic back and forth.
> Everything is much much cleaner if "fd" is single-shot, and not part
> of the loading/data interface semantics.

You might not avoid the 'loading' piece completely, how does
the userspace handle non-exist firmware image? The reasonable
way is to abort the loading from userspace via 'echo -1 > loading'
since userspace already sees that, and you may choose to let kernel
side handle that, but your current patch doesn't support it yet.

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