Re: [PATCH v2 02/21] fuse: prepare for passthrough of inode operations

On Mon, May 18, 2026 at 11:39 AM Amir Goldstein <[email protected]> wrote:
>
> On Mon, May 18, 2026 at 8:04 PM Joanne Koong <[email protected]> wrote:
> >
> > On Sat, May 16, 2026 at 9:11 AM Amir Goldstein <[email protected]> wrote:
> > >
> > > On Sat, May 16, 2026 at 3:34 AM Joanne Koong <[email protected]> wrote:
> > > >
> > > > On Fri, May 15, 2026 at 5:52 PM Joanne Koong <[email protected]> wrote:
> > > > >
> > > > > +/* op bits for fuse_backing_map ops_mask */
> > > > > +#define FUSE_PASSTHROUGH_OP_READ       FUSE_PASSTHROUGH_OP(FUSE_READ)
> > > > > +#define FUSE_PASSTHROUGH_OP_WRITE      FUSE_PASSTHROUGH_OP(FUSE_WRITE)
> > > >
> > > > Do you think we should couple the passthrough op code to the fuse
> > > > opcode so closely, instead of defining the passthrough ops separately,
> > > > eg doing something like this?:
> > > >
> > > > #define FUSE_PASSTHROUGH_READ         (1 << 0)
> > > > #define FUSE_PASSTHROUGH_WRITE       (1 << 1)
> > >
> > > Don't drop the _OP_ please
> > > Need it to distinguish from FUSE_PASSTHROUGH{,_INO}
> > > and we need to think if we want to format this as 32bit and extend later
> > > or start with a u64 ops_mask format from the start.
> > >
> > > >
> > > > In the (far) future when some more advanced passthrough features get
> > > > added (eg full subtree passthrough), it seems like we'd want to add a
> > > > passthrough op for that,
> > >
> > > I don't know. fuse_backing_map has flags, why would we want to put that
> > > in ops_mask.
> >
> > That's a good point, I like your idea of using flags for that.
> >
> > >
> > > > but that wouldn't map to a fuse op. I think
> > > > there are also some fuse ops we might skip defining as passthrough ops
> > > > but are implicitly passed through (eg FUSE_STATX which is covered by
> > > > FUSE_GETATTR),
> > >
> > > This could also become the case with FUSE_CREATE{,_HANDLE}
> > > or {FUSE,FUSEX}_CREAT.
> > >
> > > TBH, the fact that some ops will never be in the mask and
> > > that some ops have a canonical bit in itself does not justify creating
> > > a different mapping. Do we also want to squeeze the mask into 32bit
> > > and leave reserved space? my intuition is to leave it u64.
> > >
> > > > so it seems more intuitive to define the passthrough
> > > > ops as describing capabilities rather than implying that it describes
> > > > what specific opcodes get passed through?
> > >
> > > hmm. I guess you have a point that it is a bit limiting to commit
> > > to this arithmetic in UAPI.
> > >
> > > My thinking behind this was ease of use with these helpers:
> > >
> > >        if (!fuse_passthrough_op(file_inode(in), FUSE_READ))
> > >
> > > but those could just as well be macros:
> > >
> > > static inline bool fuse_inode_passthrough_op(struct inode *inode,
> > >                                             u64 opbit)...
> > >
> > > #define FUSE_PASSTHROUGH_OP(inode, opname)    \
> > >              fuse_passthrough_op(inode, FUSE_PASSTHROUGH_OP_ ## opname)
> > >
> > > #define FUSE_BACKING_MAP_OP(map, opname) \
> > >        ((map)->ops_mask & FUSE_PASSTHROUGH_OP ## opname))
> > >
> > > ...
> > >        if (!FUSE_PASSTHROUGH_OP(file_inode(in), READ))
> > >
> > > wdyt?
> >
> > Nice, the macro idea is neat!
> >
> > I feel like it gives more flexibility to define it separately and I
> > think with that, u32 would be more than enough bits as well. But I'm
> > happy to go with what you think would be better here.
>
> I am not sure. Let's keep it u64 for now.
> Since the struct is in an ioctl, it does not really matter much.
> If we ever consider moving backing_map responses inline
> in lookup/readdirplus response (maybe over io uring) we could reconsider.

Sounds good. For v3 I'll keep it a u64 but define them separately and
use your macro helper for mapping from opcode to passthrough op.

>
> In case you missed the exciting conversations about this API
> original fuse passthough patches from Android had the backing fd
> in the open response but it was deemed unsafe writing fds to
> /dev/fuse, so ioctl was chosen instead [1].

Wow, that's a clever attack. Thanks for sharing the link.

Thanks,
Joanne
>
> With io_uring I don't think the security concern holds, so I see no reason
> why backing_map would not be possible in-place for readdirplus
> responses if we wanted to do that, but we can also ignore all this for now.
>
> Thanks,
> Amir.
>
> [1] https://lore.kernel.org/linux-fsdevel/CAG48ez3ZX8R9kRAQhung2_e3wjowu5cPh7WL3U866mkga-kftQ@mail.gmail.com/