On Mon, Sep 18, 2023 at 09:22:44PM +0200, Christophe JAILLET wrote:
> 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@xxxxxxxxxx>
> > > ---
> > > 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.
I'm on the fence, but kind of lean towards keeping lsize, but I think
it's fine either way.
> Using sizeof(*list) is better.
That I agree with, yes.
> Let see if there are some other comments, and I'll send a v2.
I note that this looks like a use-case for the very recently proposed
memdup_array_user():
https://lore.kernel.org/all/ACD75DAA-AF42-486C-B44B-9272EF302E3D@xxxxxxxxxx/
(i.e. a built-in size_mul)
-Kees
--
Kees Cook
