[GIT,PULL] Btrfs

Commit Message

Heiko Carstens Nov. 18, 2013, 9:35 a.m. UTC

On Sun, Nov 17, 2013 at 11:36:04AM +0200, Gleb Natapov wrote:
> On Fri, Nov 15, 2013 at 03:57:23PM +0100, Heiko Carstens wrote:
> > On Fri, Nov 15, 2013 at 02:42:08PM +0100, Geert Uytterhoeven wrote:
> > > I was just going to comment that
> > > 
> > > +       const void *zero_page = (const void *) page_to_phys(ZERO_PAGE(0));
> > > 
> > > won't fly. You can't just cast a physical address to "void *".
> > 
> > Ouch.. I think that only works on s390 because we have a 1:1 mapping for
> > physical to virtual addresses in kernel space due to our split address spaces.
> > 
> > So for btrfs and kvm it should be page_to_virt(), and for the dma_map_single()
> > case I have no idea. :)
> Can you send updated patch for kvm please?

See below. page_to_virt() is only defined for a couple of architectures, so I
used __va(page_to_phys()) instead.
I tested the patch on s390 only...

From b19687bad7e878aaed6edb786a22c6b05e886b97 Mon Sep 17 00:00:00 2001
From: Heiko Carstens <[e-mail]>
Date: Mon, 18 Nov 2013 10:05:57 +0100
Subject: [PATCH] kvm: kvm_clear_guest_page(): fix empty_zero_page usage

Using the address of 'empty_zero_page' as source address in order to clear
a page is wrong. On some architectures empty_zero_page is only the pointer
to the struct page of the empty_zero_page.
Therefore the clear page operation would copy the contents of a couple of
struct pages instead of clearing a page.
For kvm only arm64 is affected by this bug.

To fix this use the ZERO_PAGE macro instead which will return the struct
page address of the empty_zero_page on all architectures.

Signed-off-by: Heiko Carstens <[e-mail]>
---
 virt/kvm/kvm_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Will Deacon Nov. 18, 2013, 10:30 a.m. UTC | #1

On Mon, Nov 18, 2013 at 09:35:55AM +0000, Heiko Carstens wrote:
> From b19687bad7e878aaed6edb786a22c6b05e886b97 Mon Sep 17 00:00:00 2001
> From: Heiko Carstens <[e-mail]>
> Date: Mon, 18 Nov 2013 10:05:57 +0100
> Subject: [PATCH] kvm: kvm_clear_guest_page(): fix empty_zero_page usage
> 
> Using the address of 'empty_zero_page' as source address in order to clear
> a page is wrong. On some architectures empty_zero_page is only the pointer
> to the struct page of the empty_zero_page.
> Therefore the clear page operation would copy the contents of a couple of
> struct pages instead of clearing a page.
> For kvm only arm64 is affected by this bug.

In theory ARM is affected too, but this code doesn't seem to be invoked by
any architecture other than x86 afaict. It looks like there's a similar
issue in drivers/spi/spi-fsl-cpm.c where empty_zero_page is used as a dummy
dma buffer, but that code is basically powerpc only, so again, unlikely to
cause a problem in practice.

Will

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 662f34c3287e..a0aa84b5941a 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1615,8 +1615,9 @@ EXPORT_SYMBOL_GPL(kvm_read_guest_cached);
>  
>  int kvm_clear_guest_page(struct kvm *kvm, gfn_t gfn, int offset, int len)
>  {
> -	return kvm_write_guest_page(kvm, gfn, (const void *) empty_zero_page,
> -				    offset, len);
> +	const void *zero_page = (const void *) __va(page_to_phys(ZERO_PAGE(0)));
> +
> +	return kvm_write_guest_page(kvm, gfn, zero_page, offset, len);
>  }
>  EXPORT_SYMBOL_GPL(kvm_clear_guest_page);
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 662f34c3287e..a0aa84b5941a 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1615,8 +1615,9 @@  EXPORT_SYMBOL_GPL(kvm_read_guest_cached);
 
 int kvm_clear_guest_page(struct kvm *kvm, gfn_t gfn, int offset, int len)
 {
-	return kvm_write_guest_page(kvm, gfn, (const void *) empty_zero_page,
-				    offset, len);
+	const void *zero_page = (const void *) __va(page_to_phys(ZERO_PAGE(0)));
+
+	return kvm_write_guest_page(kvm, gfn, zero_page, offset, len);
 }
 EXPORT_SYMBOL_GPL(kvm_clear_guest_page);