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@xxxxxxxxx>

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

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

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));