| Message ID | 174343533593.843280.8788277038071990492.stgit@devnote2 (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tracing: fprobe-events: Register fprobe only when the event is enabled | expand |
Oops, I have already sent this as; https://lore.kernel.org/all/174342990779.781946.9138388479067729366.stgit@devnote2/ please ignore this. On Tue, 1 Apr 2025 00:35:36 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote: > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Commit ac91052f0ae5 ("tracing: tprobe-events: Fix leakage of module > refcount") moved try_module_get() from __find_tracepoint_module_cb() > to find_tracepoint() caller, but that introduced a possible UAF > because the module can be unloaded before try_module_get(). In this > case, the module object should be freed too. Thus, try_module_get() > does not only fail but may access to the freed object. > > To avoid that, try_module_get() in __find_tracepoint_module_cb() > again. > > Fixes: ac91052f0ae5 ("tracing: tprobe-events: Fix leakage of module refcount") > Cc: stable@vger.kernel.org > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > --- > kernel/trace/trace_fprobe.c | 26 +++++++++++++++++--------- > 1 file changed, 17 insertions(+), 9 deletions(-) > > diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c > index 985ff98272da..2cd9ff1049f1 100644 > --- a/kernel/trace/trace_fprobe.c > +++ b/kernel/trace/trace_fprobe.c > @@ -919,9 +919,15 @@ static void __find_tracepoint_module_cb(struct tracepoint *tp, struct module *mo > struct __find_tracepoint_cb_data *data = priv; > > if (!data->tpoint && !strcmp(data->tp_name, tp->name)) { > - data->tpoint = tp; > - if (!data->mod) > + /* If module is not specified, try getting module refcount. */ > + if (!data->mod && mod) { > + /* If failed to get refcount, ignore this tracepoint. */ > + if (!try_module_get(mod)) > + return; > + > data->mod = mod; > + } > + data->tpoint = tp; > } > } > > @@ -933,7 +939,11 @@ static void __find_tracepoint_cb(struct tracepoint *tp, void *priv) > data->tpoint = tp; > } > > -/* Find a tracepoint from kernel and module. */ > +/* > + * Find a tracepoint from kernel and module. If the tracepoint is on the module, > + * the module's refcount is incremented and returned as *@tp_mod. Thus, if it is > + * not NULL, caller must call module_put(*tp_mod) after used the tracepoint. > + */ > static struct tracepoint *find_tracepoint(const char *tp_name, > struct module **tp_mod) > { > @@ -962,7 +972,10 @@ static void reenable_trace_fprobe(struct trace_fprobe *tf) > } > } > > -/* Find a tracepoint from specified module. */ > +/* > + * Find a tracepoint from specified module. In this case, this does not get the > + * module's refcount. The caller must ensure the module is not freed. > + */ > static struct tracepoint *find_tracepoint_in_module(struct module *mod, > const char *tp_name) > { > @@ -1169,11 +1182,6 @@ static int trace_fprobe_create_internal(int argc, const char *argv[], > if (is_tracepoint) { > ctx->flags |= TPARG_FL_TPOINT; > tpoint = find_tracepoint(symbol, &tp_mod); > - /* lock module until register this tprobe. */ > - if (tp_mod && !try_module_get(tp_mod)) { > - tpoint = NULL; > - tp_mod = NULL; > - } > if (tpoint) { > ctx->funcname = kallsyms_lookup( > (unsigned long)tpoint->probestub, >
diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c index 985ff98272da..2cd9ff1049f1 100644 --- a/kernel/trace/trace_fprobe.c +++ b/kernel/trace/trace_fprobe.c @@ -919,9 +919,15 @@ static void __find_tracepoint_module_cb(struct tracepoint *tp, struct module *mo struct __find_tracepoint_cb_data *data = priv; if (!data->tpoint && !strcmp(data->tp_name, tp->name)) { - data->tpoint = tp; - if (!data->mod) + /* If module is not specified, try getting module refcount. */ + if (!data->mod && mod) { + /* If failed to get refcount, ignore this tracepoint. */ + if (!try_module_get(mod)) + return; + data->mod = mod; + } + data->tpoint = tp; } } @@ -933,7 +939,11 @@ static void __find_tracepoint_cb(struct tracepoint *tp, void *priv) data->tpoint = tp; } -/* Find a tracepoint from kernel and module. */ +/* + * Find a tracepoint from kernel and module. If the tracepoint is on the module, + * the module's refcount is incremented and returned as *@tp_mod. Thus, if it is + * not NULL, caller must call module_put(*tp_mod) after used the tracepoint. + */ static struct tracepoint *find_tracepoint(const char *tp_name, struct module **tp_mod) { @@ -962,7 +972,10 @@ static void reenable_trace_fprobe(struct trace_fprobe *tf) } } -/* Find a tracepoint from specified module. */ +/* + * Find a tracepoint from specified module. In this case, this does not get the + * module's refcount. The caller must ensure the module is not freed. + */ static struct tracepoint *find_tracepoint_in_module(struct module *mod, const char *tp_name) { @@ -1169,11 +1182,6 @@ static int trace_fprobe_create_internal(int argc, const char *argv[], if (is_tracepoint) { ctx->flags |= TPARG_FL_TPOINT; tpoint = find_tracepoint(symbol, &tp_mod); - /* lock module until register this tprobe. */ - if (tp_mod && !try_module_get(tp_mod)) { - tpoint = NULL; - tp_mod = NULL; - } if (tpoint) { ctx->funcname = kallsyms_lookup( (unsigned long)tpoint->probestub,