[PATCH 0/3] libtraceevent

[PATCH 0/3] libtraceevent

[Steven Rostedt]

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

Google's fuzz testing found a double free in process_sizeof(). That was an
easy fix, but the reason the bug happened was because of that silly "ok"
variable called "ok", which is meaningless for what it is used for.

Also, remove the unneeded test of !ok at the end of the if/else block.

Steven Rostedt (Google) (3):
  libtraceevent: Fix double free in parsing sizeof()
  libtraceevent: No need for testing ok in else if (!ok) in process_sizeof()
  libtraceevent: Rename "ok" to "end" in process_sizeof()

 src/event-parse.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

-- 
2.39.1

[PATCH 1/3] libtraceevent: Fix double free in parsing sizeof()

[Steven Rostedt]

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

Google's fuzz testing caught a double free in process_sizeof(). If "ok" is
set, it means that token contains the last part of sizeof() (should be the
')'). Otherwise, the token contains the last item in the parenthesis of
sizeof(), and the next token needs to be read.

The problem is, in this case, the token is read into the token holder
"tok" and not to token. That means the next "free_token()" will free the
token that was already freed and what was just read.

Note, the "ok" variable is a horrible name and needs to be changed, but
that's outside the scope of this update.

Fixes: 2d0573af4dfda ("libtraceevent: Be able to handle some sizeof() calls")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 src/event-parse.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/event-parse.c b/src/event-parse.c
index e655087dad60..2584b3605136 100644
--- a/src/event-parse.c
+++ b/src/event-parse.c
@@ -3591,8 +3591,9 @@ process_sizeof(struct tep_event *event, struct tep_print_arg *arg, char **tok)
 	}
 
 	if (!ok) {
+		/* The token contains the last item before the parenthesis */
 		free_token(token);
-		type = read_token_item(event->tep, tok);
+		type = read_token_item(event->tep, &token);
 	}
 	if (test_type_token(type, token,  TEP_EVENT_DELIM, ")"))
 		goto error;
-- 
2.39.1

[PATCH 2/3] libtraceevent: No need for testing ok in else if (!ok) in process_sizeof()

[Steven Rostedt]

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

The if/else if logic in process_sizeof() has:

	if (ok || strcmp(token, "int") == 0) {
		[..]
	} else if (strcmp(token, "long") == 0) {
		[..]
	} else if (strcmp(token, "REC") == 0) {
		[..]
	} else if (!ok) {
		goto error;
	}

By the time we get to } else if (!ok) {, ok will always be false as if it
were true, it would enter the first if block.

Just make it end with:
	} else {
		goto error;
	}

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 src/event-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/event-parse.c b/src/event-parse.c
index 2584b3605136..4a8b81c33a45 100644
--- a/src/event-parse.c
+++ b/src/event-parse.c
@@ -3586,7 +3586,7 @@ process_sizeof(struct tep_event *event, struct tep_print_arg *arg, char **tok)
 		if (ret < 0)
 			goto error;
 
-	} else if (!ok) {
+	} else {
 		goto error;
 	}
 
-- 
2.39.1

[PATCH 3/3] libtraceevent: Rename "ok" to "end" in process_sizeof()

[Steven Rostedt]

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

The "ok" variable is set to true if at the end of the if/else blocks the
token contains the last element of "sizeof(..)", which would be that ")"
parenthesis. Calling it "ok" is meaningless and confusing.

Call the variable what it is for "token_has_paren". That will make the
logic much easier to understand.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 src/event-parse.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/event-parse.c b/src/event-parse.c
index 4a8b81c33a45..acf7fde4ead9 100644
--- a/src/event-parse.c
+++ b/src/event-parse.c
@@ -3522,7 +3522,7 @@ process_sizeof(struct tep_event *event, struct tep_print_arg *arg, char **tok)
 	struct tep_format_field *field;
 	enum tep_event_type type;
 	char *token = NULL;
-	bool ok = false;
+	bool token_has_paren = false;
 	int ret;
 
 	type = read_token_item(event->tep, &token);
@@ -3537,11 +3537,12 @@ process_sizeof(struct tep_event *event, struct tep_print_arg *arg, char **tok)
 		if (type == TEP_EVENT_ERROR)
 			goto error;
 
+		/* If it's not an item (like "long") then do not process more */
 		if (type != TEP_EVENT_ITEM)
-			ok = true;
+			token_has_paren = true;
 	}
 
-	if (ok || strcmp(token, "int") == 0) {
+	if (token_has_paren || strcmp(token, "int") == 0) {
 		arg->atom.atom = strdup("4");
 
 	} else if (strcmp(token, "long") == 0) {
@@ -3563,7 +3564,7 @@ process_sizeof(struct tep_event *event, struct tep_print_arg *arg, char **tok)
 				goto error;
 			}
 			/* The token is the next token */
-			ok = true;
+			token_has_paren = true;
 		}
 	} else if (strcmp(token, "REC") == 0) {
 
@@ -3590,7 +3591,7 @@ process_sizeof(struct tep_event *event, struct tep_print_arg *arg, char **tok)
 		goto error;
 	}
 
-	if (!ok) {
+	if (!token_has_paren) {
 		/* The token contains the last item before the parenthesis */
 		free_token(token);
 		type = read_token_item(event->tep, &token);
-- 
2.39.1