linux-kernel - Re: [kvm-devel] [PATCH 02/33] KVM: MMU: emulated cmpxchg8b should be atomic on i386

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]

Date:	Sun, 6 Jan 2008 12:57:28 -0600
From:	Carlo Marcelo Arenas Belon <carenas@...inet.com.pe>
To:	Avi Kivity <avi@...ranet.com>
Cc:	linux-kernel@...r.kernel.org, kvm-devel@...ts.sourceforge.net,
	Marcelo Tosatti <marcelo@...ck.org>
Subject: Re: [kvm-devel] [PATCH 02/33] KVM: MMU: emulated cmpxchg8b should be atomic on i386

On Sun, Jan 06, 2008 at 04:39:08PM +0200, Avi Kivity wrote:
> From: Marcelo Tosatti <marcelo@...ck.org>
> 
> Emulate cmpxchg8b atomically on i386. This is required to avoid a guest
> pte walker from seeing a splitted write.
> 
> [avi: make it compile]
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@...hat.com>
> Signed-off-by: Avi Kivity <avi@...ranet.com>
> ---

This one doesn't look like the one in the current tree, and avoids the reuse
of addr by moving the call to get the gpa up instead of renaming the variable
as propossed in :

  http://article.gmane.org/gmane.comp.emulators.kvm.devel/11511

IMHO renaming the parameter variable is probably better in the long run, as it
could result otherwise into scope related bugs like the one from kvm-59

> diff --git a/drivers/kvm/x86.c b/drivers/kvm/x86.c
> index 9db4e32..3b79684 100644
> --- a/drivers/kvm/x86.c
> +++ b/drivers/kvm/x86.c
> @@ -1674,6 +1675,31 @@ static int emulator_cmpxchg_emulated(unsigned long addr,
>  		reported = 1;
>  		printk(KERN_WARNING "kvm: emulating exchange as write\n");
>  	}
> +#ifndef CONFIG_X86_64
> +	/* guests cmpxchg8b have to be emulated atomically */
> +	if (bytes == 8) {
> +		gpa_t gpa = vcpu->mmu.gva_to_gpa(vcpu, addr);
> +		struct page *page;
> +		char *addr;
> +		u64 val;
> +
> +		if (gpa == UNMAPPED_GVA ||
> +		   (gpa & PAGE_MASK) == APIC_DEFAULT_PHYS_BASE)
> +			goto emul_write;
> +
> +		if (((gpa + bytes - 1) & PAGE_MASK) != (gpa & PAGE_MASK))
> +			goto emul_write;
> +
> +		val = *(u64 *)new;
> +		page = gfn_to_page(vcpu->kvm, gpa >> PAGE_SHIFT);
> +		addr = kmap_atomic(page, KM_USER0);
> +		set_64bit((u64 *)(addr + offset_in_page(gpa)), val);
> +		kunmap_atomic(addr, KM_USER0);
> +		kvm_release_page_dirty(page);
> +	}
> +emul_write:
> +#endif
> +
>  	return emulator_write_emulated(addr, new, bytes, vcpu);
>  }
>  
> -- 
> 1.5.3.7
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/