From nobody Thu Jul 31 16:53:57 2025 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4btFXj6MTXz62nw8; Thu, 31 Jul 2025 16:53:57 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4btFXj5lVWz3WhZ; Thu, 31 Jul 2025 16:53:57 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1753980837; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=lgN41iBRyX6yu263sjm9oV2O84p2CmtL4IqSybqHsYw=; b=wLWYct593+WzRJrRzMGAYVXnsbC0YkdxFojjhqB1qxsDlTQUb0ON5SDyZYS3+KZhOozNbr rS9o98GbP+Wkw+DNaaFgpj7MrU1dDGHl6dJXTeb3arvCuPmqnetFo41msh7sFGrg06lfla 0HonRfgTSEiQ/x+m6o9RZpHw62/tSRi5BCj9QVmBqoh5Klhizo2iOQR9FVDCC1Q018zS08 dUzbryXP1gUKnaH02hOm9zq76WAtEtF1Tc9i3LFVreQ5ADtLQESSeiCLK59F6iEggLpG+T 36eeqUjhdPUlBtpod8MWpU6xP7f1SnBzfuKVM0ZAcmSfKakvZO/Oqg9UFPOT2w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1753980837; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=lgN41iBRyX6yu263sjm9oV2O84p2CmtL4IqSybqHsYw=; b=inkSpyslRbXmmJO3mXuemnblZG6ZV/6+GV+t5nNaNmiT62cjaTqud7RIpnZaSlJRCnMUTb Rj/S1+0dv8nYydvWuBj67m9y5CJ7nc5vJtH7q6hoEXfB1DEbn/ddP8EbVa5NF3BGzEep3h PbPle6oKxGgkxJlaWJtXqMJf/R3v6bJnKJak1DSjmfS0uxJUg/7afNcNd5h9PjcbrbDY1j ck9K4rK8lQ9/Zvh8l0lsa8jTkd1L0zL5PYKuxV4RdV+RwCLROwBk6BeTOgWYuJzUVA9fLh pEuf02noQd6vMnkStA+6ubu1IiMxLeWw41eCczccwBQZhSBNQf9v9dNtwOqRhQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1753980837; a=rsa-sha256; cv=none; b=fUS0NUUwVxycXLhQEch6ZIw5P+3Np3ijhKk5GVFgM5UgQ8exhIrxSpCsmYi1EvzioK8k3N dCVgn6g5UWxQkNhjFlNV3xGrx/YVs6n8ZoJais6OZe5SXZX3WhmHc3ICBj438j6AUphwTq hAuBUuYrYNG4Zo1Ju1GT9Ehv0MhhJLXYsKTdhVW2Fl1WM83Xfr57OaDqk4QO7QRG1+rUqs VNFdJKzum/gmoQN7ckN7H5iiNiMkwkikCVut5Z9rywEgyTTyfFztVCm2kqC8dikyCHk1eY XK2bwDC8wtMT6ICB389slR97StLrh2gluNd0eooh2IHJ6eurHV6j1IwE7k/ToQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4btFXj4VPQz193K; Thu, 31 Jul 2025 16:53:57 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 56VGrvOI037735; Thu, 31 Jul 2025 16:53:57 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 56VGrvMU037732; Thu, 31 Jul 2025 16:53:57 GMT (envelope-from git) Date: Thu, 31 Jul 2025 16:53:57 GMT Message-Id: <202507311653.56VGrvMU037732@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: 2dde3bed347a - main - aio: Fix a race in sys_aio_cancel() List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 2dde3bed347ab46685d079c2fcc1dbd1fa149286 Auto-Submitted: auto-generated The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=2dde3bed347ab46685d079c2fcc1dbd1fa149286 commit 2dde3bed347ab46685d079c2fcc1dbd1fa149286 Author: Mark Johnston AuthorDate: 2025-07-29 14:46:53 +0000 Commit: Mark Johnston CommitDate: 2025-07-31 16:51:49 +0000 aio: Fix a race in sys_aio_cancel() sys_aio_cancel() loops over pending jobs for the process, cancelling some of them. To cancel a job with a cancel callback, it must drop the job list mutex. It uses flags, KAIOCB_CANCELLING and KAIOCB_CANCELLED, to make sure that a job isn't double-cancelled. However, when iterating over the list it uses TAILQ_FOREACH_SAFE and thus assumes that the next job isn't going to be removed while the lock is dropped. Of course, this assumption is false. We could simply start search from the beginning after cancelling a job, but that might be quite expensive. Instead, introduce the notion of a marker job, used to keep track of one's position in the queue. Use it in sys_aio_cancel() to resume iteration after a job is cancelled. Reported by: syzkaller Reviewed by: kib, jhb MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D51626 --- sys/kern/vfs_aio.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/sys/kern/vfs_aio.c b/sys/kern/vfs_aio.c index 02973146068d..e63fa4c01434 100644 --- a/sys/kern/vfs_aio.c +++ b/sys/kern/vfs_aio.c @@ -222,6 +222,7 @@ typedef struct oaiocb { #define KAIOCB_CHECKSYNC 0x08 #define KAIOCB_CLEARED 0x10 #define KAIOCB_FINISHED 0x20 +#define KAIOCB_MARKER 0x40 /* ioflags */ #define KAIOCB_IO_FOFFSET 0x01 @@ -584,6 +585,12 @@ aio_cancel_job(struct proc *p, struct kaioinfo *ki, struct kaiocb *job) int cancelled; AIO_LOCK_ASSERT(ki, MA_OWNED); + + /* + * If we're running down the queue, the process must be single-threaded, + * and so no markers should be present. + */ + MPASS((job->jobflags & KAIOCB_MARKER) == 0); if (job->jobflags & (KAIOCB_CANCELLED | KAIOCB_FINISHED)) return (0); MPASS((job->jobflags & KAIOCB_CANCELLING) == 0); @@ -658,7 +665,7 @@ restart: } /* Wait for all running I/O to be finished */ - if (TAILQ_FIRST(&ki->kaio_jobqueue) || ki->kaio_active_count != 0) { + if (!TAILQ_EMPTY(&ki->kaio_jobqueue) || ki->kaio_active_count != 0) { ki->kaio_flags |= KAIO_WAKEUP; msleep(&p->p_aioinfo, AIO_MTX(ki), PRIBIO, "aioprn", hz); goto restart; @@ -1804,6 +1811,8 @@ aio_queue_file(struct file *fp, struct kaiocb *job) } else if (job->uaiocb.aio_lio_opcode & LIO_SYNC) { AIO_LOCK(ki); TAILQ_FOREACH(job2, &ki->kaio_jobqueue, plist) { + if ((job2->jobflags & KAIOCB_MARKER) != 0) + continue; if (job2->fd_file == job->fd_file && ((job2->uaiocb.aio_lio_opcode & LIO_SYNC) == 0) && job2->seqno < job->seqno) { @@ -2033,7 +2042,7 @@ sys_aio_cancel(struct thread *td, struct aio_cancel_args *uap) { struct proc *p = td->td_proc; struct kaioinfo *ki; - struct kaiocb *job, *jobn; + struct kaiocb *job, *jobn, marker; struct file *fp; int error; int cancelled = 0; @@ -2058,16 +2067,30 @@ sys_aio_cancel(struct thread *td, struct aio_cancel_args *uap) } } + /* + * We may have to drop the list mutex in order to cancel a job. After + * that point it is unsafe to rely on the stability of the list. We + * could restart the search from the beginning after canceling a job, + * but this may inefficient. Instead, use a marker job to keep our + * place in the list. + */ + memset(&marker, 0, sizeof(marker)); + marker.jobflags = KAIOCB_MARKER; + AIO_LOCK(ki); TAILQ_FOREACH_SAFE(job, &ki->kaio_jobqueue, plist, jobn) { - if ((uap->fd == job->uaiocb.aio_fildes) && - ((uap->aiocbp == NULL) || - (uap->aiocbp == job->ujob))) { + if (uap->fd == job->uaiocb.aio_fildes && + (uap->aiocbp == NULL || uap->aiocbp == job->ujob) && + (job->jobflags & KAIOCB_MARKER) == 0) { + TAILQ_INSERT_AFTER(&ki->kaio_jobqueue, job, &marker, + plist); if (aio_cancel_job(p, ki, job)) { cancelled++; } else { notcancelled++; } + jobn = TAILQ_NEXT(&marker, plist); + TAILQ_REMOVE(&ki->kaio_jobqueue, &marker, plist); if (uap->aiocbp != NULL) break; }