linux-kernel - Re: [PATCH v2 0/2] x86/kexec: UKI Support

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: <63952cb0-5217-42a8-9b62-8be6d03f5844@app.fastmail.com>
Date:   Wed, 13 Sep 2023 16:42:33 +0200
From:   "Jan Hendrik Farr" <kernel@...rr.cc>
To:     "Philipp Rudo" <prudo@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kexec@...ts.infradead.org,
        x86@...nel.org, tglx@...utronix.de, dhowells@...hat.com,
        vgoyal@...hat.com, keyrings@...r.kernel.org,
        akpm@...ux-foundation.org, "Baoquan He" <bhe@...hat.com>,
        bhelgaas@...gle.com, "Luca Boccassi" <bluca@...ian.org>,
        lennart@...ttering.net
Subject: Re: [PATCH v2 0/2] x86/kexec: UKI Support

On Wed, Sep 13, 2023, at 4:00 PM, Philipp Rudo wrote:
> Hi Jan,
>
> All in all the code looks fine to me. Nevertheless I don't think UKI
> support should be added at the moment. This is because IMHO you
> basically reinterpret the kexec_file systemcall and thus add a new
> uapi to the kernel. Once introduced it is extremely hard to remove or
> change an uapi again. The problem I see is that the spec you based
> your work on is in such a poor shape that I don't feel comfortable to
> add any new uapi based on it.
>
> For example there are two definitions for the UKI which contradict
> each other. The dedicated one [1] you have cited earlier and the one
> in the BLS for type #2 entries [2]. In [1] the .linux and .initrd
> sections are mandatory and the .osrel and .cmdline sections are
> optional while in [2] it is the other way round. Which definition
> should the kernel follow?
>
> Furthermore, I absolutely don't understand how the spec should be
> read. All the spec does is defining some file formats. There is no
> word about which component in the boot chain is supposed to handle
> them and what exactly this component is supposed to do with it. But
> that is crucial if we want to add UKI support for kexec as the kexec
> systemcall will replace the stub. So we need to know what tasks the
> stub is supposed to perform. Currently this is only some
> implementation detail of the systemd-stub [3] that can change any
> moment and I strongly oppose to base any uapi on it.
>

I think I have to agree with you on this one.

I kinda experienced this first hand. My initial patch required a kernel
command line, because the spec in [1] didn't say it was optional. Then
I got a response saying that it's actually optional. And the spec got
changed in PR [2] (I have a WIP v3 that allows a manual cmdline if the
.cmdline section is empty as explained by Lennart in [2] and now
also in the spec since [3]). So there's already 2 changes in the spec
in the last few days related to my patch. That shows that the spec is
not stable.

What's even worse IMO is that the reason given in [2] to change the spec
is simply that systemd-stub has a different behavior than the spec and
therefore the spec should be updated. At this point it's not really a
specification but simply a flawed documentation of the behavior of systemd-
stub / ukify.

I also discovered that ukify treats the initrd as optional [4]. So is
this the next change in the spec?

In [5] Luca writes:
> [...] we fully intend for the UKI format to be an open and stable
> specification, that anybody can support and rely on.
But that is unfortunately not where the format is at this point.

What is annoying though is where this leaves a user that actually
wants this feature. They can carry a patch or they might have to wait
a long time.

Can you indicate what it would take for the kernel community to consider
this spec as stable enough?

> In the end the only benefit this series brings is to extend the
> signature checking on the whole UKI except of just the kernel image.
> Everything else can also be done in user space. Compared to the
> problems described above this is a very small gain for me.

Correct. That is the benefit of pulling the UKI apart in the
kernel. However having to sign the kernel inside the UKI defeats
the whole point.

[1] https://uapi-group.org/specifications/specs/unified_kernel_image/
[2] https://github.com/uapi-group/specifications/pull/72
[3] https://github.com/uapi-group/specifications/pull/73
[4] https://github.com/uapi-group/specifications/issues/74
[5] https://github.com/systemd/systemd/issues/28538