[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aSBuv8Ab0y8SeAHS@krava>
Date: Fri, 21 Nov 2025 14:53:03 +0100
From: Jiri Olsa <olsajiri@...il.com>
To: bot+bpf-ci@...nel.org
Cc: rostedt@...nel.org, revest@...gle.com, mark.rutland@....com,
bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-trace-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, ast@...nel.org,
daniel@...earbox.net, andrii@...nel.org, menglong8.dong@...il.com,
song@...nel.org, martin.lau@...nel.org, eddyz87@...il.com,
yonghong.song@...ux.dev, clm@...a.com, ihor.solodrai@...ux.dev
Subject: Re: [PATCHv3 bpf-next 5/8] ftrace: Add update_ftrace_direct_mod
function
On Thu, Nov 20, 2025 at 10:23:47PM +0000, bot+bpf-ci@...nel.org wrote:
> > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> > index c571deeff..4e2c0ed76 100644
> > --- a/include/linux/ftrace.h
> > +++ b/include/linux/ftrace.h
>
> [ ... ]
>
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index cc730a8fd..5243aefb6 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
>
> [ ... ]
>
> > @@ -6486,6 +6486,74 @@ int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
> > return err;
> > }
> >
> > +int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash *hash, bool do_direct_lock)
> > +{
> > + struct ftrace_hash *orig_hash = ops->func_hash->filter_hash;
> ^^^^^^^^^^^^^^^^
>
> Can this crash if ops->func_hash is NULL? In update_ftrace_direct_add()
> and update_ftrace_direct_del(), there's a defensive check:
>
> old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
>
> But update_ftrace_direct_mod() dereferences ops->func_hash before any
> validation checks occur. The check_direct_multi(ops) validation happens
> later, but the dereference occurs first at function entry.
update_ftrace_direct_add is called before update_ftrace_direct_mod which
ensures the existence of the filter_hash, but same as for the update_ftrace_direct_del
case in the other reply, it's probably better to add the check for future
thanks,
jirka
>
> > + struct ftrace_func_entry *entry, *tmp;
> > + static struct ftrace_ops tmp_ops = {
> > + .func = ftrace_stub,
> > + .flags = FTRACE_OPS_FL_STUB,
> > + };
> > + unsigned long size, i;
> > + int err;
> > +
> > + if (!hash_count(hash))
> > + return -EINVAL;
> > + if (check_direct_multi(ops))
> > + return -EINVAL;
>
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19552032346
Powered by blists - more mailing lists