ACK: [SRU][F][PATCH v3 0/3] CVE-2023-21400

[Philip Cox]

On 2025-01-05 11:15 p.m., Chengen Du wrote:
> CVE-2023-21400
>
> BugLink:https://bugs.launchpad.net/bugs/2078659
>
> SRU Justification:
>
> [Impact]
> io_commit_cqring() writes the CQ ring tail to make it visible and also triggers any deferred work.
> When a ring is set up with IOPOLL, it doesn't require locking around the CQ ring updates.
> However, if there is deferred work that needs processing, io_queue_deferred() assumes that the completion_lock is held.
> The io_uring subsystem does not properly handle locking for rings with IOPOLL, leading to a double-free vulnerability, which can be exploited as CVE-2023-21400.
>
> [Fix]
> There is a commit that fixed this issue.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fb348857e7b67eefe365052f1423427b66dedbf3
>
> There is no direct upstream commit addressing this issue, so the patch needs to be reworked to apply to version 5.4.
>
> The patch introduces the io_commit_needs_flush function to determine if flushing the cqring while holding ctx->completion_lock is necessary.
> However, implementing this logic in Focal requires backporting commits that introduce missing io_ring_ctx struct member variables.
> This approach is not ideal, as those commits are unrelated to the CVE.
> Additionally, introducing new member variables raises concerns, particularly since Focal is nearing the end of its standard support period.
> To address this tradeoff, the SUSE commit [1] serves as a suitable reference.
>
> To address this issue in the simplest way, we can wrap the ctx->completion_lock spinlock around io_commit_cqring in the io_iopoll_complete function.
> However, this approach has surfaced an existing issue related to invoking kill_fasync under completion_lock as highlighted in commit deadlock (4aa84f2ffa81 io_uring: don't kill fasync under completion_lock).
> The following patches can help us align the code more closely with the upstream flow while also addressing the deadlock issue.
> 0791015837f1 io_uring: remove extra check in __io_commit_cqring
> 4aa84f2ffa81 io_uring: dont kill fasync under completion_lock
>
> After backporting these two commits, we can adopt SUSE's approach to align the code more closely with the Linux stable branch.
>
> [Test Plan]
> This is a timing issue that can be triggered by using the liburing library to implement a test program.
> First, set the io_uring_params flag to IORING_SETUP_IOPOLL and open an XFS file with the O_RDWR | O_DIRECT flags, as the XFS filesystem implements the iopoll function hook.
> After setting up io_uring, create two threads in the process: one thread will wait for completion queue events, and the other will continuously send readv and writev requests in sequence.
> The writev requests should include the IOSQE_IO_DRAIN flag to ensure that previous submission queue events are completed first.
>
> The issue arises when writev requests add entries into the defer_list, but the io_iopoll_complete function consumes entries from defer_list without holding the appropriate lock.
>
> [Where problems could occur]
> The problematic call path can be triggered under specific usage scenarios and only affects io_uring functionality.
> If the patch contains any issues, it may lead to a deadlock.
>
> [1]https://github.com/SUSE/kernel/commit/a16c5d2daa1cf45f8694ff72bd2665d474a763a7
>
> V2 -> V3: Revise the commit comments and cover letter to clearly outline the concerns and adjustments.
> V1 -> V2: Backport the other two commits to align the code more closely with the upstream flow and resolve the deadlock issue / Ignore the io_commit_needs_flush check due to the need to backport patches that introduce missing io_ring_ctx struct member variables.
>
> Jens Axboe (1):
>    io_uring: ensure IOPOLL locks around deferred work
>
> Pavel Begunkov (2):
>    io_uring: remove extra check in __io_commit_cqring
>    io_uring: dont kill fasync under completion_lock
>
>   fs/io_uring.c | 41 ++++++++++++++++++++++++++++-------------
>   1 file changed, 28 insertions(+), 13 deletions(-)

Acked-by: Philip Cox <philip.cox at canonical.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20250106/4067c38c/attachment.html>