From nobody Wed Dec 10 20:10:48 2025 Received: from flow-b5-smtp.messagingengine.com (flow-b5-smtp.messagingengine.com [202.12.124.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id ED78E1A9FBC; Thu, 13 Nov 2025 00:42:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.140 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762994549; cv=none; b=AJqhPCa/Ww2uyqPGeN1Dc8cCWOAXNjXJldL5TxX/G3VNyMF+5TD+Xte1zsJkxcL/ATZ/s7TLJoSoyvP4FRE37i8jHs/CIFAZyXjs5LtCri/GtVpoAgqiSQigMT7/g7X4QQVMJzsSDn9J7G4G1zhGIdevkUqeaDxhbWsYIOZxjbw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762994549; c=relaxed/simple; bh=+3oWNO/USX+WbX26UwnvInu4E+Ya9z66ZBgwmlCvFV0=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Egba0ziIDASvZKRFq2FhvQuK9t9zVsG3eEAXHy1Bn9TjJGdJq1Tw+s4J0IspyzRilKPcwwgkpqJ9Tpz2RQIb+OcWYGduos5c7MLNaoAa2i7Q04N/9dAuICCFewygY39bnTCV05ekTAs0KCD00rS9BWjqGIcJ36eWrEUQ2xxDm50= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ownmail.net; spf=pass smtp.mailfrom=ownmail.net; dkim=pass (2048-bit key) header.d=ownmail.net header.i=@ownmail.net header.b=BD1pzghn; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=Ty+2pphN; arc=none smtp.client-ip=202.12.124.140 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ownmail.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ownmail.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=ownmail.net header.i=@ownmail.net header.b="BD1pzghn"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="Ty+2pphN" Received: from phl-compute-03.internal (phl-compute-03.internal [10.202.2.43]) by mailflow.stl.internal (Postfix) with ESMTP id 7C82413000C2; Wed, 12 Nov 2025 19:42:26 -0500 (EST) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-03.internal (MEProxy); Wed, 12 Nov 2025 19:42:27 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ownmail.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:reply-to:subject:subject:to:to; s=fm3; t=1762994546; x=1763001746; bh=6NxVuCfdSibZ00o8zCJhHRnn8im5+UOHTRRWKxD++2g=; b= BD1pzghnF8lP0jMY6miCHGjhgN1xZA3RXUfzxvAAV4gzIM1E89powMDJmpTt2ogT R3vb4iu09jYViCoMGawTMme8X6XZem+GXcvl0QOgzw8ndrU4MQcOAiotDnqpnVAw 57syrjGQxoHvhgusIN0XRTxvFDwvjm/UUZ54EAZs2TOX/aTwNROqfcFqGV4lwZct KQoiQWqn+bAyhFLzo6Vxion9FDs/i5uNqi8f/wjpFxqA3eAQ5JSOzy10IjCXQ/97 cXOZccd/rcikzMABdgJ/IpBZ4ex51Y9Ag3GV85R9esaowi+19HL4vPpIwNom3Mru 3Ng7gSRuLz3K6TZiTbPpWQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:reply-to:subject:subject:to:to:x-me-proxy:x-me-sender :x-me-sender:x-sasl-enc; s=fm3; t=1762994546; x=1763001746; bh=6 NxVuCfdSibZ00o8zCJhHRnn8im5+UOHTRRWKxD++2g=; b=Ty+2pphNhdBhk4KLi IWu9haWdXIDGtwHlydm8hux2CHUD4dS2xb3zf6J+qOFh2qegSLIG5WI4tjsegJwh Lp3QTlCunxonoSlZIdVSSzQXxj0FwAuHjXMAHG8Ps4AOZcWjEGbi/lUNHlVN8Q3s DCfAsTMNUoDSyZz8uNrRHuWEzqlDmIYJU0qeaXGaoIH6Scb6vE1EXW11jVjm7gFm wxrR5VlWWjlf0lBhT0EITz8gJSnR0Ed5R0UetMO0u0+MrAVMOpRygkdqi7PejCDv NHydkLfopw54X9c3y++h2Y39jmiCJSF0apPoU1/Z2a4C7FoJZ/NkrWPjNHCdPhWr rnV2w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdeggddvtdehheegucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhephffvvefufffkofgjfhhrggfgsedtkeertdertddtnecuhfhrohhmpefpvghilheu rhhofihnuceonhgvihhlsgesohifnhhmrghilhdrnhgvtheqnecuggftrfgrthhtvghrnh epveevkeffudeuvefhieeghffgudektdelkeejiedtjedugfeukedvkeffvdefvddunecu vehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepnhgvihhlsg esohifnhhmrghilhdrnhgvthdpnhgspghrtghpthhtohepgedtpdhmohguvgepshhmthhp ohhuthdprhgtphhtthhopehvihhrohesiigvnhhivhdrlhhinhhugidrohhrghdruhhkpd hrtghpthhtohepshgvlhhinhhugiesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphht thhopehlihhnuhigqdigfhhssehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtoh eplhhinhhugidquhhnihhonhhfshesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphht thhopehlihhnuhigqdhsvggtuhhrihhthidqmhhoughulhgvsehvghgvrhdrkhgvrhhnvg hlrdhorhhgpdhrtghpthhtoheplhhinhhugidqnhhfshesvhhgvghrrdhkvghrnhgvlhdr ohhrghdprhgtphhtthhopehlihhnuhigqdhkvghrnhgvlhesvhhgvghrrdhkvghrnhgvlh drohhrghdprhgtphhtthhopehlihhnuhigqdhfshguvghvvghlsehvghgvrhdrkhgvrhhn vghlrdhorhhgpdhrtghpthhtoheplhhinhhugidqtghifhhssehvghgvrhdrkhgvrhhnvg hlrdhorhhg X-ME-Proxy: Feedback-ID: iab3e480c:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Nov 2025 19:42:15 -0500 (EST) From: NeilBrown To: "Alexander Viro" , "Christian Brauner" , "Amir Goldstein" Cc: "Jan Kara" , linux-fsdevel@vger.kernel.org, Jeff Layton , Chris Mason , David Sterba , David Howells , Greg Kroah-Hartman , "Rafael J. Wysocki" , Danilo Krummrich , Tyler Hicks , Miklos Szeredi , Chuck Lever , Olga Kornievskaia , Dai Ngo , Namjae Jeon , Steve French , Sergey Senozhatsky , Carlos Maiolino , John Johansen , Paul Moore , James Morris , "Serge E. Hallyn" , Stephen Smalley , Ondrej Mosnacek , Mateusz Guzik , Lorenzo Stoakes , Stefan Berger , "Darrick J. Wong" , linux-kernel@vger.kernel.org, netfs@lists.linux.dev, ecryptfs@vger.kernel.org, linux-nfs@vger.kernel.org, linux-unionfs@vger.kernel.org, linux-cifs@vger.kernel.org, linux-xfs@vger.kernel.org, linux-security-module@vger.kernel.org, selinux@vger.kernel.org Subject: [PATCH v6 12/15] Add start_renaming_two_dentries() Date: Thu, 13 Nov 2025 11:18:35 +1100 Message-ID: <20251113002050.676694-13-neilb@ownmail.net> X-Mailer: git-send-email 2.50.0.107.gf914562f5916.dirty In-Reply-To: <20251113002050.676694-1-neilb@ownmail.net> References: <20251113002050.676694-1-neilb@ownmail.net> Reply-To: NeilBrown Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: NeilBrown A few callers want to lock for a rename and already have both dentries. Also debugfs does want to perform a lookup but doesn't want permission checking, so start_renaming_dentry() cannot be used. This patch introduces start_renaming_two_dentries() which is given both dentries. debugfs performs one lookup itself. As it will only continue with a negative dentry and as those cannot be renamed or unlinked, it is safe to do the lookup before getting the rename locks. overlayfs uses start_renaming_two_dentries() in three places and selinux uses it twice in sel_make_policy_nodes(). In sel_make_policy_nodes() we now lock for rename twice instead of just once so the combined operation is no longer atomic w.r.t the parent directory locks. As selinux_state.policy_mutex is held across the whole operation this does not open up any interesting races. Reviewed-by: Amir Goldstein Reviewed-by: Jeff Layton Signed-off-by: NeilBrown Acked-by: Paul Moore (SELinux) --- changes since v5: - sel_make_policy_nodes now uses "goto out" on error from start_renaming_t= wo_dentries() changes since v3: added missing assignment to rd.mnt_idmap in ovl_cleanup_and_whiteout --- fs/debugfs/inode.c | 48 ++++++++++++-------------- fs/namei.c | 65 ++++++++++++++++++++++++++++++++++++ fs/overlayfs/dir.c | 43 ++++++++++++++++-------- include/linux/namei.h | 2 ++ security/selinux/selinuxfs.c | 15 +++++++-- 5 files changed, 131 insertions(+), 42 deletions(-) diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index f241b9df642a..532bd7c46baf 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -842,7 +842,8 @@ int __printf(2, 3) debugfs_change_name(struct dentry *d= entry, const char *fmt, . int error =3D 0; const char *new_name; struct name_snapshot old_name; - struct dentry *parent, *target; + struct dentry *target; + struct renamedata rd =3D {}; struct inode *dir; va_list ap; =20 @@ -855,36 +856,31 @@ int __printf(2, 3) debugfs_change_name(struct dentry = *dentry, const char *fmt, . if (!new_name) return -ENOMEM; =20 - parent =3D dget_parent(dentry); - dir =3D d_inode(parent); - inode_lock(dir); + rd.old_parent =3D dget_parent(dentry); + rd.new_parent =3D rd.old_parent; + rd.flags =3D RENAME_NOREPLACE; + target =3D lookup_noperm_unlocked(&QSTR(new_name), rd.new_parent); + if (IS_ERR(target)) + return PTR_ERR(target); =20 - take_dentry_name_snapshot(&old_name, dentry); - - if (WARN_ON_ONCE(dentry->d_parent !=3D parent)) { - error =3D -EINVAL; - goto out; - } - if (strcmp(old_name.name.name, new_name) =3D=3D 0) - goto out; - target =3D lookup_noperm(&QSTR(new_name), parent); - if (IS_ERR(target)) { - error =3D PTR_ERR(target); - goto out; - } - if (d_really_is_positive(target)) { - dput(target); - error =3D -EINVAL; + error =3D start_renaming_two_dentries(&rd, dentry, target); + if (error) { + if (error =3D=3D -EEXIST && target =3D=3D dentry) + /* it isn't an error to rename a thing to itself */ + error =3D 0; goto out; } - simple_rename_timestamp(dir, dentry, dir, target); - d_move(dentry, target); - dput(target); + + dir =3D d_inode(rd.old_parent); + take_dentry_name_snapshot(&old_name, dentry); + simple_rename_timestamp(dir, dentry, dir, rd.new_dentry); + d_move(dentry, rd.new_dentry); fsnotify_move(dir, dir, &old_name.name, d_is_dir(dentry), NULL, dentry); -out: release_dentry_name_snapshot(&old_name); - inode_unlock(dir); - dput(parent); + end_renaming(&rd); +out: + dput(rd.old_parent); + dput(target); kfree_const(new_name); return error; } diff --git a/fs/namei.c b/fs/namei.c index 4b740048df97..7f0384ceb976 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3877,6 +3877,71 @@ int start_renaming_dentry(struct renamedata *rd, int= lookup_flags, } EXPORT_SYMBOL(start_renaming_dentry); =20 +/** + * start_renaming_two_dentries - Lock to dentries in given parents for ren= ame + * @rd: rename data containing parent + * @old_dentry: dentry of name to move + * @new_dentry: dentry to move to + * + * Ensure locks are in place for rename and check parentage is still corre= ct. + * + * On success the two dentries are stored in @rd.old_dentry and + * @rd.new_dentry and @rd.old_parent and @rd.new_parent are confirmed to + * be the parents of the dentries. + * + * References and the lock can be dropped with end_renaming() + * + * Returns: zero or an error. + */ +int +start_renaming_two_dentries(struct renamedata *rd, + struct dentry *old_dentry, struct dentry *new_dentry) +{ + struct dentry *trap; + int err; + + /* Already have the dentry - need to be sure to lock the correct parent */ + trap =3D lock_rename_child(old_dentry, rd->new_parent); + if (IS_ERR(trap)) + return PTR_ERR(trap); + err =3D -EINVAL; + if (d_unhashed(old_dentry) || + (rd->old_parent && rd->old_parent !=3D old_dentry->d_parent)) + /* old_dentry was removed, or moved and explicit parent requested */ + goto out_unlock; + if (d_unhashed(new_dentry) || + rd->new_parent !=3D new_dentry->d_parent) + /* new_dentry was removed or moved */ + goto out_unlock; + + if (old_dentry =3D=3D trap) + /* source is an ancestor of target */ + goto out_unlock; + + if (new_dentry =3D=3D trap) { + /* target is an ancestor of source */ + if (rd->flags & RENAME_EXCHANGE) + err =3D -EINVAL; + else + err =3D -ENOTEMPTY; + goto out_unlock; + } + + err =3D -EEXIST; + if (d_is_positive(new_dentry) && (rd->flags & RENAME_NOREPLACE)) + goto out_unlock; + + rd->old_dentry =3D dget(old_dentry); + rd->new_dentry =3D dget(new_dentry); + rd->old_parent =3D dget(old_dentry->d_parent); + return 0; + +out_unlock: + unlock_rename(old_dentry->d_parent, rd->new_parent); + return err; +} +EXPORT_SYMBOL(start_renaming_two_dentries); + void end_renaming(struct renamedata *rd) { unlock_rename(rd->old_parent, rd->new_parent); diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 6b2f88edb497..61e9484e4ab8 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -123,6 +123,7 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct= dentry *dir, struct dentry *dentry) { struct dentry *whiteout; + struct renamedata rd =3D {}; int err; int flags =3D 0; =20 @@ -134,10 +135,14 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, stru= ct dentry *dir, if (d_is_dir(dentry)) flags =3D RENAME_EXCHANGE; =20 - err =3D ovl_lock_rename_workdir(ofs->workdir, whiteout, dir, dentry); + rd.mnt_idmap =3D ovl_upper_mnt_idmap(ofs); + rd.old_parent =3D ofs->workdir; + rd.new_parent =3D dir; + rd.flags =3D flags; + err =3D start_renaming_two_dentries(&rd, whiteout, dentry); if (!err) { - err =3D ovl_do_rename(ofs, ofs->workdir, whiteout, dir, dentry, flags); - unlock_rename(ofs->workdir, dir); + err =3D ovl_do_rename_rd(&rd); + end_renaming(&rd); } if (err) goto kill_whiteout; @@ -388,6 +393,7 @@ static struct dentry *ovl_clear_empty(struct dentry *de= ntry, struct ovl_fs *ofs =3D OVL_FS(dentry->d_sb); struct dentry *workdir =3D ovl_workdir(dentry); struct dentry *upperdir =3D ovl_dentry_upper(dentry->d_parent); + struct renamedata rd =3D {}; struct path upperpath; struct dentry *upper; struct dentry *opaquedir; @@ -413,7 +419,11 @@ static struct dentry *ovl_clear_empty(struct dentry *d= entry, if (IS_ERR(opaquedir)) goto out; =20 - err =3D ovl_lock_rename_workdir(workdir, opaquedir, upperdir, upper); + rd.mnt_idmap =3D ovl_upper_mnt_idmap(ofs); + rd.old_parent =3D workdir; + rd.new_parent =3D upperdir; + rd.flags =3D RENAME_EXCHANGE; + err =3D start_renaming_two_dentries(&rd, opaquedir, upper); if (err) goto out_cleanup_unlocked; =20 @@ -431,8 +441,8 @@ static struct dentry *ovl_clear_empty(struct dentry *de= ntry, if (err) goto out_cleanup; =20 - err =3D ovl_do_rename(ofs, workdir, opaquedir, upperdir, upper, RENAME_EX= CHANGE); - unlock_rename(workdir, upperdir); + err =3D ovl_do_rename_rd(&rd); + end_renaming(&rd); if (err) goto out_cleanup_unlocked; =20 @@ -445,7 +455,7 @@ static struct dentry *ovl_clear_empty(struct dentry *de= ntry, return opaquedir; =20 out_cleanup: - unlock_rename(workdir, upperdir); + end_renaming(&rd); out_cleanup_unlocked: ovl_cleanup(ofs, workdir, opaquedir); dput(opaquedir); @@ -468,6 +478,7 @@ static int ovl_create_over_whiteout(struct dentry *dent= ry, struct inode *inode, struct ovl_fs *ofs =3D OVL_FS(dentry->d_sb); struct dentry *workdir =3D ovl_workdir(dentry); struct dentry *upperdir =3D ovl_dentry_upper(dentry->d_parent); + struct renamedata rd =3D {}; struct dentry *upper; struct dentry *newdentry; int err; @@ -499,7 +510,11 @@ static int ovl_create_over_whiteout(struct dentry *den= try, struct inode *inode, if (IS_ERR(newdentry)) goto out_dput; =20 - err =3D ovl_lock_rename_workdir(workdir, newdentry, upperdir, upper); + rd.mnt_idmap =3D ovl_upper_mnt_idmap(ofs); + rd.old_parent =3D workdir; + rd.new_parent =3D upperdir; + rd.flags =3D 0; + err =3D start_renaming_two_dentries(&rd, newdentry, upper); if (err) goto out_cleanup_unlocked; =20 @@ -536,16 +551,16 @@ static int ovl_create_over_whiteout(struct dentry *de= ntry, struct inode *inode, if (err) goto out_cleanup; =20 - err =3D ovl_do_rename(ofs, workdir, newdentry, upperdir, upper, - RENAME_EXCHANGE); - unlock_rename(workdir, upperdir); + rd.flags =3D RENAME_EXCHANGE; + err =3D ovl_do_rename_rd(&rd); + end_renaming(&rd); if (err) goto out_cleanup_unlocked; =20 ovl_cleanup(ofs, workdir, upper); } else { - err =3D ovl_do_rename(ofs, workdir, newdentry, upperdir, upper, 0); - unlock_rename(workdir, upperdir); + err =3D ovl_do_rename_rd(&rd); + end_renaming(&rd); if (err) goto out_cleanup_unlocked; } @@ -565,7 +580,7 @@ static int ovl_create_over_whiteout(struct dentry *dent= ry, struct inode *inode, return err; =20 out_cleanup: - unlock_rename(workdir, upperdir); + end_renaming(&rd); out_cleanup_unlocked: ovl_cleanup(ofs, workdir, newdentry); dput(newdentry); diff --git a/include/linux/namei.h b/include/linux/namei.h index c47713e9867c..9104c7104191 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -161,6 +161,8 @@ int start_renaming(struct renamedata *rd, int lookup_fl= ags, struct qstr *old_last, struct qstr *new_last); int start_renaming_dentry(struct renamedata *rd, int lookup_flags, struct dentry *old_dentry, struct qstr *new_last); +int start_renaming_two_dentries(struct renamedata *rd, + struct dentry *old_dentry, struct dentry *new_dentry); void end_renaming(struct renamedata *rd); =20 /** diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 232e087bce3e..404e08bf60ba 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -506,6 +506,7 @@ static int sel_make_policy_nodes(struct selinux_fs_info= *fsi, { int ret =3D 0; struct dentry *tmp_parent, *tmp_bool_dir, *tmp_class_dir; + struct renamedata rd =3D {}; unsigned int bool_num =3D 0; char **bool_names =3D NULL; int *bool_values =3D NULL; @@ -539,9 +540,14 @@ static int sel_make_policy_nodes(struct selinux_fs_inf= o *fsi, if (ret) goto out; =20 - lock_rename(tmp_parent, fsi->sb->s_root); + rd.old_parent =3D tmp_parent; + rd.new_parent =3D fsi->sb->s_root; =20 /* booleans */ + ret =3D start_renaming_two_dentries(&rd, tmp_bool_dir, fsi->bool_dir); + if (ret) + goto out; + d_exchange(tmp_bool_dir, fsi->bool_dir); =20 swap(fsi->bool_num, bool_num); @@ -549,12 +555,17 @@ static int sel_make_policy_nodes(struct selinux_fs_in= fo *fsi, swap(fsi->bool_pending_values, bool_values); =20 fsi->bool_dir =3D tmp_bool_dir; + end_renaming(&rd); =20 /* classes */ + ret =3D start_renaming_two_dentries(&rd, tmp_class_dir, fsi->class_dir); + if (ret) + goto out; + d_exchange(tmp_class_dir, fsi->class_dir); fsi->class_dir =3D tmp_class_dir; =20 - unlock_rename(tmp_parent, fsi->sb->s_root); + end_renaming(&rd); =20 out: sel_remove_old_bool_data(bool_num, bool_names, bool_values); --=20 2.50.0.107.gf914562f5916.dirty