linux-kernel - Re: [PATCH 3/5] coda: use iterate_dir() in coda

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7DCEE406-F27B-4120-BD7A-B0A5E9028FAF@cs.cmu.edu>
Date: Mon, 09 Jun 2025 10:35:36 -0400
From: Jan Harkes <jaharkes@...cmu.edu>
To: NeilBrown <neil@...wn.name>, Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 Chuck Lever <chuck.lever@...cle.com>, Jeff Layton <jlayton@...nel.org>,
 Amir Goldstein <amir73il@...il.com>, David Howells <dhowells@...hat.com>,
 Tyler Hicks <code@...icks.com>, Miklos Szeredi <miklos@...redi.hu>,
 Carlos Maiolino <cem@...nel.org>
CC: linux-fsdevel@...r.kernel.org, coda@...cmu.edu, linux-nfs@...r.kernel.org,
 netfs@...ts.linux.dev, ecryptfs@...r.kernel.org,
 linux-unionfs@...r.kernel.org, linux-xfs@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/5] coda: use iterate_dir() in coda_readdir()

That change is probably good.

The Coda user space always writes directory data to a file, so the normal path always uses coda_venus_readdir.

The iterate_dir code was afaik mostly used while developing the original kernel module during the Linux-2.1 era. It was using a trivial user space helper that would simply re-export an existing filesystem subtree. Sort of like a bind mount before bind mounts existed.

Jan

On June 8, 2025 7:37:25 PM EDT, NeilBrown <neil@...wn.name> wrote:
>The code in coda_readdir() is nearly identical to iterate_dir().
>Differences are:
> - iterate_dir() is killable
> - iterate_dir() adds permission checking and accessing notifications
>
>I believe these are not harmful for coda so it is best to use
>iterate_dir() directly.  This will allow locking changes without
>touching the code in coda.
>
>Signed-off-by: NeilBrown <neil@...wn.name>
>---
> fs/coda/dir.c | 12 ++----------
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
>diff --git a/fs/coda/dir.c b/fs/coda/dir.c
>index ab69d8f0cec2..ca9990017265 100644
>--- a/fs/coda/dir.c
>+++ b/fs/coda/dir.c
>@@ -429,17 +429,9 @@ static int coda_readdir(struct file *coda_file, struct dir_context *ctx)
> 	cfi = coda_ftoc(coda_file);
> 	host_file = cfi->cfi_container;
> 
>-	if (host_file->f_op->iterate_shared) {
>-		struct inode *host_inode = file_inode(host_file);
>-		ret = -ENOENT;
>-		if (!IS_DEADDIR(host_inode)) {
>-			inode_lock_shared(host_inode);
>-			ret = host_file->f_op->iterate_shared(host_file, ctx);
>-			file_accessed(host_file);
>-			inode_unlock_shared(host_inode);
>-		}
>+	ret = iterate_dir(host_file, ctx);
>+	if (ret != -ENOTDIR)
> 		return ret;
>-	}
> 	/* Venus: we must read Venus dirents from a file */
> 	return coda_venus_readdir(coda_file, ctx);
> }