xen/gntdev: Prevent leaking grants

In the Linux kernel, the following vulnerability has been resolved:

xen/gntdev: Prevent leaking grants

Prior to this commit, if a grant mapping operation failed partially,

some of the entries in the map_ops array would be invalid, whereas all

of the entries in the kmap_ops array would be valid. This in turn would

cause the following logic in gntdev_map_grant_pages to become invalid:

for (i = 0; i < map->count; i++) {

if (map->map_ops[i].status == GNTST_okay) {

map->unmap_ops[i].handle = map->map_ops[i].handle;

if (!use_ptemod)

alloced++;

}

if (use_ptemod) {

if (map->kmap_ops[i].status == GNTST_okay) {

if (map->map_ops[i].status == GNTST_okay)

alloced++;

map->kunmap_ops[i].handle = map->kmap_ops[i].handle;

}

}

}

...

atomic_add(alloced, &map->live_grants);

Assume that use_ptemod is true (i.e., the domain mapping the granted

pages is a paravirtualized domain). In the code excerpt above, note that

the "alloced" variable is only incremented when both kmap_ops[i].status

and map_ops[i].status are set to GNTST_okay (i.e., both mapping

operations are successful). However, as also noted above, there are

cases where a grant mapping operation fails partially, breaking the

assumption of the code excerpt above.

The aforementioned causes map->live_grants to be incorrectly set. In

some cases, all of the map_ops mappings fail, but all of the kmap_ops

mappings succeed, meaning that live_grants may remain zero. This in turn

makes it impossible to unmap the successfully grant-mapped pages pointed

to by kmap_ops, because unmap_grant_pages has the following snippet of

code at its beginning:

if (atomic_read(&map->live_grants) == 0)

return; /* Nothing to do */

In other cases where only some of the map_ops mappings fail but all

kmap_ops mappings succeed, live_grants is made positive, but when the

user requests unmapping the grant-mapped pages, __unmap_grant_pages_done

will then make map->live_grants negative, because the latter function

does not check if all of the pages that were requested to be unmapped

were actually unmapped, and the same function unconditionally subtracts

"data->count" (i.e., a value that can be greater than map->live_grants)

from map->live_grants. The side effects of a negative live_grants value

have not been studied.

The net effect of all of this is that grant references are leaked in one

of the above conditions. In Qubes OS v4.1 (which uses Xen's grant

mechanism extensively for X11 GUI isolation), this issue manifests

itself with warning messages like the following to be printed out by the

Linux kernel in the VM that had granted pages (that contain X11 GUI

window data) to dom0: "g.e. 0x1234 still pending", especially after the

user rapidly resizes GUI VM windows (causing some grant-mapping

operations to partially or completely fail, due to the fact that the VM

unshares some of the pages as part of the window resizing, making the

pages impossible to grant-map from dom0).

The fix for this issue involves counting all successful map_ops and

kmap_ops mappings separately, and then adding the sum to live_grants.

During unmapping, only the number of successfully unmapped grants is

subtracted from live_grants. The code is also modified to check for

negative live_grants values after the subtraction and warn the user.