linux-kernel - Re: [PATCH v5 07/12] x86/cpu/keylocker: Load an internal wrapping key at boot-time

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:   Wed, 24 Aug 2022 15:52:32 -0700
From:   Evan Green <evgreen@...omium.org>
To:     "Chang S. Bae" <chang.seok.bae@...el.com>
Cc:     linux-crypto@...r.kernel.org, dm-devel@...hat.com,
        herbert@...dor.apana.org.au, Eric Biggers <ebiggers@...nel.org>,
        Ard Biesheuvel <ardb@...nel.org>, x86@...nel.org,
        luto@...nel.org, Thomas Gleixner <tglx@...utronix.de>, bp@...e.de,
        dave.hansen@...ux.intel.com, mingo@...nel.org,
        LKML <linux-kernel@...r.kernel.org>,
        Dan Williams <dan.j.williams@...el.com>,
        charishma1.gairuboyina@...el.com, kumar.n.dwarakanath@...el.com,
        ravi.v.shankar@...el.com
Subject: Re: [PATCH v5 07/12] x86/cpu/keylocker: Load an internal wrapping key
 at boot-time

On Wed, Aug 24, 2022 at 3:21 PM Chang S. Bae <chang.seok.bae@...el.com> wrote:
>
> On 8/23/2022 8:49 AM, Evan Green wrote:
> > On Wed, Jan 12, 2022 at 1:21 PM Chang S. Bae <chang.seok.bae@...el.com> wrote:
> >>
> <snip>
> >> +
> >> +static void __init load_keylocker(void)
> >
> > I am late to this party by 6 months, but:
> > load_keylocker() cannot be __init, as it gets called during SMP core onlining.
>
> Yeah, it looks like the case with this patch only.
>
> But the next patch [1] limits the call during boot time only:
>
>         if (c == &boot_cpu_data) {
>                 ...
>                 load_keylocker();
>                 ...
>         } else {
>                 ...
>                 if (!kl_setup.initialized) {
>                         load_keylocker();
>                 } else if (valid_kl) {
>                         rc = copy_keylocker();
>                         ...
>                 }
>         }
>
> kl_setup.initialized is set by native_smp_cpus_done() ->
> destroy_keylocker_data() when CPUs are booted. Then load_keylocker() is
> not called because the root key (aka IWKey) is no longer available in
> memory.
>
> Now this 'valid_kl' flag should be always on unless the root key backup
> is corrupted. Then copy_keylocker() loads the root key from the backup
> in the platform state.
>
> So I think the onlining CPU won't call it.
>
> Maybe this bit can be much clarified in a separate (new) patch, instead
> of being part of another like [1].

Whatever we ended up landing in the ChromeOS tree (which I think was
v4 of this series) actively hit this bug in hibernation, which is how
I found it. I couldn't get a full backtrace because the backtracing
code tripped over itself as well for some reason. If the next patch in
this series is different from what we landed in ChromeOS, then maybe
your description is correct, but I haven't dug in to understand the
delta.

-Evan