On Tue 22-04-25 16:59:02, Christian Brauner wrote:
> On Fri, Mar 21, 2025 at 08:48:42PM +0100, Andrey Albershteyn wrote:
> > From: Andrey Albershteyn <aalbe...@redhat.com>
> >
> > Introduce getfsxattrat and setfsxattrat syscalls to manipulate inode
> > extended attributes/flags. The syscalls take parent directory fd and
> > path to the child together with struct fsxattr.
> >
> > This is an alternative to FS_IOC_FSSETXATTR ioctl with a difference
> > that file don't need to be open as we can reference it with a path
> > instead of fd. By having this we can manipulated inode extended
> > attributes not only on regular files but also on special ones. This
> > is not possible with FS_IOC_FSSETXATTR ioctl as with special files
> > we can not call ioctl() directly on the filesystem inode using fd.
> >
> > This patch adds two new syscalls which allows userspace to get/set
> > extended inode attributes on special files by using parent directory
> > and a path - *at() like syscall.
> >
> > CC: linux-...@vger.kernel.org
> > CC: linux-fsde...@vger.kernel.org
> > CC: linux-...@vger.kernel.org
> > Signed-off-by: Andrey Albershteyn <aalbe...@redhat.com>
> > Acked-by: Arnd Bergmann <a...@arndb.de>
...
> > + struct fsxattr __user *, ufsx, size_t, usize,
> > + unsigned int, at_flags)
> > +{
> > + struct fileattr fa = {};
> > + struct path filepath;
> > + int error;
> > + unsigned int lookup_flags = 0;
> > + struct filename *name;
> > + struct fsxattr fsx = {};
> > +
> > + BUILD_BUG_ON(sizeof(struct fsxattr) < FSXATTR_SIZE_VER0);
> > + BUILD_BUG_ON(sizeof(struct fsxattr) != FSXATTR_SIZE_LATEST);
> > +
> > + if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> > + return -EINVAL;
> > +
> > + if (!(at_flags & AT_SYMLINK_NOFOLLOW))
> > + lookup_flags |= LOOKUP_FOLLOW;
> > +
> > + if (at_flags & AT_EMPTY_PATH)
> > + lookup_flags |= LOOKUP_EMPTY;
> > +
> > + if (usize > PAGE_SIZE)
> > + return -E2BIG;
> > +
> > + if (usize < FSXATTR_SIZE_VER0)
> > + return -EINVAL;
> > +
> > + name = getname_maybe_null(filename, at_flags);
> > + if (!name) {
>
> This is broken as it doesn't handle AT_FDCWD correctly. You need:
>
> name = getname_maybe_null(filename, at_flags);
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> if (!name && dfd >= 0) {
> CLASS(fd, f)(dfd);
Ah, you're indeed right that if dfd == AT_FDCWD and filename == NULL, the
we should operate on cwd but we'd bail with error here. I've missed that
during my review. But as far as I've checked the same bug is there in
path_setxattrat() and path_getxattrat() so we should fix this there as
well?
Honza
--
Jan Kara <j...@suse.com>
SUSE Labs, CR
