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
