linux-kernel - Re: [PATCH v5 4/4] KVM: mmu: remove over-aggressive warnings

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: <YdXvUaBUvaRPsv6m@google.com>
Date:   Wed, 5 Jan 2022 19:19:45 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     David Stevens <stevensd@...omium.org>
Cc:     Marc Zyngier <maz@...nel.org>, Paolo Bonzini <pbonzini@...hat.com>,
        James Morse <james.morse@....com>,
        Alexandru Elisei <alexandru.elisei@....com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Will Deacon <will@...nel.org>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH v5 4/4] KVM: mmu: remove over-aggressive warnings

On Wed, Jan 05, 2022, Sean Christopherson wrote:
> Ah, I got royally confused by ensure_pfn_ref()'s comment
> 
>   * Certain IO or PFNMAP mappings can be backed with valid
>   * struct pages, but be allocated without refcounting e.g.,
>   * tail pages of non-compound higher order allocations, which
>   * would then underflow the refcount when the caller does the
>   * required put_page. Don't allow those pages here.
>                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> that doesn't apply here because kvm_faultin_pfn() uses the low level
> __gfn_to_pfn_page_memslot().

On fifth thought, I think this is wrong and doomed to fail.  By mapping these pages
into the guest, KVM is effectively saying it supports these pages.  But if the guest
uses the corresponding gfns for an action that requires KVM to access the page,
e.g. via kvm_vcpu_map(), ensure_pfn_ref() will reject the access and all sorts of
bad things will happen to the guest.

So, why not fully reject these types of pages?  If someone is relying on KVM to
support these types of pages, then we'll fail fast and get a bug report letting us
know we need to properly support these types of pages.  And if not, then we reduce
KVM's complexity and I get to keep my precious WARN :-)