From nobody Mon Sep 09 00:02:16 2024 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 4X26TN2pPnz5WZTH; Mon, 09 Sep 2024 00:02:16 +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 "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4X26TN1d0Pz4qQc; Mon, 9 Sep 2024 00:02:16 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1725840136; 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=0HZYezLdql+igcBX9XuUV2FtBpLovIbzQKHU3sb5DNc=; b=KOy5J+ewtnRUTYbXFqYu6GpxPgVxh0cDzet8lHqlZTAWatzrFoK43Ph/XGE37h70ceAxk1 LTqSBmbMCGGu4MHLVX9eaMiVtQ16wdJ4iOA4FW4M0A9+QakY0zM/F/BSE9veskSjqePbWQ zO4yywkZrbgzQTR/8//jxHDDsEQTP2iIJL9mYUju3syf2Ut6eB4my1IW7nb7gmACRyQjl3 Tw8XKLmrcRvjDd3WfrpsUSEUY3Ut1GbsKRDEd7wtNxBoxvVZ1kusoW9q6SG96RqWzFNMlu CsgUp4efezL/+ZTCAJsnLthHexWEFvde+anQFVvCCbt92EhX2X1woFhrZwOAow== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1725840136; a=rsa-sha256; cv=none; b=B2u3PhpaXuNF75DGoZmWF4C8xtEWrPbhtgucQElGeYJbAk9RCVKMr3NX16fNxAqtrsAViF Z2I/rqE01Bui83PTiceMJ6XPqf1X96Ck6SZjCffl+BZeFiaOGseUdHSVhsicpZONki0EHR kzuB0uOXGanPviScpTdfNq7XqSHbyfdw+JX76ZIWWi0uWXQdc/xy8tWukmj04PL4NK5lE2 IGdDcvKIAnCUWUQGq0E5GPIY3pVP/rZh01O0NAYXBZVU+vZsMFIK/nY9zGZQqFzmNSg2ud cEwQZDU4FVMQn8t/t0wb7Atu5hWlJB0Q5BqAAPEFUPiG6iIkvCob6cknB+ygcQ== 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=1725840136; 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=0HZYezLdql+igcBX9XuUV2FtBpLovIbzQKHU3sb5DNc=; b=tPMWqgPU8avbQZ9QcoxbW74S2GLyquGRsBQ8kCr0nCJ6v7lf6szSZ02EEgYN5k7EOjnqXU lw04cpBzMsD0+W3gC+cew2tyQdcsLgHtbyCSxIXknxGHKm0jV/9iv3p0RMiPcLsupQ2T6p o6S/Ztv4Ov3M/Ov1NB8Zv0fRrH7q5wEpC1yVyKj2bHzz+bwXiS/4N8N+RvOVSPwz1ArBZH 7aMI+TYopokEXEX4xwKIO0pFu1QN/U79h0BMHItnKLZGYpALZWudQSYGeSSozwidSDEclT AWYiKDeZReaZqGCRv1JpmTa+TZ7xBzRqv1i5lOE3y3uHGspu1vZpVdhX/Sj8AQ== 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 4X26TN1DKrzgrw; Mon, 9 Sep 2024 00:02:16 +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 48902GJU006732; Mon, 9 Sep 2024 00:02:16 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 48902GZm006729; Mon, 9 Sep 2024 00:02:16 GMT (envelope-from git) Date: Mon, 9 Sep 2024 00:02:16 GMT Message-Id: <202409090002.48902GZm006729@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: "Jason A. Harmening" Subject: git: 8fa5e0f21fd1 - main - tmpfs: Account for whiteouts during rename/rmdir 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: jah X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 8fa5e0f21fd14bb3f5d977ae9130dae3e197f2ba Auto-Submitted: auto-generated The branch main has been updated by jah: URL: https://cgit.FreeBSD.org/src/commit/?id=8fa5e0f21fd14bb3f5d977ae9130dae3e197f2ba commit 8fa5e0f21fd14bb3f5d977ae9130dae3e197f2ba Author: Jason A. Harmening AuthorDate: 2024-08-06 04:30:30 +0000 Commit: Jason A. Harmening CommitDate: 2024-09-08 23:34:15 +0000 tmpfs: Account for whiteouts during rename/rmdir The existing tmpfs implementation will return ENOTEMPTY for VOP_RMDIR, or for the destination directory of VOP_RENAME, for any case in which the directory is non-empty, even if the directory only contains whiteouts. Fix this by tracking total whiteout dirent allocation separately for each directory, and avoid returning ENOTEMPTY if IGNOREWHITEOUT has been specified by the caller and the total allocation of dirents is not greater than the total whiteout allocation. This addresses "directory not empty" failures seen on some recently-added unionfs stress2 tests which use tmpfs as a base-layer filesystem. A separate issue for independent consideration is that unionfs' default behavior when deleting files or directories is to create whiteouts even when it does not truly need to do so. Differential Revision: https://reviews.freebsd.org/D45987 Reviewed by: kib (prior version), olce Tested by: pho --- sys/fs/tmpfs/tmpfs.h | 12 +++++++++++ sys/fs/tmpfs/tmpfs_subr.c | 36 ++++++++++++++++++++++++++++++- sys/fs/tmpfs/tmpfs_vnops.c | 54 +++++++++++++++++++++++++++++++++++----------- 3 files changed, 88 insertions(+), 14 deletions(-) diff --git a/sys/fs/tmpfs/tmpfs.h b/sys/fs/tmpfs/tmpfs.h index c28f3a02a7bf..58f8720c3f84 100644 --- a/sys/fs/tmpfs/tmpfs.h +++ b/sys/fs/tmpfs/tmpfs.h @@ -292,6 +292,15 @@ struct tmpfs_node { */ off_t tn_readdir_lastn; struct tmpfs_dirent * tn_readdir_lastp; + + /* + * Total size of whiteout directory entries. This + * must be a multiple of sizeof(struct tmpfs_dirent) + * and is used to determine whether a directory is + * empty (excluding whiteout entries) during rename/ + * rmdir operations. + */ + off_t tn_wht_size; /* (v) */ } tn_dir; /* Valid when tn_type == VLNK. */ @@ -484,6 +493,7 @@ int tmpfs_dir_getdents(struct tmpfs_mount *, struct tmpfs_node *, struct uio *, int, uint64_t *, int *); int tmpfs_dir_whiteout_add(struct vnode *, struct componentname *); void tmpfs_dir_whiteout_remove(struct vnode *, struct componentname *); +void tmpfs_dir_clear_whiteouts(struct vnode *); int tmpfs_reg_resize(struct vnode *, off_t, boolean_t); int tmpfs_reg_punch_hole(struct vnode *vp, off_t *, off_t *); int tmpfs_chflags(struct vnode *, u_long, struct ucred *, struct thread *); @@ -533,6 +543,8 @@ tmpfs_update(struct vnode *vp) #define TMPFS_VALIDATE_DIR(node) do { \ MPASS((node)->tn_type == VDIR); \ MPASS((node)->tn_size % sizeof(struct tmpfs_dirent) == 0); \ + MPASS((node)->tn_dir.tn_wht_size % sizeof(struct tmpfs_dirent) == 0); \ + MPASS((node)->tn_dir.tn_wht_size <= (node)->tn_size); \ } while (0) /* diff --git a/sys/fs/tmpfs/tmpfs_subr.c b/sys/fs/tmpfs/tmpfs_subr.c index a5eb865f2996..41d1f27caf13 100644 --- a/sys/fs/tmpfs/tmpfs_subr.c +++ b/sys/fs/tmpfs/tmpfs_subr.c @@ -646,6 +646,7 @@ tmpfs_alloc_node(struct mount *mp, struct tmpfs_mount *tmp, __enum_uint8(vtype) nnode->tn_dir.tn_parent = (parent == NULL) ? nnode : parent; nnode->tn_dir.tn_readdir_lastn = 0; nnode->tn_dir.tn_readdir_lastp = NULL; + nnode->tn_dir.tn_wht_size = 0; nnode->tn_links++; TMPFS_NODE_LOCK(nnode->tn_dir.tn_parent); nnode->tn_dir.tn_parent->tn_links++; @@ -1831,13 +1832,16 @@ int tmpfs_dir_whiteout_add(struct vnode *dvp, struct componentname *cnp) { struct tmpfs_dirent *de; + struct tmpfs_node *dnode; int error; error = tmpfs_alloc_dirent(VFS_TO_TMPFS(dvp->v_mount), NULL, cnp->cn_nameptr, cnp->cn_namelen, &de); if (error != 0) return (error); + dnode = VP_TO_TMPFS_DIR(dvp); tmpfs_dir_attach(dvp, de); + dnode->tn_dir.tn_wht_size += sizeof(*de); return (0); } @@ -1845,13 +1849,43 @@ void tmpfs_dir_whiteout_remove(struct vnode *dvp, struct componentname *cnp) { struct tmpfs_dirent *de; + struct tmpfs_node *dnode; - de = tmpfs_dir_lookup(VP_TO_TMPFS_DIR(dvp), NULL, cnp); + dnode = VP_TO_TMPFS_DIR(dvp); + de = tmpfs_dir_lookup(dnode, NULL, cnp); MPASS(de != NULL && de->td_node == NULL); + MPASS(dnode->tn_dir.tn_wht_size >= sizeof(*de)); + dnode->tn_dir.tn_wht_size -= sizeof(*de); tmpfs_dir_detach(dvp, de); tmpfs_free_dirent(VFS_TO_TMPFS(dvp->v_mount), de); } +/* + * Frees any dirents still associated with the directory represented + * by dvp in preparation for the removal of the directory. This is + * required when removing a directory which contains only whiteout + * entries. + */ +void +tmpfs_dir_clear_whiteouts(struct vnode *dvp) +{ + struct tmpfs_dir_cursor dc; + struct tmpfs_dirent *de; + struct tmpfs_node *dnode; + + dnode = VP_TO_TMPFS_DIR(dvp); + + while ((de = tmpfs_dir_first(dnode, &dc)) != NULL) { + KASSERT(de->td_node == NULL, ("%s: non-whiteout dirent %p", + __func__, de)); + dnode->tn_dir.tn_wht_size -= sizeof(*de); + tmpfs_dir_detach(dvp, de); + tmpfs_free_dirent(VFS_TO_TMPFS(dvp->v_mount), de); + } + MPASS(dnode->tn_size == 0); + MPASS(dnode->tn_dir.tn_wht_size == 0); +} + /* * Resizes the aobj associated with the regular file pointed to by 'vp' to the * size 'newsize'. 'vp' must point to a vnode that represents a regular file. diff --git a/sys/fs/tmpfs/tmpfs_vnops.c b/sys/fs/tmpfs/tmpfs_vnops.c index 718cfef6bfa3..b278c3153863 100644 --- a/sys/fs/tmpfs/tmpfs_vnops.c +++ b/sys/fs/tmpfs/tmpfs_vnops.c @@ -1078,7 +1078,9 @@ tmpfs_rename(struct vop_rename_args *v) } if (fnode->tn_type == VDIR && tnode->tn_type == VDIR) { - if (tnode->tn_size > 0) { + if (tnode->tn_size != 0 && + ((tcnp->cn_flags & IGNOREWHITEOUT) == 0 || + tnode->tn_size > tnode->tn_dir.tn_wht_size)) { error = ENOTEMPTY; goto out_locked; } @@ -1239,6 +1241,16 @@ tmpfs_rename(struct vop_rename_args *v) tde = tmpfs_dir_lookup(tdnode, tnode, tcnp); tmpfs_dir_detach(tdvp, tde); + /* + * If we are overwriting a directory, per the ENOTEMPTY check + * above it must either be empty or contain only whiteout + * entries. In the latter case (which can only happen if + * IGNOREWHITEOUT was passed in tcnp->cn_flags), clear the + * whiteout entries to avoid leaking memory. + */ + if (tnode->tn_type == VDIR && tnode->tn_size > 0) + tmpfs_dir_clear_whiteouts(tvp); + /* Update node's ctime because of possible hardlinks. */ tnode->tn_status |= TMPFS_NODE_CHANGED; tmpfs_update(tvp); @@ -1309,6 +1321,7 @@ tmpfs_rmdir(struct vop_rmdir_args *v) { struct vnode *dvp = v->a_dvp; struct vnode *vp = v->a_vp; + struct componentname *cnp = v->a_cnp; int error; struct tmpfs_dirent *de; @@ -1320,12 +1333,16 @@ tmpfs_rmdir(struct vop_rmdir_args *v) dnode = VP_TO_TMPFS_DIR(dvp); node = VP_TO_TMPFS_DIR(vp); - /* Directories with more than two entries ('.' and '..') cannot be - * removed. */ - if (node->tn_size > 0) { - error = ENOTEMPTY; - goto out; - } + /* + * Directories with more than two non-whiteout entries ('.' and '..') + * cannot be removed. + */ + if (node->tn_size != 0 && + ((cnp->cn_flags & IGNOREWHITEOUT) == 0 || + node->tn_size > node->tn_dir.tn_wht_size)) { + error = ENOTEMPTY; + goto out; + } if ((dnode->tn_flags & APPEND) || (node->tn_flags & (NOUNLINK | IMMUTABLE | APPEND))) { @@ -1334,15 +1351,15 @@ tmpfs_rmdir(struct vop_rmdir_args *v) } /* This invariant holds only if we are not trying to remove "..". - * We checked for that above so this is safe now. */ + * We checked for that above so this is safe now. */ MPASS(node->tn_dir.tn_parent == dnode); /* Get the directory entry associated with node (vp). This was * filled by tmpfs_lookup while looking up the entry. */ - de = tmpfs_dir_lookup(dnode, node, v->a_cnp); + de = tmpfs_dir_lookup(dnode, node, cnp); MPASS(TMPFS_DIRENT_MATCHES(de, - v->a_cnp->cn_nameptr, - v->a_cnp->cn_namelen)); + cnp->cn_nameptr, + cnp->cn_namelen)); /* Check flags to see if we are allowed to remove the directory. */ if ((dnode->tn_flags & APPEND) != 0 || @@ -1353,8 +1370,19 @@ tmpfs_rmdir(struct vop_rmdir_args *v) /* Detach the directory entry from the directory (dnode). */ tmpfs_dir_detach(dvp, de); - if (v->a_cnp->cn_flags & DOWHITEOUT) - tmpfs_dir_whiteout_add(dvp, v->a_cnp); + + /* + * If we are removing a directory, per the ENOTEMPTY check above it + * must either be empty or contain only whiteout entries. In the + * latter case (which can only happen if IGNOREWHITEOUT was passed + * in cnp->cn_flags), clear the whiteout entries to avoid leaking + * memory. + */ + if (node->tn_size > 0) + tmpfs_dir_clear_whiteouts(vp); + + if (cnp->cn_flags & DOWHITEOUT) + tmpfs_dir_whiteout_add(dvp, cnp); /* No vnode should be allocated for this entry from this point */ TMPFS_NODE_LOCK(node);