Re: [Xen-devel] [PATCH v5 for-4.9 3/4] hvm/dmop: Implement copy_{to, from}_guest_buf_offset() helpers

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: 07 April 2017 20:36
> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich
> <JBeulich@xxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Julien Grall
> <julien.grall@xxxxxxx>
> Subject: [PATCH v5 for-4.9 3/4] hvm/dmop: Implement
> copy_{to,from}_guest_buf_offset() helpers
> 
> copy_{to,from}_guest_buf() are now implemented using an offset of 0.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
> ---
>  xen/arch/x86/hvm/dm.c | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
> index 3d8ae89..d584aba 100644
> --- a/xen/arch/x86/hvm/dm.c
> +++ b/xen/arch/x86/hvm/dm.c
> @@ -37,9 +37,9 @@ struct dmop_bufs {
>  #undef MAX_NR_BUFS
>  };
> 
> -static bool _raw_copy_from_guest_buf(
> +static bool _raw_copy_from_guest_buf_offset(
>      const struct dmop_bufs *bufs, unsigned int idx,
> -    void *dst, size_t dst_bytes)
> +    size_t offset_bytes, void *dst, size_t dst_bytes)
>  {
>      size_t buf_bytes;
> 
> @@ -48,17 +48,20 @@ static bool _raw_copy_from_guest_buf(
> 
>      buf_bytes = bufs->buf[idx].size;
> 
> -    if ( dst_bytes > buf_bytes )
> +    if ( offset_bytes >= dst_bytes ||
> +         (offset_bytes + dst_bytes) < offset_bytes ||
> +         (offset_bytes + dst_bytes) > dst_bytes )
>          return false;
> 
>      memset(dst, 0, dst_bytes);
> 
> -    return !copy_from_guest(dst, bufs->buf[idx].h, dst_bytes);
> +    return !copy_from_guest_offset(dst, bufs->buf[idx].h,
> +                                   offset_bytes, dst_bytes);

Not sure what's going on here. 'buf_bytes' is being assigned but no longer 
seems to be used (since it's dropped from the if statement). Also, I'm not 
entirely sure what range check that if statement is trying to perform. Can we 
at least have a comment it it's actually correct (which I'm not at all 
convinced of).

  Paul

>  }
> 
> -static bool _raw_copy_to_guest_buf(
> +static bool _raw_copy_to_guest_buf_offset(
>      struct dmop_bufs *bufs, unsigned int idx,
> -    const void *src, size_t src_bytes)
> +    size_t offset_bytes, const void *src, size_t src_bytes)
>  {
>      size_t buf_bytes;
> 
> @@ -67,17 +70,28 @@ static bool _raw_copy_to_guest_buf(
> 
>      buf_bytes = bufs->buf[idx].size;
> 
> -    if ( src_bytes > buf_bytes )
> +    if ( offset_bytes >= src_bytes ||
> +         (offset_bytes + src_bytes) < offset_bytes ||
> +         (offset_bytes + src_bytes) > src_bytes )
>          return false;
> 
> -    return !copy_to_guest(bufs->buf[idx].h, src, src_bytes);
> +    return !copy_to_guest_offset(bufs->buf[idx].h, offset_bytes,
> +                                 src, src_bytes);
>  }
> 
> +#define copy_from_guest_buf_offset(bufs, buf_idx, offset_bytes, dst) \
> +    _raw_copy_from_guest_buf_offset(bufs, buf_idx, offset_bytes, \
> +                                    dst, sizeof(*(dst)))
> +
> +#define copy_to_guest_buf_offset(bufs, buf_idx, offset_bytes, src) \
> +    _raw_copy_to_guest_buf_offset(bufs, buf_idx, offset_bytes, \
> +                                  src, sizeof(*(src)))
> +
>  #define copy_from_guest_buf(bufs, buf_idx, dst) \
> -    _raw_copy_from_guest_buf(bufs, buf_idx, dst, sizeof(*(dst)))
> +    copy_from_guest_buf_offset(bufs, buf_idx, 0, dst)
> 
>  #define copy_to_guest_buf(bufs, buf_idx, src) \
> -    _raw_copy_to_guest_buf(bufs, buf_idx, src, sizeof(*(src)))
> +    copy_to_guest_buf_offset(bufs, buf_idx, 0, src)
> 
>  static int track_dirty_vram(struct domain *d, xen_pfn_t first_pfn,
>                              unsigned int nr, struct xen_dm_op_buf *buf)
> --
> 2.1.4


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