lkml.org 
[lkml]   [2023]   [Sep]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH] udmabuf: Fix a potential (and unlikely) access to unallocated memory
From


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@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`:

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

\
 
 \ /
  Last update: 2023-09-18 21:09    [W:0.075 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site