Messages in this thread Patch in this message | | | Date | Sun, 17 Sep 2023 21:10:05 -0600 | Subject | Re: [PATCH] udmabuf: Fix a potential (and unlikely) access to unallocated memory | From | "Gustavo A. R. Silva" <> |
| |
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
| |