Skip to contentSkip to conversation

ftrace/direct: Fix lockup in modify_ftrace_direct_multi

PATCH In review Jiri Olsa 2 participants 3 messages Nov 9, 2021

Subsystems (2)
lore
Share & tools
lore

Jiri Olsa

author
#0 lore raw

PATCH 1/1 ftrace/direct: Fix lockup in modify_ftrace_direct_multi

We can't call unregister_ftrace_function under ftrace_lock.

Fixes ed29271894aa ("ftrace/direct: Do not disable when switching direct callers")
---
 kernel/trace/ftrace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/trace/ftrace.c +2 -1
index b4ed1a301232..fc49e8809a56 100644
@@ -5602,10 +5602,11 @@ int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
 		}
 	}
 
+	mutex_unlock(&ftrace_lock);
+
 	/* Removing the tmp_ops will add the updated direct callers to the functions */
 	unregister_ftrace_function(&tmp_ops);
 
-	mutex_unlock(&ftrace_lock);
  out_direct:
 	mutex_unlock(&direct_mutex);
 	return err;
Signature · 1 line
2.32.0
maintainer
#1 lore raw
Quoted 1 line · On Tue, 9 Nov 2021 12:42:17 +0100 Jiri Olsa <[email protected]> wrote:

We can't call unregister_ftrace_function under ftrace_lock.

Ouch.

Do you have a sample module that triggers this?

I thought we had something that tested this code, but I don't see it in my tree.

-- Steve

Quoted 24 lines
Fixes ed29271894aa ("ftrace/direct: Do not disable when switching direct callers")
---
 kernel/trace/ftrace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/trace/ftrace.c +2 -1
index b4ed1a301232..fc49e8809a56 100644
@@ -5602,10 +5602,11 @@ int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
 		}
 	}
 
+	mutex_unlock(&ftrace_lock);
+
 	/* Removing the tmp_ops will add the updated direct callers to the functions */
 	unregister_ftrace_function(&tmp_ops);
 
-	mutex_unlock(&ftrace_lock);
  out_direct:
 	mutex_unlock(&direct_mutex);
 	return err;
author
#2 lore raw
Quoted 11 lines · Steven Rostedt
Quoted 1 line · On Tue, 9 Nov 2021 12:42:17 +0100 Jiri Olsa <[email protected]> wrote:

We can't call unregister_ftrace_function under ftrace_lock.

Ouch.

Do you have a sample module that triggers this?

I thought we had something that tested this code, but I don't see it in my tree.

I triggered that with my bpf batch changes

there's not test for modify at the moment,
just register/unregister I'll add one

jirka

Quoted 28 lines

-- Steve

Quoted 24 lines
Fixes ed29271894aa ("ftrace/direct: Do not disable when switching direct callers")
---
 kernel/trace/ftrace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/trace/ftrace.c +2 -1
index b4ed1a301232..fc49e8809a56 100644
@@ -5602,10 +5602,11 @@ int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
 		}
 	}
 
+	mutex_unlock(&ftrace_lock);
+
 	/* Removing the tmp_ops will add the updated direct callers to the functions */
 	unregister_ftrace_function(&tmp_ops);
 
-	mutex_unlock(&ftrace_lock);
  out_direct:
 	mutex_unlock(&direct_mutex);
 	return err;