[Xen-devel] [PATCH] xen/arm: Correctly handle non-page aligned pointer in raw_copy_*

[Julien Grall <julien.grall@xxxxxxxxxx>]

The current implementation of raw_copy_* helpers may lead to data corruption
and sometimes Xen crash when the guest virtual address is not aligned to
PAGE_SIZE.

When the total length is higher than a page, the length to read is badly
compute with
    min(len, (unsigned)(PAGE_SIZE - offset))

As the offset is only computed one time per function, if the start address was
not aligned to PAGE_SIZE, we can end up in same iteration:
    - to read accross page boundary => xen crash
    - read the previous page => data corruption

This issue can be resolved by computing the offset on every iteration.

Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>

---
    This patch is a bug fix for Xen 4.4. Without this patch the data may be
    corrupted between Xen and the guest when the guest virtual address is
    not aligned to PAGE_SIZE. Sometimes it can also crash Xen.

    These functions are used in numerous place in Xen. If it introduce another
    bug we can see quickly with small amount of data.
---
 xen/arch/arm/guestcopy.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index af0af6b..b3b54e9 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -9,12 +9,11 @@ static unsigned long raw_copy_to_guest_helper(void *to, const 
void *from,
                                               unsigned len, int flush_dcache)
 {
     /* XXX needs to handle faults */
-    unsigned offset = (vaddr_t)to & ~PAGE_MASK;
-
     while ( len )
     {
         paddr_t g;
         void *p;
+        unsigned offset = (vaddr_t)to & ~PAGE_MASK;
         unsigned size = min(len, (unsigned)PAGE_SIZE - offset);
 
         if ( gvirt_to_maddr((vaddr_t) to, &g) )
@@ -50,12 +49,12 @@ unsigned long raw_copy_to_guest_flush_dcache(void *to, 
const void *from,
 unsigned long raw_clear_guest(void *to, unsigned len)
 {
     /* XXX needs to handle faults */
-    unsigned offset = (vaddr_t)to & ~PAGE_MASK;
 
     while ( len )
     {
         paddr_t g;
         void *p;
+        unsigned offset = (vaddr_t)to & ~PAGE_MASK;
         unsigned size = min(len, (unsigned)PAGE_SIZE - offset);
 
         if ( gvirt_to_maddr((vaddr_t) to, &g) )
@@ -76,12 +75,11 @@ unsigned long raw_clear_guest(void *to, unsigned len)
 
 unsigned long raw_copy_from_guest(void *to, const void __user *from, unsigned 
len)
 {
-    unsigned offset = (vaddr_t)from & ~PAGE_MASK;
-
     while ( len )
     {
         paddr_t g;
         void *p;
+        unsigned offset = (vaddr_t)from & ~PAGE_MASK;
         unsigned size = min(len, (unsigned)(PAGE_SIZE - offset));
 
         if ( gvirt_to_maddr((vaddr_t) from & PAGE_MASK, &g) )
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel