[PATCH] dma-buf: udmabuf: make list limit unsigned

[PATCH] dma-buf: udmabuf: make list limit unsigned

[Yousef Alhouseen]

UDMABUF_CREATE_LIST uses list_limit only as an upper bound for the
unsigned entry count supplied by userspace. Negative values have no
useful meaning and complicate the bounds check.

Make the module parameter unsigned and keep the checked array copy so
large counts cannot wrap the allocation size before udmabuf_create()
walks the copied list.

Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
 drivers/dma-buf/udmabuf.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index b4078ec84..620113df3 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -16,8 +16,8 @@
 #include <linux/vmalloc.h>
 #include <linux/iosys-map.h>
 
-static int list_limit = 1024;
-module_param(list_limit, int, 0644);
+static uint list_limit = 1024;
+module_param(list_limit, uint, 0644);
 MODULE_PARM_DESC(list_limit, "udmabuf_create_list->count limit. Default is 1024.");
 
 static int size_limit_mb = 64;
@@ -469,12 +469,10 @@ static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg)
 	struct udmabuf_create_list head;
 	struct udmabuf_create_item *list;
 	int ret = -EINVAL;
-	int limit;
 
 	if (copy_from_user(&head, (void __user *)arg, sizeof(head)))
 		return -EFAULT;
-	limit = READ_ONCE(list_limit);
-	if (!head.count || limit <= 0 || head.count > limit)
+	if (!head.count || head.count > READ_ONCE(list_limit))
 		return -EINVAL;
 	list = memdup_array_user((void __user *)(arg + sizeof(head)),
 				 head.count, sizeof(*list));
-- 
2.54.0

Re: [PATCH] dma-buf: udmabuf: make list limit unsigned

[Christian König]

On 6/25/26 10:57, Yousef Alhouseen wrote:
> UDMABUF_CREATE_LIST uses list_limit only as an upper bound for the
> unsigned entry count supplied by userspace. Negative values have no
> useful meaning and complicate the bounds check.
> 
> Make the module parameter unsigned and keep the checked array copy so
> large counts cannot wrap the allocation size before udmabuf_create()
> walks the copied list.
> 
> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>

Looks good to me, but somebody more familiar with udmabuf should probably take another look.

Acked-by: Christian König <christian.koenig@amd.com>

Regards,
Christian.

> ---
>  drivers/dma-buf/udmabuf.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index b4078ec84..620113df3 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -16,8 +16,8 @@
>  #include <linux/vmalloc.h>
>  #include <linux/iosys-map.h>
>  
> -static int list_limit = 1024;
> -module_param(list_limit, int, 0644);
> +static uint list_limit = 1024;
> +module_param(list_limit, uint, 0644);
>  MODULE_PARM_DESC(list_limit, "udmabuf_create_list->count limit. Default is 1024.");
>  
>  static int size_limit_mb = 64;
> @@ -469,12 +469,10 @@ static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg)
>  	struct udmabuf_create_list head;
>  	struct udmabuf_create_item *list;
>  	int ret = -EINVAL;
> -	int limit;
>  
>  	if (copy_from_user(&head, (void __user *)arg, sizeof(head)))
>  		return -EFAULT;
> -	limit = READ_ONCE(list_limit);
> -	if (!head.count || limit <= 0 || head.count > limit)
> +	if (!head.count || head.count > READ_ONCE(list_limit))
>  		return -EINVAL;
>  	list = memdup_array_user((void __user *)(arg + sizeof(head)),
>  				 head.count, sizeof(*list));