[PATCH] io_uring/memmap: bound io_pin_pages() by page array byte size

[PATCH] io_uring/memmap: bound io_pin_pages() by page array byte size

[Deepanshu Kartikey]

io_pin_pages() checks that nr_pages does not exceed INT_MAX, then
allocates a struct page * array of nr_pages entries. kvmalloc() limits
allocations to INT_MAX bytes, but the check counts pages, not bytes.
On 64-bit each entry is 8 bytes, so the array hits the INT_MAX byte
limit at INT_MAX / sizeof(struct page *) pages, well before the page
count check fires.

Since commit b4e41050b212 ("io_uring/rsrc: raise registered buffer 1GB
limit") raised the per-buffer cap to 1TB, a buffer near that cap maps
~2^28 pages, making the array allocation exceed INT_MAX bytes. This
passes the page count check, reaches kvmalloc(), and triggers the
WARN_ON_ONCE() for oversized allocations in __kvmalloc_node_noprof().

Check nr_pages against INT_MAX / sizeof(struct page *) so the buffer is
rejected with -EOVERFLOW before the allocation is attempted.

Reported-by: syzbot+f99b00a963915b6b52c6@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f99b00a963915b6b52c6
Fixes: b4e41050b212 ("io_uring/rsrc: raise registered buffer 1GB limit")
Tested-by: syzbot+f99b00a963915b6b52c6@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 io_uring/memmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/memmap.c b/io_uring/memmap.c
index 4f9b439319c4..da1f6c5d07f8 100644
--- a/io_uring/memmap.c
+++ b/io_uring/memmap.c
@@ -53,7 +53,7 @@ struct page **io_pin_pages(unsigned long uaddr, unsigned long len, int *npages)
 	nr_pages = end - start;
 	if (WARN_ON_ONCE(!nr_pages))
 		return ERR_PTR(-EINVAL);
-	if (WARN_ON_ONCE(nr_pages > INT_MAX))
+	if (nr_pages > INT_MAX / sizeof(struct page *))
 		return ERR_PTR(-EOVERFLOW);
 
 	pages = kvmalloc_objs(struct page *, nr_pages, GFP_KERNEL_ACCOUNT);
-- 
2.43.0

Re: [PATCH] io_uring/memmap: bound io_pin_pages() by page array byte size

[Gabriel Krisman Bertazi]

Deepanshu Kartikey <kartikey406@gmail.com> writes:

> io_pin_pages() checks that nr_pages does not exceed INT_MAX, then
> allocates a struct page * array of nr_pages entries. kvmalloc() limits
> allocations to INT_MAX bytes, but the check counts pages, not bytes.
> On 64-bit each entry is 8 bytes, so the array hits the INT_MAX byte
> limit at INT_MAX / sizeof(struct page *) pages, well before the page
> count check fires.
>
> Since commit b4e41050b212 ("io_uring/rsrc: raise registered buffer 1GB
> limit") raised the per-buffer cap to 1TB, a buffer near that cap maps
> ~2^28 pages, making the array allocation exceed INT_MAX bytes. This
> passes the page count check, reaches kvmalloc(), and triggers the
> WARN_ON_ONCE() for oversized allocations in __kvmalloc_node_noprof().
>
> Check nr_pages against INT_MAX / sizeof(struct page *) so the buffer is
> rejected with -EOVERFLOW before the allocation is attempted.
>
> Reported-by: syzbot+f99b00a963915b6b52c6@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f99b00a963915b6b52c6
> Fixes: b4e41050b212 ("io_uring/rsrc: raise registered buffer 1GB limit")
> Tested-by: syzbot+f99b00a963915b6b52c6@syzkaller.appspotmail.com
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>

Looks good, feel free to add:

Reviewed-by: Gabriel Krisman Bertazi <krisman@suse.de>

> ---
>  io_uring/memmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/io_uring/memmap.c b/io_uring/memmap.c
> index 4f9b439319c4..da1f6c5d07f8 100644
> --- a/io_uring/memmap.c
> +++ b/io_uring/memmap.c
> @@ -53,7 +53,7 @@ struct page **io_pin_pages(unsigned long uaddr, unsigned long len, int *npages)
>  	nr_pages = end - start;
>  	if (WARN_ON_ONCE(!nr_pages))
>  		return ERR_PTR(-EINVAL);
> -	if (WARN_ON_ONCE(nr_pages > INT_MAX))
> +	if (nr_pages > INT_MAX / sizeof(struct page *))
>  		return ERR_PTR(-EOVERFLOW);
>  
>  	pages = kvmalloc_objs(struct page *, nr_pages, GFP_KERNEL_ACCOUNT);
> -- 
> 2.43.0
>

-- 
Gabriel Krisman Bertazi