Skip to content

Commit dd94150

Browse files
committed
tracing: fprobe events: Fix possible UAF on modules
Commit ac91052 ("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. Link: https://lore.kernel.org/all/174342990779.781946.9138388479067729366.stgit@devnote2/ Fixes: ac91052 ("tracing: tprobe-events: Fix leakage of module refcount") Cc: [email protected] Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
1 parent d24fa97 commit dd94150

File tree

1 file changed

+17
-9
lines changed

1 file changed

+17
-9
lines changed

kernel/trace/trace_fprobe.c

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -919,9 +919,15 @@ static void __find_tracepoint_module_cb(struct tracepoint *tp, struct module *mo
919919
struct __find_tracepoint_cb_data *data = priv;
920920

921921
if (!data->tpoint && !strcmp(data->tp_name, tp->name)) {
922-
data->tpoint = tp;
923-
if (!data->mod)
922+
/* If module is not specified, try getting module refcount. */
923+
if (!data->mod && mod) {
924+
/* If failed to get refcount, ignore this tracepoint. */
925+
if (!try_module_get(mod))
926+
return;
927+
924928
data->mod = mod;
929+
}
930+
data->tpoint = tp;
925931
}
926932
}
927933

@@ -933,7 +939,11 @@ static void __find_tracepoint_cb(struct tracepoint *tp, void *priv)
933939
data->tpoint = tp;
934940
}
935941

936-
/* Find a tracepoint from kernel and module. */
942+
/*
943+
* Find a tracepoint from kernel and module. If the tracepoint is on the module,
944+
* the module's refcount is incremented and returned as *@tp_mod. Thus, if it is
945+
* not NULL, caller must call module_put(*tp_mod) after used the tracepoint.
946+
*/
937947
static struct tracepoint *find_tracepoint(const char *tp_name,
938948
struct module **tp_mod)
939949
{
@@ -962,7 +972,10 @@ static void reenable_trace_fprobe(struct trace_fprobe *tf)
962972
}
963973
}
964974

965-
/* Find a tracepoint from specified module. */
975+
/*
976+
* Find a tracepoint from specified module. In this case, this does not get the
977+
* module's refcount. The caller must ensure the module is not freed.
978+
*/
966979
static struct tracepoint *find_tracepoint_in_module(struct module *mod,
967980
const char *tp_name)
968981
{
@@ -1169,11 +1182,6 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
11691182
if (is_tracepoint) {
11701183
ctx->flags |= TPARG_FL_TPOINT;
11711184
tpoint = find_tracepoint(symbol, &tp_mod);
1172-
/* lock module until register this tprobe. */
1173-
if (tp_mod && !try_module_get(tp_mod)) {
1174-
tpoint = NULL;
1175-
tp_mod = NULL;
1176-
}
11771185
if (tpoint) {
11781186
ctx->funcname = kallsyms_lookup(
11791187
(unsigned long)tpoint->probestub,

0 commit comments

Comments
 (0)