2021-12-30 14:26:57

by Pavel Skripkin

[permalink] [raw]
Subject: [PATCH] udmabuf: validate ubuf->pagecount

Syzbot has reported GPF in sg_alloc_append_table_from_pages(). The
problem was in ubuf->pages == ZERO_PTR.

ubuf->pagecount is calculated from arguments passed from user-space. If
user creates udmabuf with list.size == 0 then ubuf->pagecount will be
also equal to zero; it causes kmalloc_array() to return ZERO_PTR.

Fix it by validating ubuf->pagecount before passing it to
kmalloc_array().

Fixes: fbb0de795078 ("Add udmabuf misc device")
Reported-and-tested-by: [email protected]
Signed-off-by: Pavel Skripkin <[email protected]>
---

Happy New Year and Merry Christmas! :)


With regards,
Pavel Skripkin

---
drivers/dma-buf/udmabuf.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index c57a609db75b..e7330684d3b8 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -190,6 +190,10 @@ static long udmabuf_create(struct miscdevice *device,
if (ubuf->pagecount > pglimit)
goto err;
}
+
+ if (!ubuf->pagecount)
+ goto err;
+
ubuf->pages = kmalloc_array(ubuf->pagecount, sizeof(*ubuf->pages),
GFP_KERNEL);
if (!ubuf->pages) {
--
2.34.1



2022-01-12 18:10:31

by Pavel Skripkin

[permalink] [raw]
Subject: Re: [PATCH] udmabuf: validate ubuf->pagecount

On 12/30/21 17:26, Pavel Skripkin wrote:
> Syzbot has reported GPF in sg_alloc_append_table_from_pages(). The
> problem was in ubuf->pages == ZERO_PTR.
>
> ubuf->pagecount is calculated from arguments passed from user-space. If
> user creates udmabuf with list.size == 0 then ubuf->pagecount will be
> also equal to zero; it causes kmalloc_array() to return ZERO_PTR.
>
> Fix it by validating ubuf->pagecount before passing it to
> kmalloc_array().
>
> Fixes: fbb0de795078 ("Add udmabuf misc device")
> Reported-and-tested-by: [email protected]
> Signed-off-by: Pavel Skripkin <[email protected]>
> ---
>

Gentle ping :)

>
> ---
> drivers/dma-buf/udmabuf.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index c57a609db75b..e7330684d3b8 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -190,6 +190,10 @@ static long udmabuf_create(struct miscdevice *device,
> if (ubuf->pagecount > pglimit)
> goto err;
> }
> +
> + if (!ubuf->pagecount)
> + goto err;
> +
> ubuf->pages = kmalloc_array(ubuf->pagecount, sizeof(*ubuf->pages),
> GFP_KERNEL);
> if (!ubuf->pages) {

With regards,
Pavel Skripkin

2022-01-14 22:54:11

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] udmabuf: validate ubuf->pagecount

On Wed, Jan 12, 2022 at 09:08:46PM +0300, Pavel Skripkin wrote:
> On 12/30/21 17:26, Pavel Skripkin wrote:
> > Syzbot has reported GPF in sg_alloc_append_table_from_pages(). The
> > problem was in ubuf->pages == ZERO_PTR.
> >
> > ubuf->pagecount is calculated from arguments passed from user-space. If
> > user creates udmabuf with list.size == 0 then ubuf->pagecount will be
> > also equal to zero; it causes kmalloc_array() to return ZERO_PTR.
> >
> > Fix it by validating ubuf->pagecount before passing it to
> > kmalloc_array().
> >
> > Fixes: fbb0de795078 ("Add udmabuf misc device")
> > Reported-and-tested-by: [email protected]
> > Signed-off-by: Pavel Skripkin <[email protected]>
> > ---
> >
>
> Gentle ping :)

Gerd Hoffmann should pick this one up, pls holler again if it doesn't
happen.
-Daniel

>
> >
> > ---
> > drivers/dma-buf/udmabuf.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> > index c57a609db75b..e7330684d3b8 100644
> > --- a/drivers/dma-buf/udmabuf.c
> > +++ b/drivers/dma-buf/udmabuf.c
> > @@ -190,6 +190,10 @@ static long udmabuf_create(struct miscdevice *device,
> > if (ubuf->pagecount > pglimit)
> > goto err;
> > }
> > +
> > + if (!ubuf->pagecount)
> > + goto err;
> > +
> > ubuf->pages = kmalloc_array(ubuf->pagecount, sizeof(*ubuf->pages),
> > GFP_KERNEL);
> > if (!ubuf->pages) {
>
> With regards,
> Pavel Skripkin

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2022-01-19 20:11:58

by Gerd Hoffmann

[permalink] [raw]
Subject: Re: [PATCH] udmabuf: validate ubuf->pagecount

On Thu, Dec 30, 2021 at 05:26:49PM +0300, Pavel Skripkin wrote:
> Syzbot has reported GPF in sg_alloc_append_table_from_pages(). The
> problem was in ubuf->pages == ZERO_PTR.
>
> ubuf->pagecount is calculated from arguments passed from user-space. If
> user creates udmabuf with list.size == 0 then ubuf->pagecount will be
> also equal to zero; it causes kmalloc_array() to return ZERO_PTR.
>
> Fix it by validating ubuf->pagecount before passing it to
> kmalloc_array().
>
> Fixes: fbb0de795078 ("Add udmabuf misc device")
> Reported-and-tested-by: [email protected]
> Signed-off-by: Pavel Skripkin <[email protected]>
> ---

Pushed to drm-misc-next.

thanks,
Gerd