Re: [PATCH v2 19/21] fuse: add passthrough setattr

On Sat, May 16, 2026 at 4:03 PM Amir Goldstein <[email protected]> wrote:
>
> On Sat, May 16, 2026 at 3:04 AM Joanne Koong <[email protected]> wrote:
> >
> > On Fri, May 15, 2026 at 5:53 PM Joanne Koong <[email protected]> wrote:
> > >
> > > Add passthrough setattr which sets attributes directly on the backing
> > > inode through backing_inode_setattr() instead of sending FUSE_SETATTR to
> > > the server.
> > >
> > > Passthrough setattr is checked before the
> > > handle_killpriv/handle_killpriv_v2 suid/sgid stripping because the
> > > stripping is handled natively by notify_change() on the backing inode.
> > >
> > > Signed-off-by: Joanne Koong <[email protected]>
> > > ---
> > >  fs/fuse/dir.c             |  3 +++
> > >  fs/fuse/fuse_i.h          |  3 ++-
> > >  fs/fuse/passthrough.c     | 28 ++++++++++++++++++++++++++++
> > >  include/uapi/linux/fuse.h |  1 +
> > >  4 files changed, 34 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> > > index 4c7e3e1604af..b67b3b334e69 100644
> > > --- a/fs/fuse/dir.c
> > > +++ b/fs/fuse/dir.c
> > > @@ -2506,6 +2506,9 @@ static int fuse_setattr(struct mnt_idmap *idmap, struct dentry *entry,
> > >         if (!fuse_allow_current_process(get_fuse_conn(inode)))
> > >                 return -EACCES;
> > >
> > > +       if (fuse_passthrough_op(inode, FUSE_SETATTR))
> > > +               return fuse_passthrough_setattr(entry, attr);
> >
> > In the v1 discussion [1], there was a suggestion about gating
> > FUSE_PASSTHROUGH_INO on FUSE_HANDLE_KILLPRIV_V2 to avoid the
> > non-atomic killpriv path, but I think we were only encountering the
> > non-atomic killpriv path because I was calling
> > fuse_passthrough_setattr() in the wrong place. I think we can avoid
> > this altogether by just having the backing filesystem handle all of
> > the suid/sgid stripping atomically through its own notify_change()
> > path, which will let us skip all the fuse killpriv handling logic in
> > this function.
>
> I think it still matters, but to the only-passthrough-GETATTR case
> and I am still in favor of making this change.
> My point was why have the discussion whether getattr
> for the purpose of killpriv should be passthrough or not (next patch)?
> Why not drop the next patch instead because it will become moot.
>
> If FUSE_PASSTHROUGH_INO requires FUSE_HANDLE_KILLPRIV_V2
> it means that FUSE kernel is never trying to play games removing privs
> itself, not with passthrough and not without passthrough.
>
> Quoting your comment on next patch
> "I think there might be some use cases
> though where having only getattr passed through and not setattr is
> useful (eg server wants fast stat() but needs to intercept attribute
> changes for access control or policy enforcement)."
>
> Valid use case, but in this valid use case, said server may reject/allow
> attribute changes, but eventually if it allows them it will likely apply
> them to the very file which was set as backing inode, so essentially
> FUSE_HANDLE_KILLPRIV_V2 is the natural choice for a
> FUSE_PASSTHROUGH_INO server, even when it wants to introspect
> setattr.
>
> I think the same is true for ATOMIC_O_TRUNC.
> Either backing fs deals with O_TRUNC or server deals with O_TRUNC.
>
> But this is my opinion - I am open to hearing other opinions.

I was thinking from the user perspective, it'd be nicer to not couple
things unless it was absolutely necessary, especially since
passthrough is a per-inode/per-file attribute whereas KILLPRIV_V2 is a
global server configuration (eg server might want passthrough on just
a few files without taking on killpriv responsibility for the whole
filesystem).

But I think everything you wrote makes sense and I don't think
handling the stripping stuff is too burdensome to ask of the server.

For v3, I'll make the change to require FUSE_KILLPRIV_V2 for
FUSE_PASSTHROUGH_INO.

Thanks,
Joanne

>
> Thanks,
> Amir.