linux-kernel - Re: [PATCH v4 19/19] KVM: selftests: Add test for KVM_SET_USER_MEMORY

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] [day] [month] [year] [list]

Date:   Tue, 24 Mar 2020 11:20:48 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Christian Borntraeger <borntraeger@...ibm.com>
Cc:     James Hogan <jhogan@...nel.org>,
        Paul Mackerras <paulus@...abs.org>,
        Janosch Frank <frankja@...ux.ibm.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Marc Zyngier <maz@...nel.org>,
        David Hildenbrand <david@...hat.com>,
        Cornelia Huck <cohuck@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        James Morse <james.morse@....com>,
        Julien Thierry <julien.thierry.kdev@...il.com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        linux-mips@...r.kernel.org, kvm-ppc@...r.kernel.org,
        kvm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        kvmarm@...ts.cs.columbia.edu, linux-kernel@...r.kernel.org,
        Christoffer Dall <christoffer.dall@....com>,
        Philippe Mathieu-Daudé <f4bug@...at.org>
Subject: Re: [PATCH v4 19/19] KVM: selftests: Add test for
 KVM_SET_USER_MEMORY_REGION

On Tue, Mar 24, 2020 at 10:43:07AM +0100, Christian Borntraeger wrote:
> 
> On 18.12.19 17:39, Sean Christopherson wrote:
> > On Wed, Dec 18, 2019 at 12:39:43PM +0100, Christian Borntraeger wrote:
> >>
> I started looking into this what it would cost to implement this on s390.
> s390 is also returning EFAULT if no memory slot is available.
> 
> According to the doc this is not documented at all. So this part of the test
>         vm = vm_create(VM_MODE_DEFAULT, 0, O_RDWR);
>         vm_vcpu_add(vm, VCPU_ID);
>         /* Fails with ENOSPC because the MMU can't create pages (no slots). */
>         TEST_ASSERT(_vcpu_run(vm, VCPU_ID) == -1 && errno == ENOSPC,
>                     "Unexpected error code = %d", errno);
>         kvm_vm_free(vm);
> 
> is actually just testing that the implementation for x86 does not change the error
> from ENOSPC to something else.

It's even worse than that.  There error isn't directly due to no having
a memslots, it occurs because the limit on number of pages in the MMU is
zero.  On x86, that limit is automatically derived from the total size of
memslots.

The selftest could add an explicit ioctl() call to manually override the
number of allowed MMU pages, but that didn't seem any cleaner as it'd still
rely on undocumented internal KVM behavior.

TL;DR: I'm not a huge fan of the code either.

> The question is: do we want to document the error for the "no memslot" case
> and then change all existing platforms?

At first blush, I like the idea of adding an explicit check in KVM_RUN to
return an error if there isn't at least one usable memslot.  But, it'd be a
little weird/kludgy on x86/VMX due to the existence of "private" memslots,
i.e. the check should really fire on "no public memslots".  At that point,
I'm not sure whether the well defined errno would be worth the extra code.