Re: [PATCH] KVM: s390: vsie: BUG correction by shadow_crycb

[David Hildenbrand]

On 21.08.2018 16:43, Janosch Frank wrote:
> On 21.08.2018 16:19, Pierre Morel wrote:
>> Copy the key mask to the right offset inside the shadow CRYCB
>>
>> Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
>> ---
>>  arch/s390/kvm/vsie.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>> index 63844b9..a2b28cd 100644
>> --- a/arch/s390/kvm/vsie.c
>> +++ b/arch/s390/kvm/vsie.c
>> @@ -173,7 +173,8 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>  		return set_validity_icpt(scb_s, 0x0039U);
>>  
>>  	/* copy only the wrapping keys */
>> -	if (read_guest_real(vcpu, crycb_addr + 72, &vsie_page->crycb, 56))
>> +	if (read_guest_real(vcpu, crycb_addr + 72,
>> +			    vsie_page->crycb.dea_wrapping_key_mask, 56))
>>  		return set_validity_icpt(scb_s, 0x0035U);
>>  
>>  	scb_s->ecb3 |= ecb3_flags;
>>
> 
> Are we able to use offsetof and sizeof here? I'd rather have a few more
> characters than magic offsets.
> What about CC stable?

Thought about both things, too.

1. offsetof and sizeof will most likely make sense (although will most
likely make this very ugly due to the long names involved)

2. I am not sure about wrapping keys. We never had migration problems,
so I assume this does not break migration (maybe it will break once we
fix it on one side :) ). As we xor with the g2 keys, we will never match
the g2 keys. HW will xor with our keys either way, so it cannot match
our keys.

But two g3 guests will now have the same wrapping keys, I assume that's bad?

I guess we'll have to wait for Christian, I remember he was one of the
people that understood what wrapping keys are actually good for (and the
real key used in HW can simply, silently change, e.g. when migrating to
another system - due to the xoring).

Not having understood/looked into the details, I guess this should be
stable material.

> 
> 
> Reviewed-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
> 


-- 

Thanks,

David / dhildenb