[Xen-changelog] [xen master] libs/gnttab: fix FreeBSD gntdev interface

[patchbot@xxxxxxx]

commit af7e907c33e7a8d81448e3fd2ce7939e24b200f8
Author:     Roger Pau Monne <roger.pau@xxxxxxxxxx>
AuthorDate: Tue Apr 17 14:03:41 2018 +0100
Commit:     Wei Liu <wei.liu2@xxxxxxxxxx>
CommitDate: Thu Apr 19 15:50:16 2018 +0100

    libs/gnttab: fix FreeBSD gntdev interface
    
    Current interface to the gntdev in FreeBSD is wrong, and mostly worked
    out of luck before the PTI FreeBSD fixes, when kernel and user-space
    where sharing the same page tables.
    
    On FreeBSD ioctls have the size of the passed struct encoded in the ioctl
    number, because the generic ioctl handler in the OS takes care of
    copying the data from user-space to kernel space, and then calls the
    device specific ioctl handler. Thus using ioctl structs with variable
    sizes is not possible.
    
    The fix is to turn the array of structs at the end of
    ioctl_gntdev_alloc_gref and ioctl_gntdev_map_grant_ref into pointers,
    that can be properly accessed from the kernel gntdev driver using the
    copyin/copyout functions. Note that this is exactly how it's done for
    the privcmd driver.
    
    Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Release-acked-by: Juergen Gross <jgross@xxxxxxxx>
    Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
 tools/include/xen-sys/FreeBSD/gntdev.h |  4 +--
 tools/libs/gnttab/freebsd.c            | 63 +++++++++++++++++-----------------
 2 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/tools/include/xen-sys/FreeBSD/gntdev.h 
b/tools/include/xen-sys/FreeBSD/gntdev.h
index f3af9a46de..1e39e2f51a 100644
--- a/tools/include/xen-sys/FreeBSD/gntdev.h
+++ b/tools/include/xen-sys/FreeBSD/gntdev.h
@@ -138,7 +138,7 @@ struct ioctl_gntdev_alloc_gref {
     /* OUT parameters */
     uint64_t index;
     /* Variable OUT parameter */
-    uint32_t gref_ids[1];
+    uint32_t *gref_ids;
 };
 
 #define GNTDEV_ALLOC_FLAG_WRITABLE 1
@@ -167,7 +167,7 @@ struct ioctl_gntdev_map_grant_ref {
     /* OUT parameters */
     uint64_t index;
     /* Variable IN parameter */
-    struct ioctl_gntdev_grant_ref refs[1];
+    struct ioctl_gntdev_grant_ref *refs;
 };
 
 #define IOCTL_GNTDEV_UNMAP_GRANT_REF                                   \
