linux-kernel - Re: [Part2 PATCH v5.1 12.4/31] crypto: ccp: Implement SEV_PLATFORM

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: <cd9e71bc-5187-fc88-ad80-4aa171370af6@amd.com>
Date:   Wed, 11 Oct 2017 15:10:49 -0500
From:   Brijesh Singh <brijesh.singh@....com>
To:     Borislav Petkov <bp@...e.de>
Cc:     brijesh.singh@....com, Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Gary Hook <gary.hook@....com>,
        Tom Lendacky <thomas.lendacky@....com>,
        linux-crypto@...r.kernel.org, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [Part2 PATCH v5.1 12.4/31] crypto: ccp: Implement
 SEV_PLATFORM_STATUS ioctl command



On 10/11/2017 03:04 PM, Borislav Petkov wrote:
> On Wed, Oct 11, 2017 at 02:49:55PM -0500, Brijesh Singh wrote:
>> This is OK for now. But in future if FW steals another bit from reserved1
>> field to expose a new flag then 'owner' name will no longer be valid. If you
>> don't to use bit field then we have to use some generic name instead of
>> naming the field as 'owner'. Please note that its not just userspace, KVM
>> driver also uses the same fields and it may also need to know which bit
>> position to use.
> 
> So what is this field called in the spec?
> 

See Section 5.5.2 [1]

[1] http://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf

The bit0 is named as 'owner' and bits 7:1 is left blank (no name is given).


>> This is a tricky one. The 32-bit are packed as:
>>
>> BIT0 - config.es
>> BIT1-23: reserved
>> BIT24-31: build
> 
> Is that what the firmware gives?
> 

Yes

> Then it is easy:
> 
> 	<firmware_field_name> &= GENMASK(23, 1);
> 
> and then userspace can pick apart bit 0 and bit24-31.
> 

The bit0 is named as config.es and bit1-23 is left blank and bit31-24 is 
named as build.

The current 'struct sev_data_status' matches with the firmware names and 
the bit fields. Only thing I did was the fields with no name is called 
as "reservedX"


> Just use one raw struct which the firmware gives you and the rest is
> done by the sw. Like clearing reserved fields before copying to user.
> 
> You don't want to be updating that struct layout later: think of old
> qemu with new kernel and all those different configurations.
>