Re: [PATCH] KVM: x86: Initialize the PIO bounce buffer on PIT control-port reads

[Sean Christopherson <seanjc@google.com>]

On Sun, Oct 18, 2026, Pablo Azuaje wrote:
> pit_ioport_read() bails out for reads of the control word register
> (port 0x43) without writing to the PIO bounce buffer.  The read is
> nevertheless reported as successful, so the guest receives whatever the
> buffer happens to hold -- typically its own bytes from a previous OUT
> or IN on the same port -- as the readback value.
> 
> Real hardware returns undefined data for reads of the control word
> register, so keep KVM's behavior well-defined and predictable: zero the
> bounce buffer before returning.  The read is still reported as
> successful, and returning the guest's own prior IO bytes as a
> pseudo-readback is observable state with no architectural meaning;
> zeroing is the least-surprise choice.  

Hmm, IMO, returning all ones is more appropriate and would be less surprising as
it's more obviously an error, e.g. mimics bus aborts.  Unless someone objects,
I'll fixup to that when applying.

> This is also hardening rather than a security fix: the leaked bytes belong to
> the guest itself, never to the host.
> 
> Fixes: ee73f656a604 ("KVM: PIT: control word is write-only")
> Signed-off-by: Pablo Azuaje <pablo45154@gmail.com>
> ---
>  arch/x86/kvm/i8254.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
> index 1982b0077ddd..a4c23962f161 100644
> --- a/arch/x86/kvm/i8254.c
> +++ b/arch/x86/kvm/i8254.c
> @@ -522,8 +522,16 @@ static int pit_ioport_read(struct kvm_vcpu *vcpu,
>  		return -EOPNOTSUPP;
>  
>  	addr &= KVM_PIT_CHANNEL_MASK;
> -	if (addr == 3)
> +	if (addr == 3) {
> +		/*
> +		 * The control word register is write-only.  Zero the PIO
> +		 * bounce buffer instead of leaving it untouched, which
> +		 * would feed the guest whatever it last wrote (via OUT or a
> +		 * previous IN) as the readback value.
> +		 */

I'll shorten this to:

		/*
		 * Control words are write-only, arbitrarily return all ones,
		 * e.g. to match bus abort semantics.
		 */

Because this code really should stand on its own (which is the whole point of
hardening this code) and not describe the emulator's rather obscure and subtle
io_read cache behavior.

> +		memset(data, 0, len);
>  		return 0;
> +	}
>  
>  	s = &pit_state->channels[addr];
>  
> -- 
> 2.53.0
> 
>