linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] libtracefs: Some fixes for sqlhis
@ 2022-08-19  2:03 Steven Rostedt
  2022-08-19  2:03 ` [PATCH 1/3] libtracefs: Fix use after free in tracefs_synth_alloc() Steven Rostedt
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Steven Rostedt @ 2022-08-19  2:03 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

While playing with sqlhist, I found three bugs.

 Two dealing with freeing issues.

 One where the matching is more restrictive than the kernel.

Steven Rostedt (Google) (3):
  libtracefs: Fix use after free in tracefs_synth_alloc()
  libtracefs: Remove double free attempt of new_event in
    tracefs_synth_echo_cmd()
  libtracefs sqlhist: Allow pointers to match longs

 src/tracefs-hist.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] libtracefs: Fix use after free in tracefs_synth_alloc()
  2022-08-19  2:03 [PATCH 0/3] libtracefs: Some fixes for sqlhis Steven Rostedt
@ 2022-08-19  2:03 ` Steven Rostedt
  2022-08-19  2:03 ` [PATCH 2/3] libtracefs: Remove double free attempt of new_event in tracefs_synth_echo_cmd() Steven Rostedt
  2022-08-19  2:03 ` [PATCH 3/3] libtracefs sqlhist: Allow pointers to match longs Steven Rostedt
  2 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2022-08-19  2:03 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The synth new_format is set after the error condition is checked and the
synth is freed (on error), causing a SIGSEV when that occurs.

Fixes: 74a6754b9e67b ("libtracefs: Check README to know if we should do old onmatch format")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 src/tracefs-hist.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index 2f12cc471294..6f7d657bd404 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -1091,9 +1091,8 @@ struct tracefs_synth *tracefs_synth_alloc(struct tep_handle *tep,
 	if (!synth->name || !synth->start_keys || !synth->end_keys || ret) {
 		tracefs_synth_free(synth);
 		synth = NULL;
-	}
-
-	synth->new_format = has_new_format();
+	} else
+		synth->new_format = has_new_format();
 
 	return synth;
 }
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] libtracefs: Remove double free attempt of new_event in tracefs_synth_echo_cmd()
  2022-08-19  2:03 [PATCH 0/3] libtracefs: Some fixes for sqlhis Steven Rostedt
  2022-08-19  2:03 ` [PATCH 1/3] libtracefs: Fix use after free in tracefs_synth_alloc() Steven Rostedt
@ 2022-08-19  2:03 ` Steven Rostedt
  2022-08-19  2:03 ` [PATCH 3/3] libtracefs sqlhist: Allow pointers to match longs Steven Rostedt
  2 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2022-08-19  2:03 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The "out_free" path frees the synth->dyn_event if new_event is set, but it
is also freed in the success path (that falls through into the out_free
path). The only reason this did not crash is because both cases set the
"synth->dyn_event" to NULL, where the second attempt to free it does
nothing. But this is still a bug.

Fixes: d7c5dbb7a231e ("libtracefs: Use the internal dynamic events API when creating synthetic events")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 src/tracefs-hist.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index 6f7d657bd404..302b9a75e6ee 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -2311,11 +2311,6 @@ int tracefs_synth_echo_cmd(struct trace_seq *seq,
 			 hist, path, synth->end_event->system,
 			 synth->end_event->name);
 
-	if (new_event) {
-		tracefs_dynevent_free(synth->dyn_event);
-		synth->dyn_event = NULL;
-	}
-
 	ret = 0;
  out_free:
 	free(hist);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] libtracefs sqlhist: Allow pointers to match longs
  2022-08-19  2:03 [PATCH 0/3] libtracefs: Some fixes for sqlhis Steven Rostedt
  2022-08-19  2:03 ` [PATCH 1/3] libtracefs: Fix use after free in tracefs_synth_alloc() Steven Rostedt
  2022-08-19  2:03 ` [PATCH 2/3] libtracefs: Remove double free attempt of new_event in tracefs_synth_echo_cmd() Steven Rostedt
@ 2022-08-19  2:03 ` Steven Rostedt
  2 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2022-08-19  2:03 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Currently, if a field that is a long tries to match a field that is a
pointer, tracefs_sqlhist() will fail with incompatible fields. But the
kernel will still allow it to match, do not fail here.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 src/tracefs-hist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index 302b9a75e6ee..14988d8dc185 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -796,6 +796,7 @@ static bool verify_event_fields(struct tep_event *start_event,
 {
 	const struct tep_format_field *start_field;
 	const struct tep_format_field *end_field;
+	int start_flags, end_flags;
 
 	if (!trace_verify_event_field(start_event, start_field_name,
 				      &start_field))
@@ -806,7 +807,11 @@ static bool verify_event_fields(struct tep_event *start_event,
 					      &end_field))
 			return false;
 
-		if (start_field->flags != end_field->flags ||
+		/* A pointer can still match a long */
+		start_flags = start_field->flags & ~TEP_FIELD_IS_POINTER;
+		end_flags = end_field->flags & ~TEP_FIELD_IS_POINTER;
+
+		if (start_flags != end_flags ||
 		    start_field->size != end_field->size) {
 			errno = EBADE;
 			return false;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-08-19  2:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-19  2:03 [PATCH 0/3] libtracefs: Some fixes for sqlhis Steven Rostedt
2022-08-19  2:03 ` [PATCH 1/3] libtracefs: Fix use after free in tracefs_synth_alloc() Steven Rostedt
2022-08-19  2:03 ` [PATCH 2/3] libtracefs: Remove double free attempt of new_event in tracefs_synth_echo_cmd() Steven Rostedt
2022-08-19  2:03 ` [PATCH 3/3] libtracefs sqlhist: Allow pointers to match longs Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).