linux-ext4 - Re: [PATCH 2/2] fuse_trace: move the passthrough-specific code back to passthrough.c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJnrk1Z+yA+jd_wmEz1sxEpFEvqL2VLU6up-DCmMO0ozHOS2HQ@mail.gmail.com>
Date: Mon, 10 Nov 2025 09:50:55 -0800
From: Joanne Koong <joannelkoong@...il.com>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: miklos@...redi.hu, bernd@...ernd.com, neal@...pa.dev, 
	linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 2/2] fuse_trace: move the passthrough-specific code back
 to passthrough.c

On Fri, Nov 7, 2025 at 4:24 PM Darrick J. Wong <djwong@...nel.org> wrote:
>
> On Fri, Nov 07, 2025 at 12:55:39PM -0800, Joanne Koong wrote:
> > On Tue, Oct 28, 2025 at 5:44 PM Darrick J. Wong <djwong@...nel.org> wrote:
> > >
> > > From: Darrick J. Wong <djwong@...nel.org>
> > >
> > > Add tracepoints for the previous patch.
> > >
> > > Signed-off-by: "Darrick J. Wong" <djwong@...nel.org>
> > > ---
> > >  fs/fuse/fuse_trace.h |   35 +++++++++++++++++++++++++++++++++++
> > >  fs/fuse/backing.c    |    5 +++++
> > >  2 files changed, 40 insertions(+)
> > >
> > >
> > > diff --git a/fs/fuse/fuse_trace.h b/fs/fuse/fuse_trace.h
> > > index bbe9ddd8c71696..286a0845dc0898 100644
> > > --- a/fs/fuse/fuse_trace.h
> > > +++ b/fs/fuse/fuse_trace.h
> > > @@ -124,6 +124,41 @@ TRACE_EVENT(fuse_request_end,
> > >                   __entry->unique, __entry->len, __entry->error)
> > >  );
> > >
> > > +#ifdef CONFIG_FUSE_BACKING
> > > +TRACE_EVENT(fuse_backing_class,
> > > +       TP_PROTO(const struct fuse_conn *fc, unsigned int idx,
> > > +                const struct fuse_backing *fb),
> > > +
> > > +       TP_ARGS(fc, idx, fb),
> > > +
> > > +       TP_STRUCT__entry(
> > > +               __field(dev_t,                  connection)
> > > +               __field(unsigned int,           idx)
> > > +               __field(unsigned long,          ino)
> > > +       ),
> > > +
> > > +       TP_fast_assign(
> > > +               struct inode *inode = file_inode(fb->file);
> > > +
> > > +               __entry->connection     =       fc->dev;
> > > +               __entry->idx            =       idx;
> > > +               __entry->ino            =       inode->i_ino;
> > > +       ),
> > > +
> > > +       TP_printk("connection %u idx %u ino 0x%lx",
> > > +                 __entry->connection,
> > > +                 __entry->idx,
> > > +                 __entry->ino)
> > > +);
> > > +#define DEFINE_FUSE_BACKING_EVENT(name)                \
> > > +DEFINE_EVENT(fuse_backing_class, name,         \
> > > +       TP_PROTO(const struct fuse_conn *fc, unsigned int idx, \
> > > +                const struct fuse_backing *fb), \
> > > +       TP_ARGS(fc, idx, fb))
> > > +DEFINE_FUSE_BACKING_EVENT(fuse_backing_open);
> > > +DEFINE_FUSE_BACKING_EVENT(fuse_backing_close);
> > > +#endif /* CONFIG_FUSE_BACKING */
> > > +
> > >  #endif /* _TRACE_FUSE_H */
> > >
> > >  #undef TRACE_INCLUDE_PATH
> > > diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
> > > index f5efbffd0f456b..b83a3c1b2dff7a 100644
> > > --- a/fs/fuse/backing.c
> > > +++ b/fs/fuse/backing.c
> > > @@ -72,6 +72,7 @@ static int fuse_backing_id_free(int id, void *p, void *data)
> > >
> > >         WARN_ON_ONCE(refcount_read(&fb->count) != 1);
> > >
> > > +       trace_fuse_backing_close((struct fuse_conn *)data, id, fb);
> > >         fuse_backing_free(fb);
> > >         return 0;
> > >  }
> > > @@ -145,6 +146,8 @@ int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map)
> > >                 fb = NULL;
> > >                 goto out;
> > >         }
> > > +
> > > +       trace_fuse_backing_open(fc, res, fb);
> > >  out:
> > >         pr_debug("%s: fb=0x%p, ret=%i\n", __func__, fb, res);
> > >
> > > @@ -194,6 +197,8 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
> > >         if (err)
> > >                 goto out_fb;
> > >
> > > +       trace_fuse_backing_close(fc, backing_id, fb);
> > > +
> >
> > If I'm understanding it correctly, the lines above (added from the
> > previous patch) are
> >
> > + err = ops->may_admin ? ops->may_admin(fc, 0) : 0;
> > + if (err)
> > +       goto out_fb;
> > +
> > + err = ops->may_close ? ops->may_close(fc, fb->file) : 0;
> > + if (err)
> > +        goto out_fb;
>
> That's correct.
>
> > and will also do the close in the out_fb goto. So should the
> > trace_fuse_backing_close() be moved to before the "err =
> > ops->may_admin..." line so it doesn't get missed in the "if (err)..."
> > cases?
>
> No.  If either ->may_admin or ->may_close return nonzero, then this
> function drops the active reference to @fb that it obtained from
> __fuse_backing_lookup without calling fuse_backing_id_remove.
> Therefore, the fuse connection retains its mapping of backing_id to a
> fuse_backing object, which means that nothing is closed.
>
> --D

That makes sense, thanks for the explanation.

Reviewed-by: Joanne Koong <joannelkoong@...il.com>

>
> > Thanks,
> > Joanne
> >
> > >         err = -ENOENT;
> > >         test_fb = fuse_backing_id_remove(fc, backing_id);
> > >         if (!test_fb)
> > >