linux-kernel - Re: [PATCH v2 3/3] pidfs: implement file handle support

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAOQ4uxhP9_WPinm2wM6uW+L0rH_xwwrw=qAUd_YjzbFCJBf0+g@mail.gmail.com>
Date: Fri, 15 Nov 2024 08:50:30 +0100
From: Amir Goldstein <amir73il@...il.com>
To: Erin Shepherd <erin.shepherd@....eu>
Cc: Christian Brauner <brauner@...nel.org>, Alexander Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>, 
	Chuck Lever <chuck.lever@...cle.com>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Jeff Layton <jlayton@...nel.org>, 
	linux-nfs@...r.kernel.org
Subject: Re: [PATCH v2 3/3] pidfs: implement file handle support

On Thu, Nov 14, 2024 at 11:51 PM Erin Shepherd <erin.shepherd@....eu> wrote:
>
> On 14/11/2024 15:13, Christian Brauner wrote:
>
> > On Thu, Nov 14, 2024 at 02:13:06PM +0100, Erin Shepherd wrote:
> >> These two concerns combined with the special flag make me wonder if pidfs
> >> is so much of a special snowflake we should just special case it up front
> >> and skip all of the shared handle decode logic?
> > Care to try a patch and see what it looks like?
>
> The following is a completely untested sketch on top of the existing patch series.
> Some notes:
>
> - I made heavy use of the cleanup macros. I'm happy to convert things back to
>   goto out_xx style if preferred - writing things this way just made bashing out
>   the code without dropping resources on the floor easier

Your cleanup is very welcome, just please! not in the same patch as refactoring
and logic changes. Please do these 3 different things in different commits.
This patch is unreviewable as far as I am concerned.

> - If you don't implement fh_to_dentry then name_to_handle_at will just return an error
>   unless called with AT_HANDLE_FID. We need to decide what to do about that

What's to decide? I did not understand the problem.

> - The GET_PATH_FD_IS_NORMAL/etc constants don't match (what I see as) usual kernel style
>   but I'm not sure how to conventionally express something like that

I believe the conventional way to express a custom operation is an
optional method.

For example:

static int exportfs_get_name(struct vfsmount *mnt, struct dentry *dir,
                char *name, struct dentry *child)
{
        const struct export_operations *nop = dir->d_sb->s_export_op;
        struct path path = {.mnt = mnt, .dentry = dir};

        if (nop->get_name)
                return nop->get_name(dir, name, child);
        else
                return get_name(&path, name, child);
}

There are plenty of optional custom inode, file, sb, dentry
operations with default fallback. some examples:

        if (dir_inode->i_op->atomic_open) {
                dentry = atomic_open(nd, dentry, file, open_flag, mode);

        if (!splice && file_out->f_op->copy_file_range) {
                ret = file_out->f_op->copy_file_range(file_in, pos_in,
                                                      file_out, pos_out,
                                                      len, flags);
        } else if (!splice && file_in->f_op->remap_file_range && samesb) {
                ret = file_in->f_op->remap_file_range(file_in, pos_in,

So I think the right model for you to follow is a custom optional
s_export_op->open_by_handle() operation.

Thanks,
Amir.