Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Al Viro <viro@zeniv.linux.org.uk>]

On Mon, Oct 07, 2024 at 04:29:29PM -0600, Jens Axboe wrote:
> > Can I put your s-o-b on that, with e.g.
> > 
> > io_uring: IORING_OP_F[GS]ETXATTR is fine with REQ_F_FIXED_FILE
> > 
> > Rejection of IOSQE_FIXED_FILE combined with IORING_OP_[GS]ETXATTR
> > is fine - these do not take a file descriptor, so such combination
> > makes no sense.  The checks are misplaced, though - as it is, they
> > triggers on IORING_OP_F[GS]ETXATTR as well, and those do take 
> > a file reference, no matter the origin. 
> 
> Yep that's perfect, officially:
> 
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> Thanks Al!

OK, updated and force-pushed (with slight reordering).  I can almost
promise no-rebase mode for that thing from now on, as long as nobody
on fsdevel objects to fs/xattr.c part of things after I repost the
series in the current form.

One possible exception: I'm not sure that fs/internal.h is a good
place for those primitives.  OTOH, any bikeshedding in that direction
can be delayed until the next cycle...

To expand the circle of potential bikeshedders: s/do_mkdirat/filename_mkdirat/
is a reasonable idea for this series, innit?  How about turning e.g.

int __init init_mkdir(const char *pathname, umode_t mode)
{
        struct dentry *dentry;
        struct path path;
        int error;

        dentry = kern_path_create(AT_FDCWD, pathname, &path, LOOKUP_DIRECTORY);
        if (IS_ERR(dentry))
                return PTR_ERR(dentry);
        mode = mode_strip_umask(d_inode(path.dentry), mode);
        error = security_path_mkdir(&path, dentry, mode);
        if (!error)
                error = vfs_mkdir(mnt_idmap(path.mnt), path.dentry->d_inode,
                                  dentry, mode);
        done_path_create(&path, dentry);
        return error;
}

into

int __init init_mkdir(const char *pathname, umode_t mode)
{
	return filename_mkdirat(AT_FDCWD, getname_kernel(pathname), mode);
}

reducing the duplication?  It really should not be accessible to random
places in the kernel, but syscalls in core VFS + io_uring interface for
the same + possibly init/*.c...

OTOH, I'm afraid to let the "but our driver is sooo special!" crowd play
with the full set of syscalls...  init_syscalls.h is already bad enough.
Hell knows, fs/internal.h just might be a bit of deterrent...