Re: [PATCH stable 4.4] KVM: x86: Fix misplaced backport of "work around leak of uninitialized stack contents"

[Greg KH <gregkh@linuxfoundation.org>]

On Tue, Feb 01, 2022 at 06:17:51PM +0100, Guillaume Bertholon wrote:
> The upstream commit 541ab2aeb282 ("KVM: x86: work around leak of
> uninitialized stack contents") resets `exception` in the function
> `kvm_write_guest_virt_system`.
> However, its backported version in stable (commit ba7f1c934f2e
> ("KVM: x86: work around leak of uninitialized stack contents")) applied
> the change in `emulator_write_std` instead.
> 
> This patch moves the memset instruction back to
> `kvm_write_guest_virt_system`.
> 
> Fixes: ba7f1c934f2e ("KVM: x86: work around leak of uninitialized stack contents")
> Signed-off-by: Guillaume Bertholon <guillaume.bertholon@ens.fr>
> ---
>  arch/x86/kvm/x86.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 8dce61c..9101002 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4417,13 +4417,6 @@ static int emulator_write_std(struct x86_emulate_ctxt *ctxt, gva_t addr, void *v
>  	if (!system && kvm_x86_ops->get_cpl(vcpu) == 3)
>  		access |= PFERR_USER_MASK;
> 
> -	/*
> -	 * FIXME: this should call handle_emulation_failure if X86EMUL_IO_NEEDED
> -	 * is returned, but our callers are not ready for that and they blindly
> -	 * call kvm_inject_page_fault.  Ensure that they at least do not leak
> -	 * uninitialized kernel stack memory into cr2 and error code.
> -	 */
> -	memset(exception, 0, sizeof(*exception));
>  	return kvm_write_guest_virt_helper(addr, val, bytes, vcpu,
>  					   access, exception);
>  }
> @@ -4431,6 +4424,13 @@ static int emulator_write_std(struct x86_emulate_ctxt *ctxt, gva_t addr, void *v
>  int kvm_write_guest_virt_system(struct kvm_vcpu *vcpu, gva_t addr, void *val,
>  				unsigned int bytes, struct x86_exception *exception)
>  {
> +	/*
> +	 * FIXME: this should call handle_emulation_failure if X86EMUL_IO_NEEDED
> +	 * is returned, but our callers are not ready for that and they blindly
> +	 * call kvm_inject_page_fault.  Ensure that they at least do not leak
> +	 * uninitialized kernel stack memory into cr2 and error code.
> +	 */
> +	memset(exception, 0, sizeof(*exception));
>  	return kvm_write_guest_virt_helper(addr, val, bytes, vcpu,
>  					   PFERR_WRITE_MASK, exception);
>  }
> --
> 2.7.4
> 

All 3 now queued up.

Note, 4.4.y is about to go end-of-life now, so I wouldn't spend much
more time on it if you do not want to.

thanks,

greg k-h