On 08/06/18 12:25, Juergen Gross wrote: > On 08/06/18 12:09, Andrew Cooper wrote: >> On 08/06/18 10:51, Juergen Gross wrote: >>> xencall_alloc_buffer() is used throughout Xen tools for allocating >>> hypercall buffers. Allocation is done at page granularity. For simple >>> administration each allocated set of pages contains a small header >>> holding the number of pages of that set. The hypercall buffer is >>> located directly after the 4 byte sized header, leading to a wrong >>> alignment for e.g. pointers. >>> >>> Repair that by using a 16 byte sized header enforcing the same >>> alignment as malloc(). >>> >>> Signed-off-by: Juergen Gross <[email protected]> >>> --- >>> Not sure whether this wants to be backported. In case the answer is >>> "yes" I think it should be part of 4.11. >>> --- >>> tools/libs/call/buffer.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/tools/libs/call/buffer.c b/tools/libs/call/buffer.c >>> index 2d8fc29ac6..0b6af2db60 100644 >>> --- a/tools/libs/call/buffer.c >>> +++ b/tools/libs/call/buffer.c >>> @@ -151,6 +151,7 @@ void xencall_free_buffer_pages(xencall_handle *xcall, >>> void *p, size_t nr_pages) >>> >>> struct allocation_header { >>> int nr_pages; >> >> This hunk reveals another bug... >> >> There are a rather large number of size_t => int truncations in the >> code, which is surely going to catch people by surprise. (Again - I was >> certain I commented on this during the original review of this library.) > > On the practical side I don't think this really matters here. Passing > more than 2GB as a hypercall buffer is nothing we want to support... > >> >>> + int pad[3]; >> >> This brings it to 16 byte alignment, but is that enough? An xsave state >> block in x86 for example has 64 byte alignment. I suppose we mostly >> care about ARM here, as its memcpy() will be most impacted by this >> misalignment. > > Special purpose buffers can still be directly allocated via > xencall_alloc_buffer_pages(). I don't think we'll need alignment better > than malloc(). > >> Irrespective, might be easier to accomplish with an >> __attribute__((__aligned__($N))) rather than explicit padding? > > I don't mind which way to go here. I'll leave the decision for the > maintainers. :-)
Ping? Juergen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
