On 11/15/19 9:19 AM, Thomas Huth wrote:
> On 14/11/2019 17.21, Janosch Frank wrote:
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>> ---
>> arch/s390/kvm/kvm-s390.c | 19 +++++++++++++------
>> 1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 0fa7c6d9ed0e..9820fde04887 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -4432,13 +4432,21 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>> if (mop->size > MEM_OP_MAX_SIZE)
>> return -E2BIG;
>>
>> - /* Protected guests move instruction data over the satellite
>> + /*
>> + * Protected guests move instruction data over the satellite
>> * block which has its own size limit
>> */
>> if (kvm_s390_pv_is_protected(vcpu->kvm) &&
>> - mop->size > ((vcpu->arch.sie_block->sidad & 0x0f) + 1) * PAGE_SIZE)
>> + mop->size > ((vcpu->arch.sie_block->sidad & 0xff) + 1) * PAGE_SIZE)
>> return -E2BIG;
>>
>> + /* We can currently only offset into the one SIDA page. */
>> + if (kvm_s390_pv_is_protected(vcpu->kvm)) {
>> + mop->gaddr &= ~PAGE_MASK;
>> + if (mop->gaddr + mop->size > PAGE_SIZE)
>> + return -EINVAL;
>> + }
>> +
>> if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
>> tmpbuf = vmalloc(mop->size);
>> if (!tmpbuf)
>> @@ -4451,6 +4459,7 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>> case KVM_S390_MEMOP_LOGICAL_READ:
>> if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
>> if (kvm_s390_pv_is_protected(vcpu->kvm)) {
>> + /* We can always copy into the SIDA */
>> r = 0;
>> break;
>> }
>> @@ -4461,8 +4470,7 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>> if (kvm_s390_pv_is_protected(vcpu->kvm)) {
>> r = 0;
>> if (copy_to_user(uaddr, (void *)vcpu->arch.sie_block->sidad +
>> - (mop->gaddr & ~PAGE_MASK),
>> - mop->size))
>> + mop->gaddr, mop->size))
>> r = -EFAULT;
>> break;
>> }
>> @@ -4485,8 +4493,7 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
>> if (kvm_s390_pv_is_protected(vcpu->kvm)) {
>> r = 0;
>> if (copy_from_user((void *)vcpu->arch.sie_block->sidad +
>> - (mop->gaddr & ~PAGE_MASK), uaddr,
>> - mop->size))
>> + mop->gaddr, uaddr, mop->size))
>> r = -EFAULT;
>> break;
>> }
>>
>
> That looks better, indeed.
>
> Still, is there a way you could also verify that gaddr references the
> right page that is mirrored in the sidad?
>
> Thomas
>
I'm not completely sure if I understand your question correctly.
Checking that is not possible here without also looking at the
instruction bytecode and register contents which would make this patch
ridiculously large with no real benefit.
Attachment:
signature.asc
Description: OpenPGP digital signature
