On 2/6/25 10:31, Christian Brauner <brauner@xxxxxxxxxx> wrote:
> On Wed, Feb 05, 2025 at 08:47:23PM +0000, Mike Yuan wrote:
> > Cited from commit message of original patch [1]:
> >
> > > One use case will be setfiles(8) setting SELinux file contexts
> > > ("security.selinux") without race conditions and without a file
> > > descriptor opened with read access requiring SELinux read permission.
> >
> > Also, generally all *at() syscalls operate on O_PATH fds, unlike
> > f*() ones. Yet the O_PATH fds are rejected by *xattrat() syscalls
> > in the final version merged into tree. Instead, let's switch things
> > to CLASS(fd_raw).
> >
> > Note that there's one side effect: f*xattr() starts to work with
> > O_PATH fds too. It's not clear to me whether this is desirable
> > (e.g. fstat() accepts O_PATH fds as an outlier).
> >
> > [1] https://lore.kernel.org/all/20240426162042.191916-1-cgoettsche@xxxxxxxxxxxxx/
> >
> > Fixes: 6140be90ec70 ("fs/xattr: add *at family syscalls")
> > Signed-off-by: Mike Yuan <me@xxxxxxxxxxx>
> > Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> > Cc: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> > Cc: Christian Brauner <brauner@xxxxxxxxxx>
> > Cc: <stable@xxxxxxxxxxxxxxx>
> > ---
>
> I expanded on this before. O_PATH is intentionally limited in scope and
> it should not allow to perform operations that are similar to a read or
> write which getting and setting xattrs is.
>
> Patches that further weaken or dilute the semantics of O_PATH are not
> acceptable.
But the *at() variants really should be able to work with O_PATH fds, otherwise they're basically useless? I guess I just need to keep f*() as-is?
> > fs/xattr.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/xattr.c b/fs/xattr.c
> > index 02bee149ad96..15df71e56187 100644
> > --- a/fs/xattr.c
> > +++ b/fs/xattr.c
> > @@ -704,7 +704,7 @@ static int path_setxattrat(int dfd, const char __user *pathname,
> >
> > filename = getname_maybe_null(pathname, at_flags);
> > if (!filename) {
> > - CLASS(fd, f)(dfd);
> > + CLASS(fd_raw, f)(dfd);
> > if (fd_empty(f))
> > error = -EBADF;
> > else
> > @@ -848,7 +848,7 @@ static ssize_t path_getxattrat(int dfd, const char __user *pathname,
> >
> > filename = getname_maybe_null(pathname, at_flags);
> > if (!filename) {
> > - CLASS(fd, f)(dfd);
> > + CLASS(fd_raw, f)(dfd);
> > if (fd_empty(f))
> > return -EBADF;
> > return file_getxattr(fd_file(f), &ctx);
> > @@ -978,7 +978,7 @@ static ssize_t path_listxattrat(int dfd, const char __user *pathname,
> >
> > filename = getname_maybe_null(pathname, at_flags);
> > if (!filename) {
> > - CLASS(fd, f)(dfd);
> > + CLASS(fd_raw, f)(dfd);
> > if (fd_empty(f))
> > return -EBADF;
> > return file_listxattr(fd_file(f), list, size);
> > @@ -1079,7 +1079,7 @@ static int path_removexattrat(int dfd, const char __user *pathname,
> >
> > filename = getname_maybe_null(pathname, at_flags);
> > if (!filename) {
> > - CLASS(fd, f)(dfd);
> > + CLASS(fd_raw, f)(dfd);
> > if (fd_empty(f))
> > return -EBADF;
> > return file_removexattr(fd_file(f), &kname);
> >
> > base-commit: a86bf2283d2c9769205407e2b54777c03d012939
> > --
> > 2.48.1
> >
> >
>
