Skip to content

Commit 3cc4e2c

Browse files
zegao96mhiramat
authored andcommitted
fprobe: make fprobe_kprobe_handler recursion free
Current implementation calls kprobe related functions before doing ftrace recursion check in fprobe_kprobe_handler, which opens door to kernel crash due to stack recursion if preempt_count_{add, sub} is traceable in kprobe_busy_{begin, end}. Things goes like this without this patch quoted from Steven: " fprobe_kprobe_handler() { kprobe_busy_begin() { preempt_disable() { preempt_count_add() { <-- trace fprobe_kprobe_handler() { [ wash, rinse, repeat, CRASH!!! ] " By refactoring the common part out of fprobe_kprobe_handler and fprobe_handler and call ftrace recursion detection at the very beginning, the whole fprobe_kprobe_handler is free from recursion. [ Fix the indentation of __fprobe_handler() parameters. ] Link: https://lore.kernel.org/all/[email protected]/ Fixes: ab51e15 ("fprobe: Introduce FPROBE_FL_KPROBE_SHARED flag for fprobe") Signed-off-by: Ze Gao <[email protected]> Acked-by: Masami Hiramatsu (Google) <[email protected]> Cc: [email protected] Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
1 parent be243ba commit 3cc4e2c

File tree

1 file changed

+44
-15
lines changed

1 file changed

+44
-15
lines changed

kernel/trace/fprobe.c

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,30 +20,22 @@ struct fprobe_rethook_node {
2020
char data[];
2121
};
2222

23-
static void fprobe_handler(unsigned long ip, unsigned long parent_ip,
24-
struct ftrace_ops *ops, struct ftrace_regs *fregs)
23+
static inline void __fprobe_handler(unsigned long ip, unsigned long parent_ip,
24+
struct ftrace_ops *ops, struct ftrace_regs *fregs)
2525
{
2626
struct fprobe_rethook_node *fpr;
2727
struct rethook_node *rh = NULL;
2828
struct fprobe *fp;
2929
void *entry_data = NULL;
30-
int bit, ret = 0;
30+
int ret = 0;
3131

3232
fp = container_of(ops, struct fprobe, ops);
33-
if (fprobe_disabled(fp))
34-
return;
35-
36-
bit = ftrace_test_recursion_trylock(ip, parent_ip);
37-
if (bit < 0) {
38-
fp->nmissed++;
39-
return;
40-
}
4133

4234
if (fp->exit_handler) {
4335
rh = rethook_try_get(fp->rethook);
4436
if (!rh) {
4537
fp->nmissed++;
46-
goto out;
38+
return;
4739
}
4840
fpr = container_of(rh, struct fprobe_rethook_node, node);
4941
fpr->entry_ip = ip;
@@ -61,23 +53,60 @@ static void fprobe_handler(unsigned long ip, unsigned long parent_ip,
6153
else
6254
rethook_hook(rh, ftrace_get_regs(fregs), true);
6355
}
64-
out:
56+
}
57+
58+
static void fprobe_handler(unsigned long ip, unsigned long parent_ip,
59+
struct ftrace_ops *ops, struct ftrace_regs *fregs)
60+
{
61+
struct fprobe *fp;
62+
int bit;
63+
64+
fp = container_of(ops, struct fprobe, ops);
65+
if (fprobe_disabled(fp))
66+
return;
67+
68+
/* recursion detection has to go before any traceable function and
69+
* all functions before this point should be marked as notrace
70+
*/
71+
bit = ftrace_test_recursion_trylock(ip, parent_ip);
72+
if (bit < 0) {
73+
fp->nmissed++;
74+
return;
75+
}
76+
__fprobe_handler(ip, parent_ip, ops, fregs);
6577
ftrace_test_recursion_unlock(bit);
78+
6679
}
6780
NOKPROBE_SYMBOL(fprobe_handler);
6881

6982
static void fprobe_kprobe_handler(unsigned long ip, unsigned long parent_ip,
7083
struct ftrace_ops *ops, struct ftrace_regs *fregs)
7184
{
72-
struct fprobe *fp = container_of(ops, struct fprobe, ops);
85+
struct fprobe *fp;
86+
int bit;
87+
88+
fp = container_of(ops, struct fprobe, ops);
89+
if (fprobe_disabled(fp))
90+
return;
91+
92+
/* recursion detection has to go before any traceable function and
93+
* all functions called before this point should be marked as notrace
94+
*/
95+
bit = ftrace_test_recursion_trylock(ip, parent_ip);
96+
if (bit < 0) {
97+
fp->nmissed++;
98+
return;
99+
}
73100

74101
if (unlikely(kprobe_running())) {
75102
fp->nmissed++;
76103
return;
77104
}
105+
78106
kprobe_busy_begin();
79-
fprobe_handler(ip, parent_ip, ops, fregs);
107+
__fprobe_handler(ip, parent_ip, ops, fregs);
80108
kprobe_busy_end();
109+
ftrace_test_recursion_unlock(bit);
81110
}
82111

83112
static void fprobe_exit_handler(struct rethook_node *rh, void *data,

0 commit comments

Comments
 (0)