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 <[email protected]>

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

Acked-by: Christian König <[email protected]>

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

Reply via email to