Re: [PATCH 2/3] xattrat: accept empty O_PATH file descriptors

[Andreas Gruenbacher]

On Tue, Jun 30, 2026 at 2:33 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> On 2026-06-30 14:06 +0200, Andreas Gruenbacher wrote:
> > On Tue, Jun 30, 2026 at 11:12 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> > > > Right now, the setxattrat(), getxattrat(), listxattrat(), and removexattrat()
> > > > system calls fail with -EBADF when dfd is an O_PATH file descriptor, pathname
> > > > is an empty string or NULL, and the AT_EMPTY_PATH flag in at_flags is set.
> > > > (Regular non-O_PATH file descriptors are accepted.)  This is inconsistent with
> > > > the behavior of system calls like fstatat() and fchmodat() which do accept
> > > > those file descriptors.
> > > >
> > > > The same operations can also be carried out indirectly by using
> > > > "/proc/self/fd/<dfd>" as the pathname, but that is only more cumbersome, and
> > > > for no good reason.  Fix that by changing the xattrat() system calls to accept
> > > > these file descriptors directly.
> > > >
> > > > We stick with the existing practice of leaving the behavior of the fsetxattr(),
> > > > fgetxattr(), flistxattr(), and fremovexattr() system calls unchanged: those
> > > > will still reject O_PATH file descriptors.
> > > >
> > > > Signed-off-by: Andreas Gruenbacher <agruenba@xxxxxxxxxx>
> > > >
> > > > diff --git a/fs/xattr.c b/fs/xattr.c
> > > > index d58979115200..4737554df5fa 100644
> > > > --- a/fs/xattr.c
> > > > +++ b/fs/xattr.c
> > > > @@ -700,7 +700,8 @@ int filename_setxattr(int dfd, struct filename *filename,
> > > >
> > > >  static int path_setxattrat(int dfd, const char __user *pathname,
> > > >                          unsigned int at_flags, const char __user *name,
> > > > -                        const void __user *value, size_t size, int flags)
> > > > +                        const void __user *value, size_t size, int flags,
> > > > +                        fmode_t mask)
> > > >  {
> > > >       struct xattr_name kname;
> > > >       struct kernel_xattr_ctx ctx = {
> > > > @@ -725,7 +726,7 @@ static int path_setxattrat(int dfd, const char __user *pathname,
> > > >
> > > >       CLASS(filename_maybe_null, filename)(pathname, at_flags);
> > > >       if (!filename && dfd >= 0) {
> > > > -             CLASS(fd, f)(dfd);
> > > > +             CLASS(fd_except, f)(dfd, mask);
> > >
> > > So I can live with the O_PATH change, I guess...
> > > but I really dislike fd_except. The code should never bother to pass
> > > FMODE_PATH directly which is just rather ugly and we've taken pain to
> > > make it work without passing it.
> > >
> > > I also don't yet understand why this would be necessary at all.
> >
> > So we've got some system calls that should accept O_PATH file descriptors
> > ({get,set,list,remove}xattrat), and others that should not
> > (f{get,set,list,remove}xattr).
>
> I'm very confused why the f*at() variants are supposed to accept O_PATH
> but the f*() aren't? What's the point?

Oh, I see. To keep things consistent with fchmod() and fchown() vs.
fchmodat() and fchownat(): the former two also don't accept O_PATH
file descriptors, while the latter two do.

> glibc has in multiple cases used the f*at() variant to implement the f*() variant.

Yes, but not the other way around, and keeping the behaviour of the
existing f*() calls unchanged seems to be the key idea here. The *at()
variants require the AT_EMPTY_PATH flag to be set when operating
directly on a file descriptor, so it can't happen by accident.

Thanks,
Andreas