Skip to content

Commit 561e4f9

Browse files
committed
io_uring/kbuf: hold io_buffer_list reference over mmap
If we look up the kbuf, ensure that it doesn't get unregistered until after we're done with it. Since we're inside mmap, we cannot safely use the io_uring lock. Rely on the fact that we can lookup the buffer list under RCU now and grab a reference to it, preventing it from being unregistered until we're done with it. The lookup returns the io_buffer_list directly with it referenced. Cc: [email protected] # v6.4+ Fixes: 5cf4f52 ("io_uring: free io_buffer_list entries via RCU") Signed-off-by: Jens Axboe <[email protected]>
1 parent 6b69c4a commit 561e4f9

File tree

3 files changed

+36
-14
lines changed

3 files changed

+36
-14
lines changed

io_uring/io_uring.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3447,14 +3447,15 @@ static void *io_uring_validate_mmap_request(struct file *file,
34473447
ptr = ctx->sq_sqes;
34483448
break;
34493449
case IORING_OFF_PBUF_RING: {
3450+
struct io_buffer_list *bl;
34503451
unsigned int bgid;
34513452

34523453
bgid = (offset & ~IORING_OFF_MMAP_MASK) >> IORING_OFF_PBUF_SHIFT;
3453-
rcu_read_lock();
3454-
ptr = io_pbuf_get_address(ctx, bgid);
3455-
rcu_read_unlock();
3456-
if (!ptr)
3457-
return ERR_PTR(-EINVAL);
3454+
bl = io_pbuf_get_bl(ctx, bgid);
3455+
if (IS_ERR(bl))
3456+
return bl;
3457+
ptr = bl->buf_ring;
3458+
io_put_bl(ctx, bl);
34583459
break;
34593460
}
34603461
default:

io_uring/kbuf.c

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ static int __io_remove_buffers(struct io_ring_ctx *ctx,
266266
return i;
267267
}
268268

269-
static void io_put_bl(struct io_ring_ctx *ctx, struct io_buffer_list *bl)
269+
void io_put_bl(struct io_ring_ctx *ctx, struct io_buffer_list *bl)
270270
{
271271
if (atomic_dec_and_test(&bl->refs)) {
272272
__io_remove_buffers(ctx, bl, -1U);
@@ -719,16 +719,35 @@ int io_register_pbuf_status(struct io_ring_ctx *ctx, void __user *arg)
719719
return 0;
720720
}
721721

722-
void *io_pbuf_get_address(struct io_ring_ctx *ctx, unsigned long bgid)
722+
struct io_buffer_list *io_pbuf_get_bl(struct io_ring_ctx *ctx,
723+
unsigned long bgid)
723724
{
724725
struct io_buffer_list *bl;
726+
bool ret;
725727

726-
bl = __io_buffer_get_list(ctx, bgid);
727-
728-
if (!bl || !bl->is_mmap)
729-
return NULL;
730-
731-
return bl->buf_ring;
728+
/*
729+
* We have to be a bit careful here - we're inside mmap and cannot grab
730+
* the uring_lock. This means the buffer_list could be simultaneously
731+
* going away, if someone is trying to be sneaky. Look it up under rcu
732+
* so we know it's not going away, and attempt to grab a reference to
733+
* it. If the ref is already zero, then fail the mapping. If successful,
734+
* the caller will call io_put_bl() to drop the the reference at at the
735+
* end. This may then safely free the buffer_list (and drop the pages)
736+
* at that point, vm_insert_pages() would've already grabbed the
737+
* necessary vma references.
738+
*/
739+
rcu_read_lock();
740+
bl = xa_load(&ctx->io_bl_xa, bgid);
741+
/* must be a mmap'able buffer ring and have pages */
742+
ret = false;
743+
if (bl && bl->is_mmap)
744+
ret = atomic_inc_not_zero(&bl->refs);
745+
rcu_read_unlock();
746+
747+
if (ret)
748+
return bl;
749+
750+
return ERR_PTR(-EINVAL);
732751
}
733752

734753
/*

io_uring/kbuf.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ void __io_put_kbuf(struct io_kiocb *req, unsigned issue_flags);
6161

6262
bool io_kbuf_recycle_legacy(struct io_kiocb *req, unsigned issue_flags);
6363

64-
void *io_pbuf_get_address(struct io_ring_ctx *ctx, unsigned long bgid);
64+
void io_put_bl(struct io_ring_ctx *ctx, struct io_buffer_list *bl);
65+
struct io_buffer_list *io_pbuf_get_bl(struct io_ring_ctx *ctx,
66+
unsigned long bgid);
6567

6668
static inline bool io_kbuf_recycle_ring(struct io_kiocb *req)
6769
{

0 commit comments

Comments
 (0)