From nobody Thu Aug 14 13:47:37 2025 X-Original-To: dev-commits-src-all@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 4c2mlG3CWdz64TZW; Thu, 14 Aug 2025 13:47:38 +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 4c2mlG2DWDz4Jfd; Thu, 14 Aug 2025 13:47:38 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1755179258; 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=UaXBzrPnXjze7gAjXHrzqGMnEPN2OmKnBWUop58AdDc=; b=nVb2sdkWL5kFpqaH7naSbOBJpmKM2J7TrxqVrrv9luE8S5wZjyiJ0oUNxQEFqBMXqYAzQJ NLrmCL7aeKGxdS7DjZm0p54Df2Sdj4qFo4agqbnHYmlQVH6ZqQcqzLlXh9QIZOgbQR2Ys6 JJARFvGhQGppZVPYZddb5v2/o1ZCvu6P0RVBtZP9T7P9PAtAFbsIMTJfHFMI7i922FfRw3 uEU8Z+Yd/Rq00u4DUbQ/o0m0cRglm80RwW1qcevmwawBn0eCoMKwApAKoBWTyhwtzD/jO1 ibfo784d8hQ151ZNvIImEnIY+OwkTxwxCKlLvWu2dnRAQmedoyVyEZt9ckzJxg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1755179258; 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=UaXBzrPnXjze7gAjXHrzqGMnEPN2OmKnBWUop58AdDc=; b=Syw1nnsOn2TEGJF+h06oSrlR3/7lvmJfTTCy7v0Ht+4XTp6jDHVaKHgOw7D5okftlean0k N6gGlVoD9Jf/IHCi8JT4KD0QoGJIBEe8T5k2f6rD07/WtYhmveWwBoIyhokoq5IG1TS9Vb yW654UR84YDx1+/wR5Z7smEOzo9Bh0Vhl05y5HImsCOFoZx+NFI5+MJ5SzvwsjRDUxk8t2 cEv/H957hC3M4NfbD7rHsi5LiZZxBTY5xcG6SPSlIPgeQ/OlD/2gYPBmp8QYATuz36cuBA eEwwWgQwPfMt6/D1omzuDwUnZaRa/oAWkEGBs7ThwknZOmnvRgXjcb2MFx4GYw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1755179258; a=rsa-sha256; cv=none; b=y0VOdsmhBXAqvzS1GRAJ2RIIo5ieXm7AqK9XtMdyg+OqIffCJai3HfTDF5xuDqIQLtctsf QRZ7+z0A7pBu/OkXfJFcjINuIrYZ6hsbecA+s70WHAgwnbbBPsXs13J3sgVXm2Rr3CRsVn QifLhoh0E675YgGf+s1NjAvsXuyq7iPC6YYGVxXtEkWXK2WZlX+bysHUQzmkfe+jAtX3I6 XmAxAiFrpKNKi2T5S9bUVU/4Nxm/OfAvFpOSCFi+iGroG10hPK0mAbVgehJ736ojRQfy98 Ff607oiCdSMyVbN5PYCOPMTc8LK2ZXRlgOqDekrEqZOQGlYP+I9ksu/NhXkYhg== 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 4c2mlG19BGzvhv; Thu, 14 Aug 2025 13:47:38 +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 57EDlb6P017078; Thu, 14 Aug 2025 13:47:37 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 57EDlbQF017075; Thu, 14 Aug 2025 13:47:37 GMT (envelope-from git) Date: Thu, 14 Aug 2025 13:47:37 GMT Message-Id: <202508141347.57EDlbQF017075@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: e71d494c600e - stable/14 - aio: Fix a race in sys_aio_cancel() List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@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/stable/14 X-Git-Reftype: branch X-Git-Commit: e71d494c600e7cd6433f498a2ec9929ef98879fc Auto-Submitted: auto-generated The branch stable/14 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=e71d494c600e7cd6433f498a2ec9929ef98879fc commit e71d494c600e7cd6433f498a2ec9929ef98879fc Author: Mark Johnston AuthorDate: 2025-07-29 14:46:53 +0000 Commit: Mark Johnston CommitDate: 2025-08-14 12:45:59 +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 (cherry picked from commit 2dde3bed347ab46685d079c2fcc1dbd1fa149286) --- 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 a14865f6b20c..8842d8409e01 100644 --- a/sys/kern/vfs_aio.c +++ b/sys/kern/vfs_aio.c @@ -229,6 +229,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 @@ -593,6 +594,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); @@ -667,7 +674,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; @@ -1832,6 +1839,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) { @@ -2061,7 +2070,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; @@ -2086,16 +2095,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; }