[PATCH] udmabuf: Fix a potential (and unlikely) access to unallocated memory
Christophe JAILLET
christophe.jaillet at wanadoo.fr
Mon Sep 18 19:22:44 UTC 2023
Le 18/09/2023 à 05:10, Gustavo A. R. Silva a écrit :
>
>
> On 9/18/23 12:46, Christophe JAILLET wrote:
>> If 'list_limit' is set to a very high value, 'lsize' computation could
>> overflow if 'head.count' is big enough.
>>
>> In such a case, udmabuf_create() will access to memory beyond 'list'.
>>
>> Use size_mul() to saturate the value, and have memdup_user() fail.
>>
>> Fixes: fbb0de795078 ("Add udmabuf misc device")
>> Signed-off-by: Christophe JAILLET <christophe.jaillet at wanadoo.fr>
>> ---
>> drivers/dma-buf/udmabuf.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
>> index c40645999648..fb4c4b5b3332 100644
>> --- a/drivers/dma-buf/udmabuf.c
>> +++ b/drivers/dma-buf/udmabuf.c
>> @@ -314,13 +314,13 @@ 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;
>> - u32 lsize;
>> + size_t lsize;
>> if (copy_from_user(&head, (void __user *)arg, sizeof(head)))
>> return -EFAULT;
>> if (head.count > list_limit)
>> return -EINVAL;
>> - lsize = sizeof(struct udmabuf_create_item) * head.count;
>> + lsize = size_mul(sizeof(struct udmabuf_create_item), head.count);
>> list = memdup_user((void __user *)(arg + sizeof(head)), lsize);
>> if (IS_ERR(list))
>> return PTR_ERR(list);
>
> How about this, and we get rid of `lsize`:
Keeping or removing lsize is mostly a matter of taste, I think.
Using sizeof(*list) is better.
Let see if there are some other comments, and I'll send a v2.
Thanks for the feed-back.
CJ
>
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index c40645999648..5cf9d849aaa8 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -314,14 +314,13 @@ 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;
> - u32 lsize;
>
> if (copy_from_user(&head, (void __user *)arg, sizeof(head)))
> return -EFAULT;
> if (head.count > list_limit)
> return -EINVAL;
> - lsize = sizeof(struct udmabuf_create_item) * head.count;
> - list = memdup_user((void __user *)(arg + sizeof(head)), lsize);
> + list = memdup_user((void __user *)(arg + sizeof(head)),
> + size_mul(sizeof(*list), head.count));
> if (IS_ERR(list))
> return PTR_ERR(list);
>
>
> --
> Gustavo
>
More information about the dri-devel
mailing list