When gntdev_ioctl_map_grant_ref() fails to copy the operation result
back to userspace after successfully adding the mapping to the list,
the error path returns -EFAULT without releasing the reference
acquired by gntdev_alloc_map(). The mapping remains in priv->maps
with a refcount of 1, causing a memory leak and a dangling list
entry.

Additionally, gntdev_add_map() may modify map->index to avoid overlap
with existing mappings. Therefore, the index returned to userspace
must be obtained after gntdev_add_map() completes.

Fix this by holding the mutex across gntdev_add_map(), retrieving
the correct index, and copy_to_user(). If copy_to_user() fails,
remove the mapping from the list and release the reference while
still holding the lock.

Cc: stable@vger.kernel.org

Fix these issues by properly handling all error cases.

Fixes: 1401c00e59ea ("xen/gntdev: convert priv->lock to a mutex")
Fixes: 68b025c813c2 ("xen-gntdev: Add reference counting to maps")

Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/xen/gntdev.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 61ea855c4508..1dcc4675580e 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -670,11 +670,15 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv *priv,
 	mutex_lock(&priv->lock);
 	gntdev_add_map(priv, map);
 	op.index = map->index << PAGE_SHIFT;
-	mutex_unlock(&priv->lock);
 
-	if (copy_to_user(u, &op, sizeof(op)) != 0)
+	if (copy_to_user(u, &op, sizeof(op)) != 0) {
+		list_del(&map->next);
+		mutex_unlock(&priv->lock);
+		gntdev_put_map(priv, map);
 		return -EFAULT;
+	}
 
+	mutex_unlock(&priv->lock);
 	return 0;
 }
 
-- 
2.39.5 (Apple Git-154)

On 22.06.26 13:25, Wentao Liang wrote:
> When gntdev_ioctl_map_grant_ref() fails to copy the operation result
> back to userspace after successfully adding the mapping to the list,
> the error path returns -EFAULT without releasing the reference
> acquired by gntdev_alloc_map(). The mapping remains in priv->maps
> with a refcount of 1, causing a memory leak and a dangling list
> entry.
> 
> Additionally, gntdev_add_map() may modify map->index to avoid overlap
> with existing mappings. Therefore, the index returned to userspace
> must be obtained after gntdev_add_map() completes.
> 
> Fix this by holding the mutex across gntdev_add_map(), retrieving
> the correct index, and copy_to_user(). If copy_to_user() fails,
> remove the mapping from the list and release the reference while
> still holding the lock.
> 
> Cc: stable@vger.kernel.org
> 
> Fix these issues by properly handling all error cases.
> 
> Fixes: 1401c00e59ea ("xen/gntdev: convert priv->lock to a mutex")
> Fixes: 68b025c813c2 ("xen-gntdev: Add reference counting to maps")
> 
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen