Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] io_uring/rw: dmabuf_map leaked on page waitqueue retry and cancellation --
commit 671f0754c33400252fbbb2aab640bc25813b29e5 Author: Pavel Begunkov <[email protected]> io_uring/rsrc: add dmabuf backed registered buffers Implement dmabuf backed registered buffers. To register them, the user should specify IO_REGBUF_TYPE_DMABUF for the regitration and pass the desired dmabuf fd and a file for which it should be registered. > diff --git a/io_uring/dma-buf.h b/io_uring/dma-buf.h > new file mode 100644 > index 0000000000000..a876a9e5e272b > --- /dev/null > +++ b/io_uring/dma-buf.h [ ... ] > +static inline int io_attach_dmabuf_map(struct io_kiocb *req, > + struct iov_iter *iter, > + unsigned issue_flags) > +{ > + bool nowait = issue_flags & IO_URING_F_NONBLOCK; > + struct dma_buf_io_map *map; > + struct io_buf_dma *bd; > + > + if (!IS_ENABLED(CONFIG_DMA_SHARED_BUFFER)) > + return -EOPNOTSUPP; > + if (!(req->flags & REQ_F_DMABUF)) > + return 0; > + > + bd = req->buf_node->buf->priv; > + map = dma_buf_io_get_map(bd->ctx, nowait); > + if (unlikely(IS_ERR(map))) > + return PTR_ERR(map); > + req->dmabuf_map = map; [Severity: High] If a request is re-issued (for example, after returning -EIOCBQUEUED and being woken up by the waitqueue), io_init_rw_fixed() is called again and unconditionally invokes io_attach_dmabuf_map(). Does this overwrite req->dmabuf_map with a new reference without dropping the old one, causing a memory leak of the dma_buf_io_map reference? > + iter->dmabuf_map = map; > + return 0; > +} [ ... ] > diff --git a/io_uring/rw.c b/io_uring/rw.c > index 432820f862514..ba0e3e7de557d 100644 > --- a/io_uring/rw.c > +++ b/io_uring/rw.c [ ... ] > @@ -1231,7 +1239,10 @@ int io_read_fixed(struct io_kiocb *req, unsigned int > issue_flags) > if (unlikely(ret)) > return ret; > > - return io_read(req, issue_flags); > + ret = io_read(req, issue_flags); > + if (ret != -EIOCBQUEUED) > + io_detach_dmabuf_map(req); > + return ret; [Severity: High] When io_read() arms a page waitqueue (e.g., via io_rw_should_retry()) and returns -EIOCBQUEUED, the dmabuf map reference is deliberately not dropped here. If the request is later cancelled while on the waitqueue, does this cause a memory leak? The cleanup handler invoked on cancellation via io_clean_op() (which is io_readv_writev_cleanup() for IORING_OP_READ_FIXED) doesn't appear to clean up req->dmabuf_map. > } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13
