Re: [PATCH] KVM: x86: Zero reserved synic flags when delivering message to avoid stack leak

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Sean Christopherson <seanjc@google.com> writes:

> Ensure all reserved flags are zeroed when setting the msg_pending flag as
> part of SynIC message delivery, as setting only msg_pending can leak seven
> bits of kernel stack data to the guest.  E.g. gcc typically uses a
> bitwise-OR to set the msg_pending flag, without initializing the on-stack
> variable.
>
> Precisely zero the flags, e.g. as opposed to zeroing the entire structure,
> to make it somewhat more obvious that the flags *need* to be zeroed.  E.g.
> it would be quite easy to misread the initial read of guest memory and
> think that zeroing the entire structure is completely superfluous.
>
> Fixes: 3a0e7731724f ("x86: kvm: hyperv: simplify SynIC message delivery")
> Cc: stable@vger.kernel.org
> Reported-by: Stefan Teodorescu <fane@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/hyperv.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 604651cb2739..8d2669d8ef34 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -802,7 +802,9 @@ static int synic_deliver_msg(struct kvm_vcpu_hv_synic *synic, u32 sint,
>  		if (no_retry)
>  			return 0;
>  
> +		hv_hdr.message_flags.asu8 = 0;
>  		hv_hdr.message_flags.msg_pending = 1;
> +
>  		r = kvm_vcpu_write_guest_page(vcpu, msg_page_gfn,
>  					      &hv_hdr.message_flags,
>  					      msg_off +
>
> base-commit: 76671054f9a1ff6abb976583cd8da37650acdc97

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>

(although, I kind of like the idea of zeroing the whole structure with a
comment that we don't read it as a whole from guest's memory too)

-- 
Vitaly