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

[Xen-devel] [PATCH for-4.11] 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>
---
Rationale for acceptance into 4.11:
 - Without this fix the grant table device is not usable on FreeBSD.
 - It affects FreeBSD code exclusively, there's no risk for Linux or
   other OSes.
---
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
Cc: Juergen Gross <jgross@xxxxxxxx>
---
 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;
 }
-- 
2.17.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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