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