[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/4] hvm/dmop: Implement copy_{to, from}_guest_buf() in terms of raw accessors



> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 21 April 2017 09:48
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>; Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>; Jennifer Herbert
> <jennifer.herbert@xxxxxxxxxx>; Xen-devel <xen-devel@xxxxxxxxxxxxx>
> Subject: RE: [PATCH 2/4] hvm/dmop: Implement copy_{to,
> from}_guest_buf() in terms of raw accessors
> 
> >>> On 21.04.17 at 10:02, <Paul.Durrant@xxxxxxxxxx> wrote:
> >> From: jennifer.herbert@xxxxxxxxxx [mailto:jennifer.herbert@xxxxxxxxxx]
> >> Sent: 20 April 2017 19:00
> >> +#define copy_from_guest_buf(dst, args, buf_idx) \
> >> +    _raw_copy_from_guest_buf(dst, args, buf_idx, sizeof(*(dst)))
> >> +
> >> +#define copy_to_guest_buf(args, buf_idx, src) \
> >> +    _raw_copy_to_guest_buf(args, buf_idx, src, sizeof(*(src)))
> >> +
> >
> > Not sure I like the use of sizeof(*<thing>) in a macro. If someone was to
> use
> > these macros and pass a pointer to allocated memory rather than &<thing-
> on-stack>
> > then they would not have the desired effect. Clearly such use would be
> very
> > naïve but I wonder whether having something like:
> >
> > #define copy_to_guest_buf(args, buf_idx, src) \
> >     _raw_copy_to_guest_buf(args, buf_idx, &src, sizeof(src))
> >
> > would be safer.
> 
> You mean in the case the allocated memory was an array? If it's a
> pointer to a singular object, I don't think there would be anything
> wrong.

Yes, or someone erroneously passing a void *. As I said, such code would be 
very naïve, but changing the macro as I suggest would rule it out.

  Paul

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.