On Thu, Feb 03, 2011 at 12:18:58PM -0500, Daniel De Graaf wrote:
> Changes since v5:
> - Added a tested xen version to workaround in #4
> - Cleaned up variable names & structures
> - Clarified some of the cleanup in gntalloc
> - Removed copyright statement from public-domain files
>
> [PATCH 1/6] xen-gntdev: Change page limit to be global instead of per-open
> [PATCH 2/6] xen-gntdev: Use find_vma rather than iterating our vma list
> manually
> [PATCH 3/6] xen-gntdev: Add reference counting to maps
> [PATCH 4/6] xen-gntdev: Support mapping in HVM domains
> [PATCH 5/6] xen-gntalloc: Userspace grant allocation driver
> [PATCH 6/6] xen/gntalloc,gntdev: Add unmap notify ioctl
Hey Daniel,
I took a look at the patchset and then the bug-fixes:
Daniel De Graaf (12):
xen-gntdev: Change page limit to be global instead of per-open
xen-gntdev: Use find_vma rather than iterating our vma list manually
xen-gntdev: Add reference counting to maps
xen-gntdev: Support mapping in HVM domains
xen-gntalloc: Userspace grant allocation driver
xen/gntalloc,gntdev: Add unmap notify ioctl
xen-gntdev: Fix memory leak when mmap fails
xen-gntdev: Fix unmap notify on PV domains
xen-gntdev: Use map->vma for checking map validity
xen-gntdev: Avoid unmapping ranges twice
xen-gntdev: prevent using UNMAP_NOTIFY_CLEAR_BYTE on read-only mappings
xen-gntdev: Avoid double-mapping memory
And besides the two questions I posted today they look OK to me. However
I have on question that I think points to a bug.
Say that I call GNTDEV_MAP_GRANT_REF three times. The first time I provide
a count of 4, then 1, and then once more 1.
The first call would end up with priv having:
priv-map[0] => map.count=4, map.user=1, map.index=0. We return op.index as 0.
The next call:
priv-map[0] => map.count=4, map.user=1, map.index=0.
priv-map[1] => map.count=1, map.user=1, map.index=5 (gntdev_add_map
ends up adding the index and the count from the previous map to it). We return
op.index as 20480.
The last call ends up with
priv-map[0] => map.count=4, map.user=1, map.index=0.
priv-map[1] => map.count=1, map.user=1, map.index=5
priv-map[2] => map.count=1, map.user=1, map.index=0. And we return
op.index as = 0.
It looks as gntdev_add_map ends does not do anything to the
map.index if the "if (add->index + add->count < map->index)" comes
out true, and we end up with op.index=0. Which naturally is
incorrect as that is associated with grant_map that has four entries!
I hadn't yet tried to modify the nice test-case program you provided
to see if this is can happen in practice, but it sure looks like it could?
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home