linux-kernel - Re: [PATCH v2 2/2] KVM: Move INVPCID type check from vmx and svm to the common kvm_handle

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: <YYLqRRfaiXrWo7Yz@google.com>
Date:   Wed, 3 Nov 2021 20:00:05 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Vipin Sharma <vipinsh@...gle.com>
Cc:     pbonzini@...hat.com, jmattson@...gle.com, dmatlack@...gle.com,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] KVM: Move INVPCID type check from vmx and svm to
 the common kvm_handle_invpcid()

On Wed, Nov 03, 2021, Vipin Sharma wrote:
> This check will be done in switch statement of kvm_handle_invpcid(),

Please make the changelog a stand on its own, i.e. don't rely on the shortlog
for context.

> used by both VMX and SVM. It also removes (type > 3) check.

Use imperative mood, i.e. state what you're doing as a "command", don't refer to
the patch from a third-person point of view.

The changelog also needs to call out that, unlike INVVPID and INVEPT, INVPCID is
not explicitly documented as checking the "type" before reading the operand from
memory.  I.e. there's a subtle, undocumented functional change in this patch.

Something like:

  Handle #GP on INVCPID due to an invalid type in the common switch statement
  instead of relying on callers to manually verify the type is valid.  Unlike
  INVVPID and INVPET, INVPCID is not explicitly documented as checking the type
  before reading the operand from memory, so deferring the type validity check
  until after that point is architecturally allowed.

For the code:

Reviewed-by: Sean Christopherson <seanjc@...gle.com>