Skip to content

Commit 3b06ac6

Browse files
committed
xen/gntdev: replace global limit of mapped pages by limit per call
Today there is a global limit of pages mapped via /dev/xen/gntdev set to 1 million pages per default. There is no reason why that limit is existing, as total number of grant mappings is limited by the hypervisor anyway and preferring kernel mappings over userspace ones doesn't make sense. It should be noted that the gntdev device is usable by root only. Additionally checking of that limit is fragile, as the number of pages to map via one call is specified in a 32-bit unsigned variable which isn't tested to stay within reasonable limits (the only test is the value to be <= zero, which basically excludes only calls without any mapping requested). So trying to map e.g. 0xffff0000 pages while already nearly 1000000 pages are mapped will effectively lower the global number of mapped pages such that a parallel call mapping a reasonable amount of pages can succeed in spite of the global limit being violated. So drop the global limit and introduce per call limit instead. This per call limit (default: 65536 grant mappings) protects against allocating insane large arrays in the kernel for doing a hypercall which will fail anyway in case a user is e.g. trying to map billions of pages. Signed-off-by: Juergen Gross <[email protected]> Reviewed-by: Oleksandr Andrushchenko <[email protected]> Reviewed-by: Boris Ostrovsky <[email protected]> Signed-off-by: Juergen Gross <[email protected]>
1 parent d41b26d commit 3b06ac6

File tree

3 files changed

+11
-26
lines changed

3 files changed

+11
-26
lines changed

drivers/xen/gntdev-common.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ void gntdev_add_map(struct gntdev_priv *priv, struct gntdev_grant_map *add);
8181

8282
void gntdev_put_map(struct gntdev_priv *priv, struct gntdev_grant_map *map);
8383

84-
bool gntdev_account_mapped_pages(int count);
84+
bool gntdev_test_page_count(unsigned int count);
8585

8686
int gntdev_map_grant_pages(struct gntdev_grant_map *map);
8787

drivers/xen/gntdev-dmabuf.c

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ dmabuf_exp_alloc_backing_storage(struct gntdev_priv *priv, int dmabuf_flags,
446446
{
447447
struct gntdev_grant_map *map;
448448

449-
if (unlikely(count <= 0))
449+
if (unlikely(gntdev_test_page_count(count)))
450450
return ERR_PTR(-EINVAL);
451451

452452
if ((dmabuf_flags & GNTDEV_DMA_FLAG_WC) &&
@@ -459,11 +459,6 @@ dmabuf_exp_alloc_backing_storage(struct gntdev_priv *priv, int dmabuf_flags,
459459
if (!map)
460460
return ERR_PTR(-ENOMEM);
461461

462-
if (unlikely(gntdev_account_mapped_pages(count))) {
463-
pr_debug("can't map %d pages: over limit\n", count);
464-
gntdev_put_map(NULL, map);
465-
return ERR_PTR(-ENOMEM);
466-
}
467462
return map;
468463
}
469464

@@ -771,7 +766,7 @@ long gntdev_ioctl_dmabuf_exp_from_refs(struct gntdev_priv *priv, int use_ptemod,
771766
if (copy_from_user(&op, u, sizeof(op)) != 0)
772767
return -EFAULT;
773768

774-
if (unlikely(op.count <= 0))
769+
if (unlikely(gntdev_test_page_count(op.count)))
775770
return -EINVAL;
776771

777772
refs = kcalloc(op.count, sizeof(*refs), GFP_KERNEL);
@@ -818,7 +813,7 @@ long gntdev_ioctl_dmabuf_imp_to_refs(struct gntdev_priv *priv,
818813
if (copy_from_user(&op, u, sizeof(op)) != 0)
819814
return -EFAULT;
820815

821-
if (unlikely(op.count <= 0))
816+
if (unlikely(gntdev_test_page_count(op.count)))
822817
return -EINVAL;
823818

824819
gntdev_dmabuf = dmabuf_imp_to_refs(priv->dmabuf_priv,

drivers/xen/gntdev.c

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,10 @@ MODULE_AUTHOR("Derek G. Murray <[email protected]>, "
5555
"Gerd Hoffmann <[email protected]>");
5656
MODULE_DESCRIPTION("User-space granted page access driver");
5757

58-
static int limit = 1024*1024;
59-
module_param(limit, int, 0644);
60-
MODULE_PARM_DESC(limit, "Maximum number of grants that may be mapped by "
61-
"the gntdev device");
62-
63-
static atomic_t pages_mapped = ATOMIC_INIT(0);
58+
static unsigned int limit = 64*1024;
59+
module_param(limit, uint, 0644);
60+
MODULE_PARM_DESC(limit,
61+
"Maximum number of grants that may be mapped by one mapping request");
6462

6563
static int use_ptemod;
6664

@@ -71,9 +69,9 @@ static struct miscdevice gntdev_miscdev;
7169

7270
/* ------------------------------------------------------------------ */
7371

74-
bool gntdev_account_mapped_pages(int count)
72+
bool gntdev_test_page_count(unsigned int count)
7573
{
76-
return atomic_add_return(count, &pages_mapped) > limit;
74+
return !count || count > limit;
7775
}
7876

7977
static void gntdev_print_maps(struct gntdev_priv *priv,
@@ -241,8 +239,6 @@ void gntdev_put_map(struct gntdev_priv *priv, struct gntdev_grant_map *map)
241239
if (!refcount_dec_and_test(&map->users))
242240
return;
243241

244-
atomic_sub(map->count, &pages_mapped);
245-
246242
if (map->notify.flags & UNMAP_NOTIFY_SEND_EVENT) {
247243
notify_remote_via_evtchn(map->notify.event);
248244
evtchn_put(map->notify.event);
@@ -568,20 +564,14 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv *priv,
568564
if (copy_from_user(&op, u, sizeof(op)) != 0)
569565
return -EFAULT;
570566
pr_debug("priv %p, add %d\n", priv, op.count);
571-
if (unlikely(op.count <= 0))
567+
if (unlikely(gntdev_test_page_count(op.count)))
572568
return -EINVAL;
573569

574570
err = -ENOMEM;
575571
map = gntdev_alloc_map(priv, op.count, 0 /* This is not a dma-buf. */);
576572
if (!map)
577573
return err;
578574

579-
if (unlikely(gntdev_account_mapped_pages(op.count))) {
580-
pr_debug("can't map: over limit\n");
581-
gntdev_put_map(NULL, map);
582-
return err;
583-
}
584-
585575
if (copy_from_user(map->grants, &u->refs,
586576
sizeof(map->grants[0]) * op.count) != 0) {
587577
gntdev_put_map(NULL, map);

0 commit comments

Comments
 (0)