On Thu, Dec 30, 2021 at 01:41:35AM +0000, Al Viro wrote:
> On Wed, Dec 29, 2021 at 12:30:02PM -0800, Stefan Roesch wrote:
>
> > +static int io_getxattr(struct io_kiocb *req, unsigned int issue_flags)
> > +{
> > + struct io_xattr *ix = &req->xattr;
> > + unsigned int lookup_flags = LOOKUP_FOLLOW;
> > + struct path path;
> > + int ret;
> > +
> > + if (issue_flags & IO_URING_F_NONBLOCK)
> > + return -EAGAIN;
> > +
> > +retry:
> > + ret = do_user_path_at_empty(AT_FDCWD, ix->filename, lookup_flags, &path);
> > + if (!ret) {
> > + ret = do_getxattr(mnt_user_ns(path.mnt),
> > + path.dentry,
> > + ix->ctx.kname->name,
> > + (void __user *)ix->ctx.value,
> > + ix->ctx.size);
> > +
> > + path_put(&path);
> > + if (retry_estale(ret, lookup_flags)) {
> > + lookup_flags |= LOOKUP_REVAL;
> > + goto retry;
> > + }
> > + }
> > + putname(ix->filename);
> > +
> > + __io_getxattr_finish(req, ret);
> > + return 0;
> > +}
>
> Looking at that one... Is there any reason to have that loop (from retry: to
> putname() call) outside of fs/xattr.c? Come to think of that, why bother
> polluting your struct io_xattr with ->filename?
>
> Note, BTW, that we already have this:
> static ssize_t path_getxattr(const char __user *pathname,
> const char __user *name, void __user *value,
> size_t size, unsigned int lookup_flags)
> {
> struct path path;
> ssize_t error;
> retry:
> error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
> if (error)
> return error;
> error = getxattr(mnt_user_ns(path.mnt), path.dentry, name, value, size);
> path_put(&path);
> if (retry_estale(error, lookup_flags)) {
> lookup_flags |= LOOKUP_REVAL;
> goto retry;
> }
> return error;
> }
> in there. The only potential benefit here would be to avoid repeated getname
> in case of having hit -ESTALE and going to repeat the entire fucking pathwalk
> with maximal paranoia, asking the server(s) involved to revalidate on every
> step, etc.
>
> If we end up going there, who the hell *cares* about the costs of less than
> a page worth of copy_from_user()? We are already on a very slow path as it
> is, so what's the point?
BTW, if the answer is along the lines of "we want to copy the name in prep
phase fo $REASONS", I would like to hear what it is that makes getxattr()
different from statx() in that respect.
