From nobody Sat Apr 12 15:35:41 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 4ZZd1B147Kz5s9q5; Sat, 12 Apr 2025 15:35:42 +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 4ZZd1B092xz3r7L; Sat, 12 Apr 2025 15:35:42 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1744472142; 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=gotr1f5Nu0rpsELWNnvgBQhiOOx0SXYaipFc5cTulUM=; b=dYocSX8yasEPu97KnDaEtbeO6Kksi+ij5HU155QFIIubOuGD5DdtTCDGoAYk7G9mhHTC4m YuHTpY8qrnll8oBrWHNMaH7QbOVoJ68Oj7jvv2fJufMx7LnZPzPtjbl7qMGmFmTX8c0itG NQf0c1xxxYoVYgfIgL4fVHw87e8OVZmxDYWlAUnhn0YxleboT2YtfRUdZOA+5V+F5EWxjv 8+UZR4epui99YomJN/X8iR+ccvEidM4/Y0Em4iV/4vHBedMiOpzIpNjhbOHWA4mrtFKH3P N/v8C53mBQPZyZvQ9Js8u9cri/bLp++yxQL4QyQawSwIArH/r/l8ppXFZcsOyw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1744472142; a=rsa-sha256; cv=none; b=IMQGAdivtgJ61c9YDth5G8LMJXhpIXyKKWYwqS0cylYUNAyqKuszIY72Y8/7IQCEqSpqXs 8iaDyQ2CLTYaC2FpoUMWxl76Mz9iDu7dWS7ETHTzDae4u1an1Vvpv8r56Kau5M1V6+QTUQ wujYHmKscQJyd2damHt8AicPxjaZ+AXuRGPXHxCm+RPWzyu3epV8Scx6u845Rte3FPzCAi TIsxsaxOX4ruDwLyerOSaEP/TrXS2wais25Nf+cvGssk0xHft3mA28DVg8lvqJVw1Fvvuv fxy+6oaWoVPzNEeLbkYYPiIXiZPZ8ucqYKhPpbB/5fa6UHz5z6SZxdNdCBiiOA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1744472142; 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=gotr1f5Nu0rpsELWNnvgBQhiOOx0SXYaipFc5cTulUM=; b=PKJBb7HQuO2Kq1NQNijXmgqBlKaXgVIot4W7ZQTgYdUSuAMC9Wb8Bo6D/IcsDlRQ6cMCMQ N/IB+lZ3tHcIHVLAwXTngYlTs6cxM3PtI1AiJusTe2oi3NjFxQ3fLt0gPAhvhd5OxvN71M uVp2tCFcM2W/+u7kX4jm75k+Si6imrijVkRZPqXaXzXJrXjPlZHXIuVaYtYkpVy9uh9747 IktdDSJNPDXW31k+iuOlzD2IGiVbXe2EGKi9eFAu2HyDoijNfZBKPmg5O+vkfJw6zFCfBo 65mqQm6NAwDlvA1CgX9/s4YfpqVK4bs3XPj03grMwWa6TIvaekmVleSp1jyl7Q== 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 4ZZd196t67z7Zk; Sat, 12 Apr 2025 15:35:41 +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 53CFZfAL012795; Sat, 12 Apr 2025 15:35:41 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 53CFZfkW012792; Sat, 12 Apr 2025 15:35:41 GMT (envelope-from git) Date: Sat, 12 Apr 2025 15:35:41 GMT Message-Id: <202504121535.53CFZfkW012792@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: 509189bb4109 - main - fhopen: Enable handling of O_PATH, fix some bugs 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: 509189bb41099407169ea57e1a5a8d396d1418f7 Auto-Submitted: auto-generated The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=509189bb41099407169ea57e1a5a8d396d1418f7 commit 509189bb41099407169ea57e1a5a8d396d1418f7 Author: Mark Johnston AuthorDate: 2025-04-12 15:29:19 +0000 Commit: Mark Johnston CommitDate: 2025-04-12 15:35:15 +0000 fhopen: Enable handling of O_PATH, fix some bugs - kern_fhopen() permitted O_PATH but didn't clear O_ACCMODE flags when it is specified, leading to inconsistencies. For instance, opening a writeable O_PATH handle would not bump the writecount on the underlying vnode, but closing it would decrement the writecount. - kern_fhopen() didn't handle the possibility that VOP_OPEN could install a fileops table. This is how named pipes are implemented, for instance. Some devfs nodes do this as well, but devfs doesn't implement VFS_FHTOVP. Factor out some code from openatfp() and use it in kern_fhopen() to address these bugs. Reported by: syzbot+517a2c879eede02c03fb@syzkaller.appspotmail.com Reviewed by: kib MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D49717 --- sys/kern/vfs_syscalls.c | 128 +++++++++++++++++++++++++++--------------------- sys/kern/vfs_vnops.c | 3 ++ 2 files changed, 76 insertions(+), 55 deletions(-) diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c index 7b71ffc76892..5fe2ae2e23a6 100644 --- a/sys/kern/vfs_syscalls.c +++ b/sys/kern/vfs_syscalls.c @@ -1150,6 +1150,61 @@ sys_openat(struct thread *td, struct openat_args *uap) uap->mode)); } +/* + * Validate open(2) flags and convert access mode flags (O_RDONLY etc.) to their + * in-kernel representations (FREAD etc.). + */ +static int +openflags(int *flagsp) +{ + int flags; + + /* + * Only one of the O_EXEC, O_RDONLY, O_WRONLY and O_RDWR flags + * may be specified. On the other hand, for O_PATH any mode + * except O_EXEC is ignored. + */ + flags = *flagsp; + if ((flags & O_PATH) != 0) { + flags &= ~O_ACCMODE; + } else if ((flags & O_EXEC) != 0) { + if ((flags & O_ACCMODE) != 0) + return (EINVAL); + } else if ((flags & O_ACCMODE) == O_ACCMODE) { + return (EINVAL); + } else { + flags = FFLAGS(flags); + } + *flagsp = flags; + return (0); +} + +static void +finit_open(struct file *fp, struct vnode *vp, int flags) +{ + /* + * Store the vnode, for any f_type. Typically, the vnode use count is + * decremented by a direct call to vnops.fo_close() for files that + * switched type. + */ + fp->f_vnode = vp; + + /* + * If the file wasn't claimed by devfs or fifofs, bind it to the normal + * vnode operations here. + */ + if (fp->f_ops == &badfileops) { + KASSERT(vp->v_type != VFIFO || (flags & O_PATH) != 0, + ("Unexpected fifo fp %p vp %p", fp, vp)); + if ((flags & O_PATH) != 0) { + finit(fp, (flags & FMASK) | (fp->f_flag & FKQALLOWED), + DTYPE_VNODE, NULL, &path_fileops); + } else { + finit_vnode(fp, flags, NULL, &vnops); + } + } +} + /* * If fpp != NULL, opened file is not installed into the file * descriptor table, instead it is returned in *fpp. This is @@ -1179,21 +1234,9 @@ openatfp(struct thread *td, int dirfd, const char *path, cap_rights_init_one(&rights, CAP_LOOKUP); flags_to_rights(flags, &rights); - /* - * Only one of the O_EXEC, O_RDONLY, O_WRONLY and O_RDWR flags - * may be specified. On the other hand, for O_PATH any mode - * except O_EXEC is ignored. - */ - if ((flags & O_PATH) != 0) { - flags &= ~O_ACCMODE; - } else if ((flags & O_EXEC) != 0) { - if (flags & O_ACCMODE) - return (EINVAL); - } else if ((flags & O_ACCMODE) == O_ACCMODE) { - return (EINVAL); - } else { - flags = FFLAGS(flags); - } + error = openflags(&flags); + if (error != 0) + return (error); /* * Allocate a file structure. The descriptor to reference it @@ -1244,28 +1287,7 @@ openatfp(struct thread *td, int dirfd, const char *path, NDFREE_PNBUF(&nd); vp = nd.ni_vp; - /* - * Store the vnode, for any f_type. Typically, the vnode use - * count is decremented by direct call to vn_closefile() for - * files that switched type in the cdevsw fdopen() method. - */ - fp->f_vnode = vp; - - /* - * If the file wasn't claimed by devfs bind it to the normal - * vnode operations here. - */ - if (fp->f_ops == &badfileops) { - KASSERT(vp->v_type != VFIFO || (flags & O_PATH) != 0, - ("Unexpected fifo fp %p vp %p", fp, vp)); - if ((flags & O_PATH) != 0) { - finit(fp, (flags & FMASK) | (fp->f_flag & FKQALLOWED), - DTYPE_VNODE, NULL, &path_fileops); - } else { - finit_vnode(fp, flags, NULL, &vnops); - } - } - + finit_open(fp, vp, flags); VOP_UNLOCK(vp); if (flags & O_TRUNC) { error = fo_truncate(fp, 0, td->td_ucred, td); @@ -4653,21 +4675,20 @@ kern_fhopen(struct thread *td, const struct fhandle *u_fhp, int flags) struct vnode *vp; struct fhandle fhp; struct file *fp; - int fmode, error; - int indx; + int error, indx; bool named_attr; error = priv_check(td, PRIV_VFS_FHOPEN); if (error != 0) return (error); + indx = -1; - fmode = FFLAGS(flags); - /* why not allow a non-read/write open for our lockd? */ - if (((fmode & (FREAD | FWRITE)) == 0) || (fmode & O_CREAT)) - return (EINVAL); + error = openflags(&flags); + if (error != 0) + return (error); error = copyin(u_fhp, &fhp, sizeof(fhp)); if (error != 0) - return(error); + return (error); /* find the mount point */ mp = vfs_busyfs(&fhp.fh_fsid); if (mp == NULL) @@ -4685,8 +4706,8 @@ kern_fhopen(struct thread *td, const struct fhandle *u_fhp, int flags) */ named_attr = (vn_irflag_read(vp) & (VIRF_NAMEDDIR | VIRF_NAMEDATTR)) != 0; - if ((named_attr && (fmode & O_NAMEDATTR) == 0) || - (!named_attr && (fmode & O_NAMEDATTR) != 0)) { + if ((named_attr && (flags & O_NAMEDATTR) == 0) || + (!named_attr && (flags & O_NAMEDATTR) != 0)) { vput(vp); return (ENOATTR); } @@ -4696,15 +4717,13 @@ kern_fhopen(struct thread *td, const struct fhandle *u_fhp, int flags) vput(vp); return (error); } - /* - * An extra reference on `fp' has been held for us by - * falloc_noinstall(). - */ + /* Set the flags early so the finit in devfs can pick them up. */ + fp->f_flag = flags & FMASK; #ifdef INVARIANTS td->td_dupfd = -1; #endif - error = vn_open_vnode(vp, fmode, td->td_ucred, td, fp); + error = vn_open_vnode(vp, flags, td->td_ucred, td, fp); if (error != 0) { KASSERT(fp->f_ops == &badfileops, ("VOP_OPEN in fhopen() set f_ops")); @@ -4717,16 +4736,15 @@ kern_fhopen(struct thread *td, const struct fhandle *u_fhp, int flags) #ifdef INVARIANTS td->td_dupfd = 0; #endif - fp->f_vnode = vp; - finit_vnode(fp, fmode, NULL, &vnops); + finit_open(fp, vp, flags); VOP_UNLOCK(vp); - if ((fmode & O_TRUNC) != 0) { + if ((flags & O_TRUNC) != 0) { error = fo_truncate(fp, 0, td->td_ucred, td); if (error != 0) goto bad; } - error = finstall(td, fp, &indx, fmode, NULL); + error = finstall(td, fp, &indx, flags, NULL); bad: fdrop(fp, td); td->td_retval[0] = indx; diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c index c448d62e9920..4a369559111e 100644 --- a/sys/kern/vfs_vnops.c +++ b/sys/kern/vfs_vnops.c @@ -432,6 +432,9 @@ vn_open_vnode(struct vnode *vp, int fmode, struct ucred *cred, accmode_t accmode; int error; + KASSERT((fmode & O_PATH) == 0 || (fmode & O_ACCMODE) == 0, + ("%s: O_PATH and O_ACCMODE are mutually exclusive", __func__)); + if (vp->v_type == VLNK) { if ((fmode & O_PATH) == 0 || (fmode & FEXEC) != 0) return (EMLINK);