[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7567c27a-b5d0-41fc-a7e5-d65ed168b39c@kernel.dk>
Date: Mon, 2 Oct 2023 10:43:43 -0600
From: Jens Axboe <axboe@...nel.dk>
To: syzbot <syzbot+2113e61b8848fa7951d8@...kaller.appspotmail.com>,
asml.silence@...il.com, io-uring@...r.kernel.org,
linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [io-uring?] BUG: unable to handle kernel NULL pointer
dereference in __io_remove_buffers (2)
On 10/2/23 8:38 AM, syzbot wrote:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: ec8c298121e3 Merge tag 'x86-urgent-2023-10-01' of git://gi..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=16ef0ed6680000
> kernel config: https://syzkaller.appspot.com/x/.config?x=3be743fa9361d5b0
> dashboard link: https://syzkaller.appspot.com/bug?extid=2113e61b8848fa7951d8
> compiler: arm-linux-gnueabi-gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
> userspace arch: arm
I tried the syz repro in the console output, but can't trigger it. It
also makes very little sense to me... For when there is a reproducer,
the below would perhaps shed some light on it. We have bl->is_mapped ==
1, yet bl->buf_ring is NULL. Probably some artifact of 32-bit arm?
diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index 556f4df25b0f..d5133ac8005f 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -504,6 +504,9 @@ static int io_pin_pbuf_ring(struct io_uring_buf_reg *reg,
return -EINVAL;
}
#endif
+ WARN_ON_ONCE(!pages);
+ WARN_ON_ONCE(!nr_pages);
+ WARN_ON_ONCE(!br);
bl->buf_pages = pages;
bl->buf_nr_pages = nr_pages;
bl->buf_ring = br;
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index d9c853d10587..7034be555334 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -1037,39 +1037,36 @@ struct page **io_pin_pages(unsigned long ubuf, unsigned long len, int *npages)
{
unsigned long start, end, nr_pages;
struct page **pages = NULL;
- int pret, ret = -ENOMEM;
+ int ret;
end = (ubuf + len + PAGE_SIZE - 1) >> PAGE_SHIFT;
start = ubuf >> PAGE_SHIFT;
nr_pages = end - start;
+ WARN_ON(!nr_pages);
pages = kvmalloc_array(nr_pages, sizeof(struct page *), GFP_KERNEL);
if (!pages)
- goto done;
+ return ERR_PTR(-ENOMEM);
- ret = 0;
mmap_read_lock(current->mm);
- pret = pin_user_pages(ubuf, nr_pages, FOLL_WRITE | FOLL_LONGTERM,
- pages);
- if (pret == nr_pages)
+ ret = pin_user_pages(ubuf, nr_pages, FOLL_WRITE | FOLL_LONGTERM, pages);
+ mmap_read_unlock(current->mm);
+
+ /* success, mapped all pages */
+ if (ret == nr_pages) {
*npages = nr_pages;
- else
- ret = pret < 0 ? pret : -EFAULT;
+ return pages;
+ }
- mmap_read_unlock(current->mm);
- if (ret) {
+ /* partial map, or didn't map anything */
+ if (ret >= 0) {
/* if we did partial map, release any pages we did get */
- if (pret > 0)
- unpin_user_pages(pages, pret);
- goto done;
- }
- ret = 0;
-done:
- if (ret < 0) {
- kvfree(pages);
- pages = ERR_PTR(ret);
+ if (ret)
+ unpin_user_pages(pages, ret);
+ ret = -EFAULT;
}
- return pages;
+ kvfree(pages);
+ return ERR_PTR(ret);
}
static int io_sqe_buffer_register(struct io_ring_ctx *ctx, struct iovec *iov,
--
Jens Axboe
Powered by blists - more mailing lists