Re: [DTrace-devel] [PATCH 1/4] bpf: fix file descriptor leak

On Mon, Nov 10, 2025 at 02:55:52PM -0500, Eugene Loh wrote:
> Reviewed-by: Eugene Loh <[email protected]>
> 
> But I'm curious about the "if (attach)" check.  It looks a little funny that
> we first set fd= and then we check "if (attach)" to see if we are going to
> use the fd value we just set.  I suppose it does not matter since we expect
> attach to be there, but normally one would first check if a value is needed
> before going to the trouble of determining the value.  In a way, this small,
> pre-existing issue is outside the scope of this patch, except that the patch
> makes more obvious the one, isolated place where this temporary value is
> used.
> 
> Up to you whether to clean that up.

There is a real reason for that.  As you can see, fentry-based FBT probes
have their own BPF program load function (the reason a hook was added for
that in providers).  That one performs the attach as part of the program
load, so it does not have an attach hook.

> On 11/10/25 10:27, Kris Van Hees via DTrace-devel wrote:
> > When a BPF program has been attached to a perf event, it is safe to
> > close its file descriptor.
> > 
> > Signed-off-by: Kris Van Hees <[email protected]>
> > ---
> >   libdtrace/dt_bpf.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> > index 0223764a..28eb890e 100644
> > --- a/libdtrace/dt_bpf.c
> > +++ b/libdtrace/dt_bpf.c
> > @@ -1385,8 +1385,8 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
> >   		if (prp->prov->impl->attach)
> >   			rc = prp->prov->impl->attach(dtp, prp, fd);
> > +		close(fd);
> >   		if (rc < 0 && !(prp->flags & DT_PROBE_FLAG_OPTIONAL)) {
> > -			close(fd);
> >   			dt_attach_error(dtp, rc,
> >   					prp->desc->prv, prp->desc->mod,
> >   					prp->desc->fun, prp->desc->prb);