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: <A70CCC08-E8BE-4655-9158-81754F4F6B35@cs.cmu.edu>
Date: Mon, 09 Jun 2025 09:12:02 -0400
From: Jan Harkes <jaharkes@...cmu.edu>
To: Jan Kara <jack@...e.cz>, Jeff Layton <jlayton@...nel.org>
CC: NeilBrown <neil@...wn.name>, Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Chuck Lever <chuck.lever@...cle.com>,
 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>, 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()

There are definitely still users of Coda at CMU, I don't track who else uses it, but it cannot be too many for sure.

At this point it mostly keeps you honest about little locking details in the vfs. There are some tricky details in how the inode mappings are accessed and such. I think that  is helpful for overlay and user filesystems like fuse, overlayfs, etc. but Coda is quite small so it is easy to reason about how it uses these features.

Jan

On June 9, 2025 9:00:31 AM EDT, Jan Kara <jack@...e.cz> wrote:
>On Mon 09-06-25 08:17:15, Jeff Layton wrote:
>> On Mon, 2025-06-09 at 09:09 +1000, NeilBrown 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);
>> >  }
>> 
>> 
>> Is it already time for my annual ask of "Who the heck is using coda
>> these days?" Anyway, this patch looks fine to me.
>> 
>> Reviewed-by: Jeff Layton <jlayton@...nel.org>
>
>Send a patch proposing deprecating it and we might learn that :) Searching
>the web seems to suggest it is indeed pretty close to dead.
>
>								Honza