diff --git a/tools/libs/gnttab/freebsd.c b/tools/libs/gnttab/freebsd.c
index 3eaa77235f..5c12fe9b0b 100644
--- a/tools/libs/gnttab/freebsd.c
+++ b/tools/libs/gnttab/freebsd.c
@@ -70,22 +70,21 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
 {
     uint32_t i;
     int fd = xgt->fd;
-    struct ioctl_gntdev_map_grant_ref *map;
+    struct ioctl_gntdev_map_grant_ref map;
     void *addr = NULL;
     int domids_stride;
-    unsigned int map_size = ROUNDUP((sizeof(*map) + (count - 1) *
-                                    sizeof(struct ioctl_gntdev_map_grant_ref)),
-                                    PAGE_SHIFT);
+    unsigned int refs_size = ROUNDUP(count *
+                                     sizeof(struct ioctl_gntdev_map_grant_ref),
+                                     PAGE_SHIFT);
 
     domids_stride = (flags & XENGNTTAB_GRANT_MAP_SINGLE_DOMAIN) ? 0 : 1;
-    if ( map_size <= PAGE_SIZE )
-        map = malloc(sizeof(*map) +
-                     (count - 1) * sizeof(struct ioctl_gntdev_map_grant_ref));
+    if ( refs_size <= PAGE_SIZE )
+        map.refs = malloc(refs_size);
     else
     {
-        map = mmap(NULL, map_size, PROT_READ | PROT_WRITE,
-                   MAP_PRIVATE | MAP_ANON, -1, 0);
-        if ( map == MAP_FAILED )
+        map.refs = mmap(NULL, refs_size, PROT_READ | PROT_WRITE,
+                        MAP_PRIVATE | MAP_ANON, -1, 0);
+        if ( map.refs == MAP_FAILED )
         {
             GTERROR(xgt->logger, "anon mmap of map failed");
             return NULL;
@@ -94,26 +93,26 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
 
     for ( i = 0; i < count; i++ )
     {
-        map->refs[i].domid = domids[i * domids_stride];
-        map->refs[i].ref = refs[i];
+        map.refs[i].domid = domids[i * domids_stride];
+        map.refs[i].ref = refs[i];
     }
 
-    map->count = count;
+    map.count = count;
 
-    if ( ioctl(fd, IOCTL_GNTDEV_MAP_GRANT_REF, map) )
+    if ( ioctl(fd, IOCTL_GNTDEV_MAP_GRANT_REF, &map) )
     {
         GTERROR(xgt->logger, "ioctl MAP_GRANT_REF failed");
         goto out;
     }
 
     addr = mmap(NULL, PAGE_SIZE * count, prot, MAP_SHARED, fd,
-                map->index);
+                map.index);
     if ( addr != MAP_FAILED )
     {
         int rv = 0;
         struct ioctl_gntdev_unmap_notify notify;
 
-        notify.index = map->index;
+        notify.index = map.index;
         notify.action = 0;
         if ( notify_offset < PAGE_SIZE * count )
         {
@@ -141,7 +140,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
 
         /* Unmap the driver slots used to store the grant information. */
         GTERROR(xgt->logger, "mmap failed");
-        unmap_grant.index = map->index;
+        unmap_grant.index = map.index;
         unmap_grant.count = count;
         ioctl(fd, IOCTL_GNTDEV_UNMAP_GRANT_REF, &unmap_grant);
         errno = saved_errno;
@@ -149,10 +148,10 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
     }
 
  out:
-    if ( map_size > PAGE_SIZE )
-        munmap(map, map_size);
+    if ( refs_size > PAGE_SIZE )
+        munmap(map.refs, refs_size);
     else
-        free(map);
+        free(map.refs);
 
     return addr;
 }
@@ -239,16 +238,16 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
     void *area = NULL;
     struct ioctl_gntdev_unmap_notify notify;
     struct ioctl_gntdev_dealloc_gref gref_drop;
-    struct ioctl_gntdev_alloc_gref *gref_info = NULL;
+    struct ioctl_gntdev_alloc_gref gref_info;
 
-    gref_info = malloc(sizeof(*gref_info) + count * sizeof(uint32_t));
-    if ( gref_info == NULL )
+    gref_info.gref_ids = malloc(count * sizeof(uint32_t));
+    if ( gref_info.gref_ids == NULL )
         return NULL;
-    gref_info->domid = domid;
-    gref_info->flags = writable ? GNTDEV_ALLOC_FLAG_WRITABLE : 0;
-    gref_info->count = count;
+    gref_info.domid = domid;
+    gref_info.flags = writable ? GNTDEV_ALLOC_FLAG_WRITABLE : 0;
+    gref_info.count = count;
 
-    err = ioctl(fd, IOCTL_GNTDEV_ALLOC_GREF, gref_info);
+    err = ioctl(fd, IOCTL_GNTDEV_ALLOC_GREF, &gref_info);
     if ( err )
     {
         GSERROR(xgs->logger, "ioctl failed");
@@ -256,7 +255,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
     }
 
     area = mmap(NULL, count * PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED,
-                fd, gref_info->index);
+                fd, gref_info.index);
 
     if ( area == MAP_FAILED )
     {
@@ -265,7 +264,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
         goto out_remove_fdmap;
     }
 
-    notify.index = gref_info->index;
+    notify.index = gref_info.index;
     notify.action = 0;
     if ( notify_offset < PAGE_SIZE * count )
     {
@@ -286,18 +285,18 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
         area = NULL;
     }
 
-    memcpy(refs, gref_info->gref_ids, count * sizeof(uint32_t));
+    memcpy(refs, gref_info.gref_ids, count * sizeof(uint32_t));
 
  out_remove_fdmap:
     /*
      * Removing the mapping from the file descriptor does not cause the
      * pages to be deallocated until the mapping is removed.
      */
-    gref_drop.index = gref_info->index;
+    gref_drop.index = gref_info.index;
     gref_drop.count = count;
     ioctl(fd, IOCTL_GNTDEV_DEALLOC_GREF, &gref_drop);
  out:
-    free(gref_info);
+    free(gref_info.gref_ids);
 
     return area;
 }
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog