vhost_iotlb: bound map allocation in add_range

vhost_iotlb_add_range_ctx() only retires an old entry when the table
has a non-zero limit, has exactly reached that limit and has
VHOST_IOTLB_FLAG_RETIRE set. Non-retiring tables can keep allocating
entries after reaching their configured limit.

Existing vhost devices allocate their IOTLB with max_iotlb_entries from
vhost.c, which defaults to 2048 and is tunable by module parameter. Use
the caller-provided limit at the allocation point instead of adding a
separate default in the common IOTLB helper, and reject non-positive
values in vhost paths that can report an error.

Other vhost IOTLB users should not create zero-limit tables when entries
can be populated from userspace or guest-controlled requests. Add
caller-side max_iotlb_entries parameters for mlx5 vDPA, VDUSE and
vhost-vDPA. Reject non-positive VDUSE and vhost-vDPA values, and require
at least two entries for vdpa_sim and mlx5 vDPA paths that install
full-range mappings, since those mappings are split into two IOTLB
entries.

Handle full-range mappings in the common helper by checking that the
IOTLB can hold both split entries before inserting the first half. This
avoids returning an error after leaving a half mapping behind.

When the table is full, keep the existing retire behavior for retiring
tables and return -ENOSPC for non-retiring tables. Reuse the retired map
node instead of freeing it and allocating a replacement, so a stream of
IOTLB updates cannot keep forcing GFP_ATOMIC allocations after the table
has reached its limit. If a zero-limit IOTLB still reaches the common
helper, treat it as a configuration error and return -EINVAL.

I found this bug myself, though the patch was written with AI assistance.

Fixes: 0bbe30668d89 ("vhost: factor out IOTLB")
Assisted-by: OpenAI-Codex:GPT-5
Signed-off-by: Linfeng Sun <linfeng.sun.dev@gamil.com>
Message-ID: <AMYAtgAiKmgYcSQT5ukl-4qq.3.1781960405943.Hmail.241270009@hdu.edu.cn>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
index 2cedf7e2dbc49595657fefe07c5ae7b9ef6b98f1..42f2f44b383c70167d1a96b4dabc77a6f5933f27 100644 (file)
--- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
+++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
@@ -11,6 +11,8 @@
 
 #define MLX5V_ETH_HARD_MTU (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN)
 
