| Message ID | 1492711189-2857-3-git-send-email-jennifer.herbert@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 20.04.17 at 19:59, <jennifer.herbert@citrix.com> wrote: > @@ -44,15 +45,20 @@ static bool _raw_copy_from_guest_buf(void *dst, > > buf_bytes = args->buf[buf_idx].size; > > - if ( dst_bytes > buf_bytes ) > + if ( offset_bytes >= buf_bytes || > + (offset_bytes + dst_bytes) < offset_bytes || > + (offset_bytes + dst_bytes) > buf_bytes ) > return false; Looks like the first of these checks is redundant with the third one, especially since - afaics - we don't even need to consider the special case of dst_bytes being zero (and if we still wanted to, this could be folded into the second check by making it <= instead of < ). Other than that Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
> -----Original Message----- > From: jennifer.herbert@citrix.com [mailto:jennifer.herbert@citrix.com] > Sent: 20 April 2017 19:00 > To: Xen-devel <xen-devel@lists.xen.org> > Cc: Jennifer Herbert <jennifer.herbert@citrix.com>; Andrew Cooper > <Andrew.Cooper3@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>; > Jan Beulich <JBeulich@suse.com>; Julien Grall <julien.grall@arm.com> > Subject: [PATCH 3/4] hvm/dmop: Implement copy_{to, > from}_guest_buf_offset() helpers > > From: Jennifer Herbert <Jennifer.Herbert@citrix.com> > > copy_{to,from}_guest_buf() are now implemented using an offset of 0. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Jennifer Herbert <Jennifer.Herbert@citrix.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> > -- > CC: Paul Durrant <paul.durrant@citrix.com> > CC: Andrew Cooper <andrew.cooper3@citrix.com> > CC: Jan Beulich <JBeulich@suse.com> > CC: Julien Grall <julien.grall@arm.com> > --- > xen/arch/x86/hvm/dm.c | 48 +++++++++++++++++++++++++++++++++---- > ----------- > 1 file changed, 33 insertions(+), 15 deletions(-) > > diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c > index 3607ddb..6990725 100644 > --- a/xen/arch/x86/hvm/dm.c > +++ b/xen/arch/x86/hvm/dm.c > @@ -32,10 +32,11 @@ struct dmop_args { > struct xen_dm_op_buf buf[2]; > }; > > -static bool _raw_copy_from_guest_buf(void *dst, > - const struct dmop_args *args, > - unsigned int buf_idx, > - size_t dst_bytes) > +static bool _raw_copy_from_guest_buf_offset(void *dst, > + const struct dmop_args *args, > + unsigned int buf_idx, > + size_t offset_bytes, > + size_t dst_bytes) > { > size_t buf_bytes; > > @@ -44,15 +45,20 @@ static bool _raw_copy_from_guest_buf(void *dst, > > buf_bytes = args->buf[buf_idx].size; > > - if ( dst_bytes > buf_bytes ) > + if ( offset_bytes >= buf_bytes || > + (offset_bytes + dst_bytes) < offset_bytes || > + (offset_bytes + dst_bytes) > buf_bytes ) > return false; > > - return !copy_from_guest(dst, args->buf[buf_idx].h, buf_bytes); > + return !copy_from_guest_offset(dst, args->buf[buf_idx].h, > + offset_bytes, dst_bytes); > } > > -static bool _raw_copy_to_guest_buf(struct dmop_args *args, > - unsigned int buf_idx, > - const void *src, size_t src_bytes) > +static bool _raw_copy_to_guest_buf_offset(struct dmop_args *args, > + unsigned int buf_idx, > + size_t offset_bytes, > + const void *src, > + size_t src_bytes) > { > size_t buf_bytes; > > @@ -61,17 +67,29 @@ static bool _raw_copy_to_guest_buf(struct > dmop_args *args, > > buf_bytes = args->buf[buf_idx].size; > > - if ( src_bytes > buf_bytes ) > + > + if ( offset_bytes >= buf_bytes || > + (offset_bytes + src_bytes) < offset_bytes || > + (offset_bytes + src_bytes) > buf_bytes ) > return false; > > - return !copy_to_guest(args->buf[buf_idx].h, src, buf_bytes); > + return !copy_to_guest_offset(args->buf[buf_idx].h, offset_bytes, > + src, src_bytes); > } > > -#define copy_from_guest_buf(dst, args, buf_idx) \ > - _raw_copy_from_guest_buf(dst, args, buf_idx, sizeof(*(dst))) > +#define copy_from_guest_buf_offset(dst, bufs, buf_idx, offset_bytes) \ > + _raw_copy_from_guest_buf_offset(dst, bufs, buf_idx, offset_bytes, \ > + 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(dst, bufs, buf_idx) \ > + copy_from_guest_buf_offset(dst, bufs, buf_idx, 0) > > -#define copy_to_guest_buf(args, buf_idx, src) \ > - _raw_copy_to_guest_buf(args, buf_idx, src, sizeof(*(src))) > +#define copy_to_guest_buf(bufs, buf_idx, 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
diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c index 3607ddb..6990725 100644 --- a/xen/arch/x86/hvm/dm.c +++ b/xen/arch/x86/hvm/dm.c @@ -32,10 +32,11 @@ struct dmop_args { struct xen_dm_op_buf buf[2]; }; -static bool _raw_copy_from_guest_buf(void *dst, - const struct dmop_args *args, - unsigned int buf_idx, - size_t dst_bytes) +static bool _raw_copy_from_guest_buf_offset(void *dst, + const struct dmop_args *args, + unsigned int buf_idx, + size_t offset_bytes, + size_t dst_bytes) { size_t buf_bytes; @@ -44,15 +45,20 @@ static bool _raw_copy_from_guest_buf(void *dst, buf_bytes = args->buf[buf_idx].size; - if ( dst_bytes > buf_bytes ) + if ( offset_bytes >= buf_bytes || + (offset_bytes + dst_bytes) < offset_bytes || + (offset_bytes + dst_bytes) > buf_bytes ) return false; - return !copy_from_guest(dst, args->buf[buf_idx].h, buf_bytes); + return !copy_from_guest_offset(dst, args->buf[buf_idx].h, + offset_bytes, dst_bytes); } -static bool _raw_copy_to_guest_buf(struct dmop_args *args, - unsigned int buf_idx, - const void *src, size_t src_bytes) +static bool _raw_copy_to_guest_buf_offset(struct dmop_args *args, + unsigned int buf_idx, + size_t offset_bytes, + const void *src, + size_t src_bytes) { size_t buf_bytes; @@ -61,17 +67,29 @@ static bool _raw_copy_to_guest_buf(struct dmop_args *args, buf_bytes = args->buf[buf_idx].size; - if ( src_bytes > buf_bytes ) + + if ( offset_bytes >= buf_bytes || + (offset_bytes + src_bytes) < offset_bytes || + (offset_bytes + src_bytes) > buf_bytes ) return false; - return !copy_to_guest(args->buf[buf_idx].h, src, buf_bytes); + return !copy_to_guest_offset(args->buf[buf_idx].h, offset_bytes, + src, src_bytes); } -#define copy_from_guest_buf(dst, args, buf_idx) \ - _raw_copy_from_guest_buf(dst, args, buf_idx, sizeof(*(dst))) +#define copy_from_guest_buf_offset(dst, bufs, buf_idx, offset_bytes) \ + _raw_copy_from_guest_buf_offset(dst, bufs, buf_idx, offset_bytes, \ + 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(dst, bufs, buf_idx) \ + copy_from_guest_buf_offset(dst, bufs, buf_idx, 0) -#define copy_to_guest_buf(args, buf_idx, src) \ - _raw_copy_to_guest_buf(args, buf_idx, src, sizeof(*(src))) +#define copy_to_guest_buf(bufs, buf_idx, 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)