Skip to contentSkip to conversation

FAILED: patch "[PATCH] tracing: Check pid filtering when creating events" failed to apply to 5.10-stable tree

PATCH [email protected] 2 participants 5 messages Nov 28, 2021 – Nov 29, 2021

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 6cb206508b621a9a0a2c35b60540e399225c8243 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (VMware)" <[email protected]>
Date: Fri, 26 Nov 2021 13:35:26 -0500
Subject: [PATCH] tracing: Check pid filtering when creating events

When pid filtering is activated in an instance, all of the events trace files for that instance has the PID_FILTER flag set. This determines whether or not pid filtering needs to be done on the event, otherwise the event is executed as normal.

If pid filtering is enabled when an event is created (via a dynamic event or modules), its flag is not updated to reflect the current state, and the events are not filtered properly.

Fixes 3fdaf80f4a836 ("tracing: Implement event pid filtering")
kernel/trace/trace_events.c +12 -0
index 4021b9a79f93..f8965fd50d3b 100644
@@ -2678,12 +2678,24 @@ static struct trace_event_file *
 trace_create_new_event(struct trace_event_call *call,
 		       struct trace_array *tr)
 {
+	struct trace_pid_list *no_pid_list;
+	struct trace_pid_list *pid_list;
 	struct trace_event_file *file;
+	unsigned int first;
 
 	file = kmem_cache_alloc(file_cachep, GFP_TRACE);
 	if (!file)
 		return NULL;
 
+	pid_list = rcu_dereference_protected(tr->filtered_pids,
+					     lockdep_is_held(&event_mutex));
+	no_pid_list = rcu_dereference_protected(tr->filtered_no_pids,
+					     lockdep_is_held(&event_mutex));
+
+	if (!trace_pid_list_first(pid_list, &first) ||
+	    !trace_pid_list_first(pid_list, &first))
+		file->flags |= EVENT_FILE_FL_PID_FILTER;
+
 	file->event_call = call;
 	file->tr = tr;
 	atomic_set(&file->sm_ref, 0);
maintainer
#1 lore raw
Quoted 4 lines · On Sun, 28 Nov 2021 12:44:01 +0100 <[email protected]> wrote:

The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

This is a funny case, where I actually found the bug in the 5.15 kernel that I was using to do my cut and paste screen shots for the upcoming Embedded Fest conference I'll be giving a talk at on Friday.

I wrote this fix for 5.15 first (to continue working on my presentation), and since stable requires upstream to have the fix, I forward ported it to 5.16-rc. As 5.16 introduced changes in this code, the forward port had to do things differently, and my forward port added a bug which the fix has just been added:

https://git.kernel.org/torvalds/c/f8132d62a2deedca1b7558028cfe72f93ad5ba2d

That fix does not need to be backported, as the origin 5.15 patch did not have that bug.

This is the original 5.15 patch I used to create the 5.16 version with, and it should also fix 5.10. Kernels before 5.7 will need a different patch, as it did not have the "no_pid" feature yet. I'll send out the fix for those kernels later.

-- Steve

Upstream commit: 6cb206508b621a9a0a2c35b60540e399225c8243

From: "Steven Rostedt (VMware)" <[email protected]>
Subject: [PATCH] tracing: Check pid filtering when creating events

When pid filtering is activated in an instance, all of the events trace files for that instance has the PID_FILTER flag set. This determines whether or not pid filtering needs to be done on the event, otherwise the event is executed as normal.

If pid filtering is enabled when an event is created (via a dynamic event or modules), its flag is not updated to reflect the current state, and the events are not filtered properly.

Fixes 3fdaf80f4a836 ("tracing: Implement event pid filtering")
---
 kernel/trace/trace_events.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
kernel/trace/trace_events.c +11 -0
index 830b3b9940f4..e760e078c733 100644
@@ -2677,12 +2677,23 @@ static struct trace_event_file *
 trace_create_new_event(struct trace_event_call *call,
 		       struct trace_array *tr)
 {
+	struct trace_pid_list *no_pid_list;
+	struct trace_pid_list *pid_list;
 	struct trace_event_file *file;
+	unsigned int first;
 
 	file = kmem_cache_alloc(file_cachep, GFP_TRACE);
 	if (!file)
 		return NULL;
 
+	pid_list = rcu_dereference_protected(tr->filtered_pids,
+					     lockdep_is_held(&event_mutex));
+	no_pid_list = rcu_dereference_protected(tr->filtered_no_pids,
+					     lockdep_is_held(&event_mutex));
+
+	if (pid_list || no_pid_list)
+		file->flags |= EVENT_FILE_FL_PID_FILTER;
+
 	file->event_call = call;
 	file->tr = tr;
 	atomic_set(&file->sm_ref, 0);
Signature · 1 line
2.33.0
maintainer
#2 lore raw

Review static struct trace_event_file * · line 2684

Quoted 10 lines · On Sun, 28 Nov 2021 13:48:05 -0500 Steven Rostedt <[email protected]> wrote:

+++ b/kernel/trace/trace_events.c

(quoted diff) +3 -0
@@ -2677,12 +2677,23 @@ static struct trace_event_file *
 trace_create_new_event(struct trace_event_call *call,
 		       struct trace_array *tr)
 {
+	struct trace_pid_list *no_pid_list;
+	struct trace_pid_list *pid_list;
 	struct trace_event_file *file;
+	unsigned int first;
 

And even though this was the first fix, I somehow got a bit of the 5.16 patch in here, as the "first" variable is only used for that.

Take two:

From: "Steven Rostedt (VMware)" <[email protected]>
Subject: [PATCH] tracing: Check pid filtering when creating events

When pid filtering is activated in an instance, all of the events trace files for that instance has the PID_FILTER flag set. This determines whether or not pid filtering needs to be done on the event, otherwise the event is executed as normal.

If pid filtering is enabled when an event is created (via a dynamic event or modules), its flag is not updated to reflect the current state, and the events are not filtered properly.

Fixes 3fdaf80f4a836 ("tracing: Implement event pid filtering")
---
 kernel/trace/trace_events.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
kernel/trace/trace_events.c +10 -0
Index: linux-test.git/kernel/trace/trace_events.c
===================================================================
@@ -2462,12 +2462,22 @@ static struct trace_event_file *
 trace_create_new_event(struct trace_event_call *call,
 		       struct trace_array *tr)
 {
+	struct trace_pid_list *no_pid_list;
+	struct trace_pid_list *pid_list;
 	struct trace_event_file *file;
 
 	file = kmem_cache_alloc(file_cachep, GFP_TRACE);
 	if (!file)
 		return NULL;
 
+	pid_list = rcu_dereference_protected(tr->filtered_pids,
+					     lockdep_is_held(&event_mutex));
+	no_pid_list = rcu_dereference_protected(tr->filtered_no_pids,
+					     lockdep_is_held(&event_mutex));
+
+	if (pid_list || no_pid_list)
+		file->flags |= EVENT_FILE_FL_PID_FILTER;
+
 	file->event_call = call;
 	file->tr = tr;
 	atomic_set(&file->sm_ref, 0);
author maintainer
#3 lore raw
Quoted 55 lines · Steven Rostedt

kernel/trace/trace_events.c static struct trace_event_file * · line 2684

Quoted 10 lines · On Sun, 28 Nov 2021 13:48:05 -0500 Steven Rostedt <[email protected]> wrote:

+++ b/kernel/trace/trace_events.c

kernel/trace/trace_events.c +3 -0
@@ -2677,12 +2677,23 @@ static struct trace_event_file *
 trace_create_new_event(struct trace_event_call *call,
 		       struct trace_array *tr)
 {
+	struct trace_pid_list *no_pid_list;
+	struct trace_pid_list *pid_list;
 	struct trace_event_file *file;
+	unsigned int first;
 

And even though this was the first fix, I somehow got a bit of the 5.16 patch in here, as the "first" variable is only used for that.

Take two:

From: "Steven Rostedt (VMware)" <[email protected]>
Subject: [PATCH] tracing: Check pid filtering when creating events

When pid filtering is activated in an instance, all of the events trace files for that instance has the PID_FILTER flag set. This determines whether or not pid filtering needs to be done on the event, otherwise the event is executed as normal.

If pid filtering is enabled when an event is created (via a dynamic event or modules), its flag is not updated to reflect the current state, and the events are not filtered properly.

Fixes 3fdaf80f4a836 ("tracing: Implement event pid filtering")
---
 kernel/trace/trace_events.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Index: linux-test.git/kernel/trace/trace_events.c
===================================================================

kernel/trace/trace_events.c +3 -0
@@ -2462,12 +2462,22 @@ static struct trace_event_file *
 trace_create_new_event(struct trace_event_call *call,
 		       struct trace_array *tr)
 {
+	struct trace_pid_list *no_pid_list;
+	struct trace_pid_list *pid_list;
 	struct trace_event_file *file;
 
 	file = kmem_cache_alloc(file_cachep, GFP_TRACE);
 	if (!file)
 		return NULL;
 
+	pid_list = rcu_dereference_protected(tr->filtered_pids,

kernel/trace/trace_events.c static struct trace_event_file * · line 2483

Quoted 10 lines
kernel/trace/trace_events.c +7 -0
+					     lockdep_is_held(&event_mutex));
+	no_pid_list = rcu_dereference_protected(tr->filtered_no_pids,
+					     lockdep_is_held(&event_mutex));
+
+	if (pid_list || no_pid_list)
+		file->flags |= EVENT_FILE_FL_PID_FILTER;
+
 	file->event_call = call;
 	file->tr = tr;
 	atomic_set(&file->sm_ref, 0);

Oops, now I see this one, thanks...

greg "read the whole thread before reponding" k-h

author maintainer
#4 lore raw
In reply to Steven Rostedt
Quoted 81 lines · Steven Rostedt
Quoted 4 lines · On Sun, 28 Nov 2021 12:44:01 +0100 <[email protected]> wrote:

The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

This is a funny case, where I actually found the bug in the 5.15 kernel that I was using to do my cut and paste screen shots for the upcoming Embedded Fest conference I'll be giving a talk at on Friday.

I wrote this fix for 5.15 first (to continue working on my presentation), and since stable requires upstream to have the fix, I forward ported it to 5.16-rc. As 5.16 introduced changes in this code, the forward port had to do things differently, and my forward port added a bug which the fix has just been added:

https://git.kernel.org/torvalds/c/f8132d62a2deedca1b7558028cfe72f93ad5ba2d

That fix does not need to be backported, as the origin 5.15 patch did not have that bug.

This is the original 5.15 patch I used to create the 5.16 version with, and it should also fix 5.10. Kernels before 5.7 will need a different patch, as it did not have the "no_pid" feature yet. I'll send out the fix for those kernels later.

-- Steve

Upstream commit: 6cb206508b621a9a0a2c35b60540e399225c8243

From: "Steven Rostedt (VMware)" <[email protected]>
Subject: [PATCH] tracing: Check pid filtering when creating events

When pid filtering is activated in an instance, all of the events trace files for that instance has the PID_FILTER flag set. This determines whether or not pid filtering needs to be done on the event, otherwise the event is executed as normal.

If pid filtering is enabled when an event is created (via a dynamic event or modules), its flag is not updated to reflect the current state, and the events are not filtered properly.

Fixes 3fdaf80f4a836 ("tracing: Implement event pid filtering")
---
 kernel/trace/trace_events.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
kernel/trace/trace_events.c +11 -0
index 830b3b9940f4..e760e078c733 100644
@@ -2677,12 +2677,23 @@ static struct trace_event_file *
 trace_create_new_event(struct trace_event_call *call,
 		       struct trace_array *tr)
 {
+	struct trace_pid_list *no_pid_list;
+	struct trace_pid_list *pid_list;
 	struct trace_event_file *file;
+	unsigned int first;
 
 	file = kmem_cache_alloc(file_cachep, GFP_TRACE);
 	if (!file)
 		return NULL;
 
+	pid_list = rcu_dereference_protected(tr->filtered_pids,
+					     lockdep_is_held(&event_mutex));
+	no_pid_list = rcu_dereference_protected(tr->filtered_no_pids,
+					     lockdep_is_held(&event_mutex));
+
+	if (pid_list || no_pid_list)
+		file->flags |= EVENT_FILE_FL_PID_FILTER;
+
 	file->event_call = call;
 	file->tr = tr;
 	atomic_set(&file->sm_ref, 0);
Signature · 1 line
2.33.0

Why did you add the first variable here?

I get the build failure:

kernel/trace/trace_events.c: In function ‘trace_create_new_event’:
kernel/trace/trace_events.c:2684:22: error: unused variable ‘first’ [-Werror=unused-variable]
2684 | unsigned int first;
| ^~~~~
cc1: all warnings being treated as errors

because of that.

I'll go remove that line, but please, test-build your patches at the very least :)

thanks,

greg k-h