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 for Android: free password hash cracker in your pocket

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

Message-ID: <20251108002412.GM196391@frogsfrogsfrogs>
Date: Fri, 7 Nov 2025 16:24:12 -0800
From: "Darrick J. Wong" <djwong@...nel.org>
To: Joanne Koong <joannelkoong@...il.com>
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 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

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