linux-kernel - Re: [PATCH v6 24/26] powerpc/pseries: Implement secvars for dynamic secure boot

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: <87zg9f5exp.fsf@mpe.ellerman.id.au>
Date:   Wed, 15 Feb 2023 22:46:42 +1100
From:   Michael Ellerman <michaele@....ibm.com>
To:     Andrew Donnellan <ajd@...ux.ibm.com>,
        Stefan Berger <stefanb@...ux.ibm.com>,
        linuxppc-dev@...ts.ozlabs.org, linux-integrity@...r.kernel.org
Cc:     ruscur@...sell.cc, bgray@...ux.ibm.com, nayna@...ux.ibm.com,
        gcwilson@...ux.ibm.com, gjoyce@...ux.ibm.com, brking@...ux.ibm.com,
        sudhakar@...ux.ibm.com, erichte@...ux.ibm.com,
        gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
        zohar@...ux.ibm.com, joel@....id.au, npiggin@...il.com
Subject: Re: [PATCH v6 24/26] powerpc/pseries: Implement secvars for dynamic
 secure boot

Andrew Donnellan <ajd@...ux.ibm.com> writes:
> On Mon, 2023-02-13 at 22:32 +1100, Michael Ellerman wrote:
>> > > +       memcpy(&flags, data, sizeof(flags));
>> > 
>> > conversion from bytestream to integer: I think in this case it
>> > would be better to use
>> > 
>> > flags = cpu_to_be64p((__u64*)data);
>> > 
>> > so that the flags always in hypervisor/big endian format
>> 
>> I don't think it's correct to byte swap the flags here. They must be
>> in big endian format, but that's up to the caller.
>
> That was what I initially thought, until I went and tested it properly
> and found it was indeed broken (at least in our qemu environment, this
> is slightly tricky for me to test right now on real hardware with real
> PowerVM) depending on kernel endianness.
>
> - Userspace writes the flags into the buffer in BE order
>
> - The first 8 bytes of the buffer are memcpy()ed, in BE order, into
> flags (a u64)
>
> - plpar_hcall9() is called with flags as an argument, loaded into r9
>
> - r9 is moved to r8 before jumping into the hypervisor
>
> On a BE system, this works fine. On an LE system, this results in the
> bytes in the flags variable being loaded into the register in LE order,
> so the conversion is necessary.

Ah yep of course. So although the flags are written by userspace as part
of the data as a stream of bytes, they're passed to the HV via a
register.

I've had this patch in next for a few days and don't want to rebase it.
So can you send a follow-up patch to fix the flags endianess, with a
nice changelog and comment :)

cheers