+extern int mlx5_vdpa_max_iotlb_entries;
+
 struct mlx5_vdpa_direct_mr {
        u64 start;
        u64 end;

diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
index 6d02ccf9eb9104969fa38972702fcd598222f761..7d681961a5ccea361159a32135868c21f0e71ce6 100644 (file)
--- a/drivers/vdpa/mlx5/core/mr.c
+++ b/drivers/vdpa/mlx5/core/mr.c
@@ -776,6 +776,9 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev,
 {
        int err;
 
+       if (mlx5_vdpa_max_iotlb_entries < 2)
+               return -EINVAL;
+
        if (iotlb)
                err = create_user_mr(mvdev, mr, iotlb);
        else
@@ -784,7 +787,7 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev,
        if (err)
                return err;
 
-       mr->iotlb = vhost_iotlb_alloc(0, 0);
+       mr->iotlb = vhost_iotlb_alloc(mlx5_vdpa_max_iotlb_entries, 0);
        if (!mr->iotlb) {
                err = -ENOMEM;
                goto err_mr;

diff --git a/drivers/vdpa/mlx5/core/resources.c b/drivers/vdpa/mlx5/core/resources.c
index aeae31d0cefaee2d4e5256a4060d63ffc88c8c6c..28a4d7a35bf4e4e57f9a31fede2429e0674ac8c7 100644 (file)
--- a/drivers/vdpa/mlx5/core/resources.c
+++ b/drivers/vdpa/mlx5/core/resources.c
@@ -3,8 +3,14 @@
 
 #include <linux/iova.h>
 #include <linux/mlx5/driver.h>
+#include <linux/moduleparam.h>
 #include "mlx5_vdpa.h"
 
+int mlx5_vdpa_max_iotlb_entries = 2048;
+module_param_named(max_iotlb_entries, mlx5_vdpa_max_iotlb_entries, int, 0444);
+MODULE_PARM_DESC(max_iotlb_entries,
+                "Maximum number of iotlb entries. (default: 2048)");
+
 static int alloc_pd(struct mlx5_vdpa_dev *dev, u32 *pdn, u16 uid)
 {
        struct mlx5_core_dev *mdev = dev->mdev;
@@ -229,7 +235,10 @@ int mlx5_vdpa_destroy_mkey(struct mlx5_vdpa_dev *mvdev, u32 mkey)
 
 static int init_ctrl_vq(struct mlx5_vdpa_dev *mvdev)
 {
-       mvdev->cvq.iotlb = vhost_iotlb_alloc(0, 0);
+       if (mlx5_vdpa_max_iotlb_entries < 2)
+               return -EINVAL;
+
+       mvdev->cvq.iotlb = vhost_iotlb_alloc(mlx5_vdpa_max_iotlb_entries, 0);
        if (!mvdev->cvq.iotlb)
                return -ENOMEM;
 

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 8cb1cc2ea1391aab8365882f780200e7d1dab339..4d116644851d92c62451cb304b1d91dc65ba5237 100644 (file)
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -34,7 +34,7 @@ MODULE_PARM_DESC(batch_mapping, "Batched mapping 1 -Enable; 0 - Disable");
 static int max_iotlb_entries = 2048;
 module_param(max_iotlb_entries, int, 0444);
 MODULE_PARM_DESC(max_iotlb_entries,
-                "Maximum number of iotlb entries for each address space. 0 means unlimited. (default: 2048)");
+                "Maximum number of iotlb entries for each address space. (default: 2048)");
 
 static bool use_va = true;
 module_param(use_va, bool, 0444);
@@ -201,6 +201,8 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr,
 
        if (!dev_attr->alloc_size)
                return ERR_PTR(-EINVAL);
+       if (max_iotlb_entries < 2)
+               return ERR_PTR(-EINVAL);
 
        if (config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
                if (config->device_features &
@@ -261,8 +263,10 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr,
 
        for (i = 0; i < vdpasim->dev_attr.nas; i++) {
                vhost_iotlb_init(&vdpasim->iommu[i], max_iotlb_entries, 0);
-               vhost_iotlb_add_range(&vdpasim->iommu[i], 0, ULONG_MAX, 0,
-                                     VHOST_MAP_RW);
+               ret = vhost_iotlb_add_range(&vdpasim->iommu[i], 0, ULONG_MAX,
+                                           0, VHOST_MAP_RW);
+               if (ret)
+                       goto err_iommu;
                vdpasim->iommu_pt[i] = true;
        }
 

diff --git a/drivers/vdpa/vdpa_user/iova_domain.c b/drivers/vdpa/vdpa_user/iova_domain.c
index 4dc76c0d0d13d9aa6131f5f31f1bf9a74de67480..b6c958224b7cebfa8a656e86414a1caffe0261c5 100644 (file)
--- a/drivers/vdpa/vdpa_user/iova_domain.c
+++ b/drivers/vdpa/vdpa_user/iova_domain.c
@@ -12,11 +12,17 @@
 #include <linux/file.h>
 #include <linux/anon_inodes.h>
 #include <linux/highmem.h>
+#include <linux/moduleparam.h>
 #include <linux/vmalloc.h>
 #include <linux/vdpa.h>
 
 #include "iova_domain.h"
 
+static int max_iotlb_entries = 2048;
+module_param(max_iotlb_entries, int, 0444);
+MODULE_PARM_DESC(max_iotlb_entries,
+                "Maximum number of iotlb entries. (default: 2048)");
+
 static int vduse_iotlb_add_range(struct vduse_iova_domain *domain,
                                 u64 start, u64 last,
                                 u64 addr, unsigned int perm,
@@ -622,11 +628,14 @@ vduse_domain_create(unsigned long iova_limit, size_t bounce_size)
        if (iova_limit <= bounce_size)
                return NULL;
 
+       if (max_iotlb_entries <= 0)
+               return NULL;
+
        domain = kzalloc_obj(*domain);
        if (!domain)
                return NULL;
 
-       domain->iotlb = vhost_iotlb_alloc(0, 0);
+       domain->iotlb = vhost_iotlb_alloc(max_iotlb_entries, 0);
        if (!domain->iotlb)
                goto err_iotlb;
 

diff --git a/drivers/vhost/iotlb.c b/drivers/vhost/iotlb.c
index e1414c774c344c33b15f5f9fd82c2334683debde..a1d4376a5b872239dc85d337c5b67772122458b1 100644 (file)
--- a/drivers/vhost/iotlb.c
+++ b/drivers/vhost/iotlb.c
@@ -20,6 +20,14 @@ INTERVAL_TREE_DEFINE(struct vhost_iotlb_map,
                     rb, __u64, __subtree_last,
                     START, LAST, static inline, vhost_iotlb_itree);
 
+static void vhost_iotlb_map_unlink(struct vhost_iotlb *iotlb,
+                                  struct vhost_iotlb_map *map)
+{
+       vhost_iotlb_itree_remove(map, &iotlb->root);
+       list_del(&map->link);
+       iotlb->nmaps--;
+}
+
 /**
  * vhost_iotlb_map_free - remove a map node and free it
  * @iotlb: the IOTLB
@@ -28,10 +36,8 @@ INTERVAL_TREE_DEFINE(struct vhost_iotlb_map,
 void vhost_iotlb_map_free(struct vhost_iotlb *iotlb,
                          struct vhost_iotlb_map *map)
 {
-       vhost_iotlb_itree_remove(map, &iotlb->root);
-       list_del(&map->link);
+       vhost_iotlb_map_unlink(iotlb, map);
        kfree(map);
-       iotlb->nmaps--;
 }
 EXPORT_SYMBOL_GPL(vhost_iotlb_map_free);
 
@@ -57,14 +63,25 @@ int vhost_iotlb_add_range_ctx(struct vhost_iotlb *iotlb,
        if (last < start)
                return -EFAULT;
 
+       if (!iotlb->limit)
+               return -EINVAL;
+
        /* If the range being mapped is [0, ULONG_MAX], split it into two entries
         * otherwise its size would overflow u64.
         */
        if (start == 0 && last == ULONG_MAX) {
                u64 mid = last / 2;
-               int err = vhost_iotlb_add_range_ctx(iotlb, start, mid, addr,
-                               perm, opaque);
+               int err;
+
+               if (iotlb->limit < 2)
+                       return -ENOSPC;
 
+               if (!(iotlb->flags & VHOST_IOTLB_FLAG_RETIRE) &&
+                   iotlb->nmaps > iotlb->limit - 2)
+                       return -ENOSPC;
+
+               err = vhost_iotlb_add_range_ctx(iotlb, start, mid, addr,
+                                               perm, opaque);
                if (err)
                        return err;
 
@@ -72,17 +89,19 @@ int vhost_iotlb_add_range_ctx(struct vhost_iotlb *iotlb,
                start = mid + 1;
        }
 
-       if (iotlb->limit &&
-           iotlb->nmaps == iotlb->limit &&
-           iotlb->flags & VHOST_IOTLB_FLAG_RETIRE) {
-               map = list_first_entry(&iotlb->list, typeof(*map), link);
-               vhost_iotlb_map_free(iotlb, map);
+       if (iotlb->nmaps >= iotlb->limit) {
+               if (iotlb->flags & VHOST_IOTLB_FLAG_RETIRE) {
+                       map = list_first_entry(&iotlb->list, typeof(*map), link);
+                       vhost_iotlb_map_unlink(iotlb, map);
+               } else {
+                       return -ENOSPC;
+               }
+       } else {
+               map = kmalloc_obj(*map, GFP_ATOMIC);
+               if (!map)
+                       return -ENOMEM;
        }
 
-       map = kmalloc_obj(*map, GFP_ATOMIC);
-       if (!map)
-               return -ENOMEM;
-
        map->start = start;
        map->size = last - start + 1;
        map->last = last;

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index ac55275fa0d0ae863121e20747395b2e8e42e21c..ef642bc9f97e11f956c67590d8ff56adb22646e7 100644 (file)
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -34,6 +34,11 @@ enum {
 
 #define VHOST_VDPA_DEV_MAX (1U << MINORBITS)
 
+static int max_iotlb_entries = 2048;
+module_param(max_iotlb_entries, int, 0444);
+MODULE_PARM_DESC(max_iotlb_entries,
+                "Maximum number of iotlb entries. (default: 2048)");
+
 #define VHOST_VDPA_IOTLB_BUCKETS 16
 
 struct vhost_vdpa_as {
@@ -109,12 +114,14 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
 
        if (asid >= v->vdpa->nas)
                return NULL;
+       if (max_iotlb_entries <= 0)
+               return NULL;
 
        as = kmalloc_obj(*as);
        if (!as)
                return NULL;
 
-       vhost_iotlb_init(&as->iotlb, 0, 0);
+       vhost_iotlb_init(&as->iotlb, max_iotlb_entries, 0);
        as->id = asid;
        hlist_add_head(&as->hash_link, head);
 

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 4c525b3e16ead843d3aca6d4b3aefadcc9671167..ae29a7ef7baad82776fcf413b82bcf362628da81 100644 (file)
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1137,6 +1137,9 @@ EXPORT_SYMBOL_GPL(vhost_dev_set_owner);
 
 static struct vhost_iotlb *iotlb_alloc(void)
 {
+       if (max_iotlb_entries <= 0)
+               return NULL;
+
        return vhost_iotlb_alloc(max_iotlb_entries,
                                 VHOST_IOTLB_FLAG_RETIRE);
 }
@@ -1981,6 +1984,8 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
                return -EOPNOTSUPP;
        if (mem.nregions > max_mem_regions)
                return -E2BIG;
+       if (max_iotlb_entries <= 0)
+               return -EINVAL;
        newmem = kvzalloc_flex(*newmem, regions, mem.nregions);
        if (!newmem)
                return -ENOMEM;
@@ -2275,6 +2280,9 @@ int vhost_init_device_iotlb(struct vhost_dev *d)
        struct vhost_iotlb *niotlb, *oiotlb;
        int i;
 
+       if (max_iotlb_entries <= 0)
+               return -EINVAL;
+
        niotlb = iotlb_alloc();
        if (!niotlb)
                return -ENOMEM;