| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4d2c35c1b627f1ad26062a92d653677d7b79bd81.camel@kernel.org>
Date: Thu, 05 Feb 2026 07:38:30 -0500
From: Jeff Layton <jlayton@...nel.org>
To: NeilBrown <neil@...wn.name>, Christian Brauner <brauner@...nel.org>,
Alexander Viro <viro@...iv.linux.org.uk>, David Howells
<dhowells@...hat.com>, Jan Kara <jack@...e.cz>, Chuck Lever
<chuck.lever@...cle.com>, Miklos Szeredi <miklos@...redi.hu>, Amir
Goldstein <amir73il@...il.com>, John Johansen
<john.johansen@...onical.com>, Paul Moore <paul@...l-moore.com>, James
Morris <jmorris@...ei.org>, "Serge E. Hallyn" <serge@...lyn.com>, Stephen
Smalley <stephen.smalley.work@...il.com>
Cc: linux-kernel@...r.kernel.org, netfs@...ts.linux.dev,
linux-fsdevel@...r.kernel.org, linux-nfs@...r.kernel.org,
linux-unionfs@...r.kernel.org, apparmor@...ts.ubuntu.com,
linux-security-module@...r.kernel.org, selinux@...r.kernel.org
Subject: Re: [PATCH 09/13] cachefiles: change cachefiles_bury_object to use
start_renaming_dentry()
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@...wn.name>
>
> Rather then using lock_rename() and lookup_one() etc we can use
> the new start_renaming_dentry(). This is part of centralising dir
> locking and lookup so that locking rules can be changed.
>
> Some error check are removed as not necessary. Checks for rep being a
> non-dir or IS_DEADDIR and the check that ->graveyard is still a
> directory only provide slightly more informative errors and have been
> dropped.
>
> Signed-off-by: NeilBrown <neil@...wn.name>
> ---
> fs/cachefiles/namei.c | 76 ++++++++-----------------------------------
> 1 file changed, 14 insertions(+), 62 deletions(-)
>
> diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
> index e5ec90dccc27..3af42ec78411 100644
> --- a/fs/cachefiles/namei.c
> +++ b/fs/cachefiles/namei.c
> @@ -270,7 +270,8 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> struct dentry *rep,
> enum fscache_why_object_killed why)
> {
> - struct dentry *grave, *trap;
> + struct dentry *grave;
> + struct renamedata rd = {};
> struct path path, path_to_graveyard;
> char nbuffer[8 + 8 + 1];
> int ret;
> @@ -302,77 +303,36 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> (uint32_t) ktime_get_real_seconds(),
> (uint32_t) atomic_inc_return(&cache->gravecounter));
>
> - /* do the multiway lock magic */
> - trap = lock_rename(cache->graveyard, dir);
> - if (IS_ERR(trap))
> - return PTR_ERR(trap);
> -
> - /* do some checks before getting the grave dentry */
> - if (rep->d_parent != dir || IS_DEADDIR(d_inode(rep))) {
> - /* the entry was probably culled when we dropped the parent dir
> - * lock */
> - unlock_rename(cache->graveyard, dir);
> - _leave(" = 0 [culled?]");
> - return 0;
> - }
> -
> - if (!d_can_lookup(cache->graveyard)) {
> - unlock_rename(cache->graveyard, dir);
> - cachefiles_io_error(cache, "Graveyard no longer a directory");
> - return -EIO;
> - }
> -
> - if (trap == rep) {
> - unlock_rename(cache->graveyard, dir);
> - cachefiles_io_error(cache, "May not make directory loop");
> + rd.mnt_idmap = &nop_mnt_idmap;
> + rd.old_parent = dir;
> + rd.new_parent = cache->graveyard;
> + rd.flags = 0;
> + ret = start_renaming_dentry(&rd, 0, rep, &QSTR(nbuffer));
> + if (ret) {
> + cachefiles_io_error(cache, "Cannot lock/lookup in graveyard");
> return -EIO;
> }
>
> if (d_mountpoint(rep)) {
> - unlock_rename(cache->graveyard, dir);
> + end_renaming(&rd);
> cachefiles_io_error(cache, "Mountpoint in cache");
> return -EIO;
> }
>
> - grave = lookup_one(&nop_mnt_idmap, &QSTR(nbuffer), cache->graveyard);
> - if (IS_ERR(grave)) {
> - unlock_rename(cache->graveyard, dir);
> - trace_cachefiles_vfs_error(object, d_inode(cache->graveyard),
> - PTR_ERR(grave),
> - cachefiles_trace_lookup_error);
> -
> - if (PTR_ERR(grave) == -ENOMEM) {
> - _leave(" = -ENOMEM");
> - return -ENOMEM;
> - }
> -
> - cachefiles_io_error(cache, "Lookup error %ld", PTR_ERR(grave));
> - return -EIO;
> - }
> -
> + grave = rd.new_dentry;
> if (d_is_positive(grave)) {
> - unlock_rename(cache->graveyard, dir);
> - dput(grave);
> + end_renaming(&rd);
> grave = NULL;
> cond_resched();
> goto try_again;
> }
>
> if (d_mountpoint(grave)) {
> - unlock_rename(cache->graveyard, dir);
> - dput(grave);
> + end_renaming(&rd);
> cachefiles_io_error(cache, "Mountpoint in graveyard");
> return -EIO;
> }
>
> - /* target should not be an ancestor of source */
> - if (trap == grave) {
> - unlock_rename(cache->graveyard, dir);
> - dput(grave);
> - cachefiles_io_error(cache, "May not make directory loop");
> - return -EIO;
> - }
> -
> /* attempt the rename */
> path.mnt = cache->mnt;
> path.dentry = dir;
> @@ -382,13 +342,6 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> if (ret < 0) {
> cachefiles_io_error(cache, "Rename security error %d", ret);
> } else {
> - struct renamedata rd = {
> - .mnt_idmap = &nop_mnt_idmap,
> - .old_parent = dir,
> - .old_dentry = rep,
> - .new_parent = cache->graveyard,
> - .new_dentry = grave,
> - };
> trace_cachefiles_rename(object, d_inode(rep)->i_ino, why);
> ret = cachefiles_inject_read_error();
> if (ret == 0)
> @@ -402,8 +355,7 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> }
>
> __cachefiles_unmark_inode_in_use(object, d_inode(rep));
> - unlock_rename(cache->graveyard, dir);
> - dput(grave);
> + end_renaming(&rd);
> _leave(" = 0");
> return 0;
> }
Reviewed-by: Jeff Layton <jlayton@...nel.org>
Powered by blists - more mailing lists