linux-kernel - Re: [PATCH 0/8] KVM: SEV-ES: fixes for string I/O emulation

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: <YXH8hmB64gnwxIx6@google.com>
Date:   Thu, 21 Oct 2021 23:49:26 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        fwilhelm@...gle.com, oupton@...gle.com
Subject: Re: [PATCH 0/8] KVM: SEV-ES: fixes for string I/O emulation

On Wed, Oct 13, 2021, Paolo Bonzini wrote:
> Patches 2 to 7 are a bunch of cleanups to emulator_pio_in and
> emulator_pio_in_out, so that the final SEV code is a little easier
> to reason on.  Just a little, no big promises.

IMO, this series goes in the wrong direction and doesn't make the mess any better,
just different.

The underlying issue is that kernel_pio() does the completely horrendous thing
of consuming vcpu->arch.pio.  That leads to the juggling that this series tries
to clean up, but it's essentially an impossible problem to solve because the
approach itself is broken.

The _only_ reason vcpu->arch.pio (the structure) exists is to snapshot a port I/O
operation that didn't originate from the emulator before exiting to userspace,
i.e. "fast" I/O and now SEV-ES.  Ignoring those two, all info comes from the
emulator and a single flag or even the cui pointer would suffice.

Ditto for pio_data, it's purely needed to let userspace read/write values, its
use directly in any code except those specific paths is just bad code.

So instead of juggling vcpu->arch.pio.count in weird places, just don't set the
damn thing in the first place.

Untested patches attached that frame in where I think we should go with this.

I'll be offline until Monday, apologies for the inconvenience.

View attachment "0001-KVM-x86-Don-t-exit-to-userspace-when-SEV-ES-INS-is-s.patch" of type "text/x-diff" (819 bytes)

View attachment "0002-KVM-x86-WARN-if-emulated-kernel-port-I-O-fails-after.patch" of type "text/x-diff" (1357 bytes)

View attachment "0003-KVM-x86-Use-an-unsigned-int-when-emulating-string-po.patch" of type "text/x-diff" (820 bytes)

View attachment "0004-KVM-x86-Fill-kvm_pio_request-if-and-only-if-KVM-is-e.patch" of type "text/x-diff" (5733 bytes)

View attachment "0005-KVM-x86-Stop-being-clever-and-use-a-completion-handl.patch" of type "text/x-diff" (1198 bytes)

View attachment "0006-KVM-x86-Move-pointer-for-SEV-ES-fast-string-I-O-into.patch" of type "text/x-diff" (1804 bytes)