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

[George Dunlap <george.dunlap@eu.citrix.com>]

On 02/18/2014 04:56 PM, 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@linaro.org>
> Cc: George Dunlap <george.dunlap@citrix.com>
>
> ---
>      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.
>
>      This function is used in numerous place in Xen. If it introduces another
>      bug we can see quickly with small amount of data.

Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com>

>
>      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.
> +         */
> +        offset = 0;
>       }
>       return 0;
>   }