Re: [Xen-devel] [PATCH v2] xen/arm: Correctly handle non-page aligned pointer in raw_copy_from_guest

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-02-18 at 16:56 +0000, Julien Grall wrote:
> The current implementation of raw_copy_guest helper 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 setting offset to 0 at the end of the first
> iteration. Indeed, after it, the virtual guest address is always aligned
> to PAGE_SIZE.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>


> Cc: George Dunlap <george.dunlap@xxxxxxxxxx>
> 
> ---
>     This patch is a bug fix for Xen 4.4. Without this patch the data may be
>     corrupted when Xen is copied data from the guest if the guest virtual
>     address is not aligned to PAGE_SIZE. Sometimes it can also crash Xen.

The crash is what makes this undoubtedly necessary for 4.4 IMHO.

>     This function is used in numerous place in Xen. If it introduces another
>     bug we can see quickly with small amount of data.
> 
>     Changes in v2:
>         - Only raw_copy_from_guest is buggy, the other raw_copy_*
>         helpers where safe because of the "offset = 0" at the end of the loop
>         - Update commit message and title
> ---
>  xen/arch/arm/guestcopy.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
> index af0af6b..715bb4e 100644
> --- a/xen/arch/arm/guestcopy.c
> +++ b/xen/arch/arm/guestcopy.c
> @@ -96,6 +96,11 @@ unsigned long raw_copy_from_guest(void *to, const void 
> __user *from, unsigned le
>          len -= size;
>          from += size;
>          to += size;
> +        /*
> +         * After the first iteration, guest virtual address is correctly
> +         * aligned to PAGE_SIZE.
> +         */

I'd like to duplicate this comment in the other two places too -- if you
are OK with it I will do that as part of committing.

> +        offset = 0;
>      }
>      return 0;
>  }



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