| Message ID | 20240927063755.112103-2-chengen.du@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | CVE-2023-21400 | expand |
On Fri, 27 Sep 2024 14:37:51 +0800 Chengen Du <chengen.du@canonical.com> wrote: > From: Jens Axboe <axboe@kernel.dk> > > CVE-2023-21400 > > BugLink: https://bugs.launchpad.net/bugs/2078659 > > No direct upstream commit exists for this issue. It was fixed in > 5.18 as part of a larger rework of the completion side. > > io_commit_cqring() writes the CQ ring tail to make it visible, but it > also kicks off any deferred work we have. A ring setup with IOPOLL > does not need any locking around the CQ ring updates, as we're always > under the ctx uring_lock. But if we have deferred work that needs > processing, then io_queue_deferred() assumes that the completion_lock > is held, as it is for !IOPOLL. > > Add a lockdep assertion to check and document this fact, and have > io_iopoll_complete() check if we have deferred work and run that > separately with the appropriate lock grabbed. > > Cc: stable@vger.kernel.org # 5.10, 5.15 > Reported-by: dghost david <daviduniverse18@gmail.com> > Signed-off-by: Jens Axboe <axboe@kernel.dk> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > (backported from commit fb348857e7b67eefe365052f1423427b66dedbf3) > [chengen - apply the locking logic to the corresponding functions / > use spin_lock_irq because the same lock is used in the timer callback > function.] > Signed-off-by: Chengen Du <chengen.du@canonical.com> > --- > fs/io_uring.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/io_uring.c b/fs/io_uring.c > index 875dd8e0f766..aa75c32f2d16 100644 > --- a/fs/io_uring.c > +++ b/fs/io_uring.c > @@ -512,6 +512,8 @@ static void __io_commit_cqring(struct io_ring_ctx *ctx) > { > struct io_rings *rings = ctx->rings; > > + lockdep_assert_held(&ctx->completion_lock); > + > if (ctx->cached_cq_tail != READ_ONCE(rings->cq.tail)) { > /* order cqe stores with ring update */ > smp_store_release(&rings->cq.tail, ctx->cached_cq_tail); > @@ -841,7 +843,9 @@ static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events, > } > } > > + spin_lock_irq(&ctx->completion_lock); > io_commit_cqring(ctx); > + spin_unlock_irq(&ctx->completion_lock); > io_free_req_many(ctx, reqs, &to_free); > } > Not sure if that is sufficient. Your patch is (obviously) vastly different from the original. You might want to check SUSE fix for 5.3: https://github.com/SUSE/kernel/commit/a16c5d2daa1cf45f8694ff72bd2665d474a763a7 Do you have a reproducer to actually validate the fix? ...Juerg
diff --git a/fs/io_uring.c b/fs/io_uring.c index 875dd8e0f766..aa75c32f2d16 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -512,6 +512,8 @@ static void __io_commit_cqring(struct io_ring_ctx *ctx) { struct io_rings *rings = ctx->rings; + lockdep_assert_held(&ctx->completion_lock); + if (ctx->cached_cq_tail != READ_ONCE(rings->cq.tail)) { /* order cqe stores with ring update */ smp_store_release(&rings->cq.tail, ctx->cached_cq_tail); @@ -841,7 +843,9 @@ static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events, } } + spin_lock_irq(&ctx->completion_lock); io_commit_cqring(ctx); + spin_unlock_irq(&ctx->completion_lock); io_free_req_many(ctx, reqs, &to_free); }