linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Build and fuzzing related fixes
@ 2021-06-12  1:44 Ian Rogers
  2021-06-12  1:45 ` [PATCH 1/2] libtraceevent: Add eof checks Ian Rogers
  2021-06-12  1:45 ` [PATCH 2/2] libtraceevent: Changed angled brackets to double quotes Ian Rogers
  0 siblings, 2 replies; 11+ messages in thread
From: Ian Rogers @ 2021-06-12  1:44 UTC (permalink / raw)
  To: linux-trace-devel, Tzvetomir Stoyanov, Steven Rostedt, Claire Jensen
  Cc: Ian Rogers

EOF checks are missing in a number of cases that cause the parser to
enter an infinite loop if an EOF is encountered.
Some build systems are picky about angled vs quotes, fix this minor issue.

Claire Jensen (2):
  libtraceevent: Add eof checks.
  libtraceevent: Changed angled brackets to double quotes.

 src/event-parse.c | 62 ++++++++++++++++++++++++++++++++++++-----------
 src/event-utils.h |  2 +-
 2 files changed, 49 insertions(+), 15 deletions(-)

-- 
2.32.0.272.g935e593368-goog


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

* [PATCH 1/2] libtraceevent: Add eof checks.
  2021-06-12  1:44 [PATCH 0/2] Build and fuzzing related fixes Ian Rogers
@ 2021-06-12  1:45 ` Ian Rogers
  2021-06-13 23:30   ` Steven Rostedt
  2021-06-12  1:45 ` [PATCH 2/2] libtraceevent: Changed angled brackets to double quotes Ian Rogers
  1 sibling, 1 reply; 11+ messages in thread
From: Ian Rogers @ 2021-06-12  1:45 UTC (permalink / raw)
  To: linux-trace-devel, Tzvetomir Stoyanov, Steven Rostedt, Claire Jensen
  Cc: Ian Rogers

From: Claire Jensen <cjense@google.com>

Added checking for __read_char and peek_char to make sure value is not at end
of file.

Signed-off-by: Claire Jensen <cjense@google.com>
Signed-off-by: Ian Rogers <irogers@google.com>
---
 src/event-parse.c | 62 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 48 insertions(+), 14 deletions(-)

diff --git a/src/event-parse.c b/src/event-parse.c
index 9915cb4..ac11887 100644
--- a/src/event-parse.c
+++ b/src/event-parse.c
@@ -1155,17 +1155,16 @@ static enum tep_event_type force_token(const char *str, char **tok);
 static enum tep_event_type __read_token(char **tok)
 {
 	char buf[BUFSIZ];
-	int ch, last_ch, quote_ch, next_ch;
+	int ch, last_ch, quote_ch, next_ch, read_ch, peek_ch;
 	int i = 0;
 	int tok_size = 0;
 	enum tep_event_type type;
 
 	*tok = NULL;
 
-
-	ch = __read_char();
+        ch = __read_char();
 	if (ch < 0)
-		return TEP_EVENT_NONE;
+		goto out_eof_error;
 
 	type = get_type(ch);
 	if (type == TEP_EVENT_NONE)
@@ -1184,9 +1183,15 @@ static enum tep_event_type __read_token(char **tok)
 	case TEP_EVENT_OP:
 		switch (ch) {
 		case '-':
-			next_ch = peek_char();
+			peek_ch = peek_char();
+			if (peek_ch < 0)
+				goto out_eof_error;
+			next_ch = peek_ch;
 			if (next_ch == '>') {
-				buf[i++] = __read_char();
+				read_ch = __read_char();
+				if (read_ch < 0)
+					goto out_eof_error;
+				buf[i++] = read_ch;
 				break;
 			}
 			/* fall through */
@@ -1197,9 +1202,14 @@ static enum tep_event_type __read_token(char **tok)
 		case '<':
 			last_ch = ch;
 			ch = peek_char();
+			if (ch < 0)
+				goto out_eof_error;
 			if (ch != last_ch)
 				goto test_equal;
-			buf[i++] = __read_char();
+			read_ch = __read_char();
+			if (read_ch < 0)
+				goto out_eof_error;
+			buf[i++] = read_ch;
 			switch (last_ch) {
 			case '>':
 			case '<':
@@ -1219,10 +1229,17 @@ static enum tep_event_type __read_token(char **tok)
 		return type;
 
  test_equal:
-		ch = peek_char();
-		if (ch == '=')
-			buf[i++] = __read_char();
-		goto out;
+		peek_ch = peek_char();
+		if (peek_ch < 0)
+			goto out_eof_error;
+		ch = peek_ch;
+		if (ch == '=') {
+			read_ch = __read_char();
+			if (read_ch < 0)
+				goto out_eof_error;
+			buf[i++] = read_ch;
+			goto out;
+		}
 
 	case TEP_EVENT_DQUOTE:
 	case TEP_EVENT_SQUOTE:
@@ -1242,6 +1259,8 @@ static enum tep_event_type __read_token(char **tok)
 			}
 			last_ch = ch;
 			ch = __read_char();
+			if(ch < 0)
+				goto out_eof_error;
 			buf[i++] = ch;
 			/* the '\' '\' will cancel itself */
 			if (ch == '\\' && last_ch == '\\')
@@ -1259,6 +1278,8 @@ static enum tep_event_type __read_token(char **tok)
 
 			do {
 				ch = __read_char();
+				if(ch < 0)
+					return TEP_EVENT_NONE;
 			} while (isspace(ch));
 			if (ch == '"')
 				goto concat;
@@ -1273,7 +1294,13 @@ static enum tep_event_type __read_token(char **tok)
 		break;
 	}
 
-	while (get_type(peek_char()) == type) {
+	while (1) {
+		peek_ch = peek_char();
+		if (peek_ch < 0)
+			goto out_eof_error;
+		if (get_type(peek_ch) != type)
+			break;
+
 		if (i == (BUFSIZ - 1)) {
 			buf[i] = 0;
 			tok_size += BUFSIZ;
@@ -1282,8 +1309,10 @@ static enum tep_event_type __read_token(char **tok)
 				return TEP_EVENT_NONE;
 			i = 0;
 		}
-		ch = __read_char();
-		buf[i++] = ch;
+		read_ch = __read_char();
+		if (read_ch < 0)
+			goto out_eof_error;
+		buf[i++] = read_ch;
 	}
 
  out:
@@ -1316,6 +1345,11 @@ static enum tep_event_type __read_token(char **tok)
 	}
 
 	return type;
+
+out_eof_error:
+	free(*tok);
+	*tok = NULL;
+	return TEP_EVENT_NONE;
 }
 
 static enum tep_event_type force_token(const char *str, char **tok)
-- 
2.32.0.272.g935e593368-goog


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

* [PATCH 2/2] libtraceevent: Changed angled brackets to double quotes.
  2021-06-12  1:44 [PATCH 0/2] Build and fuzzing related fixes Ian Rogers
  2021-06-12  1:45 ` [PATCH 1/2] libtraceevent: Add eof checks Ian Rogers
@ 2021-06-12  1:45 ` Ian Rogers
  2021-06-12 23:00   ` Steven Rostedt
  1 sibling, 1 reply; 11+ messages in thread
From: Ian Rogers @ 2021-06-12  1:45 UTC (permalink / raw)
  To: linux-trace-devel, Tzvetomir Stoyanov, Steven Rostedt, Claire Jensen
  Cc: Ian Rogers

From: Claire Jensen <cjense@google.com>

Signed-off-by: Claire Jensen <cjense@google.com>
Signed-off-by: Ian Rogers <irogers@google.com>
---
 src/event-utils.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/event-utils.h b/src/event-utils.h
index d377201..44f7968 100644
--- a/src/event-utils.h
+++ b/src/event-utils.h
@@ -10,7 +10,7 @@
 #include <stdarg.h>
 #include <stdbool.h>
 
-#include <event-parse.h>
+#include "event-parse.h"
 
 void tep_warning(const char *fmt, ...);
 void tep_info(const char *fmt, ...);
-- 
2.32.0.272.g935e593368-goog


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

* Re: [PATCH 2/2] libtraceevent: Changed angled brackets to double quotes.
  2021-06-12  1:45 ` [PATCH 2/2] libtraceevent: Changed angled brackets to double quotes Ian Rogers
@ 2021-06-12 23:00   ` Steven Rostedt
  2021-06-13 21:31     ` Ian Rogers
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2021-06-12 23:00 UTC (permalink / raw)
  To: Ian Rogers; +Cc: linux-trace-devel, Tzvetomir Stoyanov, Claire Jensen

On Fri, 11 Jun 2021 18:45:01 -0700
Ian Rogers <irogers@google.com> wrote:

> From: Claire Jensen <cjense@google.com>
> 
> Signed-off-by: Claire Jensen <cjense@google.com>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  src/event-utils.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/event-utils.h b/src/event-utils.h
> index d377201..44f7968 100644
> --- a/src/event-utils.h
> +++ b/src/event-utils.h
> @@ -10,7 +10,7 @@
>  #include <stdarg.h>
>  #include <stdbool.h>
>  
> -#include <event-parse.h>
> +#include "event-parse.h"

Thanks for the update, but event-parse.h is part of the exported
library and even though it happens to be local to this file, we use the
'<' and '>' to denote that it is a library header and not a local one.

-- Steve


>  
>  void tep_warning(const char *fmt, ...);
>  void tep_info(const char *fmt, ...);


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

* Re: [PATCH 2/2] libtraceevent: Changed angled brackets to double quotes.
  2021-06-12 23:00   ` Steven Rostedt
@ 2021-06-13 21:31     ` Ian Rogers
  2021-06-13 23:29       ` Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: Ian Rogers @ 2021-06-13 21:31 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Linux Trace Devel, Tzvetomir Stoyanov, Claire Jensen

On Sat, Jun 12, 2021 at 4:00 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri, 11 Jun 2021 18:45:01 -0700
> Ian Rogers <irogers@google.com> wrote:
>
> > From: Claire Jensen <cjense@google.com>
> >
> > Signed-off-by: Claire Jensen <cjense@google.com>
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  src/event-utils.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/event-utils.h b/src/event-utils.h
> > index d377201..44f7968 100644
> > --- a/src/event-utils.h
> > +++ b/src/event-utils.h
> > @@ -10,7 +10,7 @@
> >  #include <stdarg.h>
> >  #include <stdbool.h>
> >
> > -#include <event-parse.h>
> > +#include "event-parse.h"
>
> Thanks for the update, but event-parse.h is part of the exported
> library and even though it happens to be local to this file, we use the
> '<' and '>' to denote that it is a library header and not a local one.
>
> -- Steve

Would it be more conventional in this situation to use quotes within
the library and angles outside? This change is within the library and
not with code trying to link against it.

Thanks,
Ian

>
> >
> >  void tep_warning(const char *fmt, ...);
> >  void tep_info(const char *fmt, ...);
>

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

* Re: [PATCH 2/2] libtraceevent: Changed angled brackets to double quotes.
  2021-06-13 21:31     ` Ian Rogers
@ 2021-06-13 23:29       ` Steven Rostedt
  2021-06-14 16:14         ` Ian Rogers
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2021-06-13 23:29 UTC (permalink / raw)
  To: Ian Rogers; +Cc: Linux Trace Devel, Tzvetomir Stoyanov, Claire Jensen

On Sun, 13 Jun 2021 14:31:42 -0700
Ian Rogers <irogers@google.com> wrote:


> Would it be more conventional in this situation to use quotes within
> the library and angles outside? This change is within the library and
> not with code trying to link against it.

Does it matter? If not, what's the purpose of changing it?

Note, the change log is empty, and is where rationale for the change is
suppose to be added.

-- Steve

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

* Re: [PATCH 1/2] libtraceevent: Add eof checks.
  2021-06-12  1:45 ` [PATCH 1/2] libtraceevent: Add eof checks Ian Rogers
@ 2021-06-13 23:30   ` Steven Rostedt
       [not found]     ` <CAFPGG2iQK8XMv6Z1-KurgjOnYuk=m=uWNWJXj6OEb_SBQkokZA@mail.gmail.com>
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2021-06-13 23:30 UTC (permalink / raw)
  To: Ian Rogers; +Cc: linux-trace-devel, Tzvetomir Stoyanov, Claire Jensen

On Fri, 11 Jun 2021 18:45:00 -0700
Ian Rogers <irogers@google.com> wrote:

> From: Claire Jensen <cjense@google.com>
> 
> Added checking for __read_char and peek_char to make sure value is not at end
> of file.

Was there a case where this happened?

-- Steve

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

* Re: [PATCH 2/2] libtraceevent: Changed angled brackets to double quotes.
  2021-06-13 23:29       ` Steven Rostedt
@ 2021-06-14 16:14         ` Ian Rogers
  2021-06-14 18:43           ` Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: Ian Rogers @ 2021-06-14 16:14 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Linux Trace Devel, Tzvetomir Stoyanov, Claire Jensen

On Sun, Jun 13, 2021 at 4:29 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Sun, 13 Jun 2021 14:31:42 -0700
> Ian Rogers <irogers@google.com> wrote:
>
>
> > Would it be more conventional in this situation to use quotes within
> > the library and angles outside? This change is within the library and
> > not with code trying to link against it.
>
> Does it matter? If not, what's the purpose of changing it?

Sorry for not having this in the body, in the cover message I added to
Claire's email with:

> Some build systems are picky about angled vs quotes, fix this minor issue.

The problem is with certain build system, we use bazel at Google,
where iquote is used for includes within the library, and other
flavors outside the library. There's a similar conversation here:
https://groups.google.com/g/bazel-discuss/c/6MNuZ5bKoa8?pli=1

the failure you see when building is:

In file included from src/event-parse.c:27:
src/event-utils.h:13:10: error: 'event-parse.h' file not found with
<angled> include; use "quotes" instead
#include <event-parse.h>
         ^~~~~~~~~~~~~~~
         "event-parse.h"

Arguably -iquote is more correct within the library build, hence bazel
using it. The libtraceevent Makefile is using -I, hence this not being
a problem for others. We prefer not to carry internal patches, hence
wanting to send this upstream.

Thanks,
Ian

> Note, the change log is empty, and is where rationale for the change is
> suppose to be added.
>
> -- Steve

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

* Re: [PATCH 2/2] libtraceevent: Changed angled brackets to double quotes.
  2021-06-14 16:14         ` Ian Rogers
@ 2021-06-14 18:43           ` Steven Rostedt
  0 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2021-06-14 18:43 UTC (permalink / raw)
  To: Ian Rogers; +Cc: Linux Trace Devel, Tzvetomir Stoyanov, Claire Jensen

On Mon, 14 Jun 2021 09:14:08 -0700
Ian Rogers <irogers@google.com> wrote:

> On Sun, Jun 13, 2021 at 4:29 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Sun, 13 Jun 2021 14:31:42 -0700
> > Ian Rogers <irogers@google.com> wrote:
> >
> >  
> > > Would it be more conventional in this situation to use quotes within
> > > the library and angles outside? This change is within the library and
> > > not with code trying to link against it.  
> >
> > Does it matter? If not, what's the purpose of changing it?  
> 
> Sorry for not having this in the body, in the cover message I added to
> Claire's email with:
> 
> > Some build systems are picky about angled vs quotes, fix this minor issue.  
> 
> The problem is with certain build system, we use bazel at Google,
> where iquote is used for includes within the library, and other
> flavors outside the library. There's a similar conversation here:
> https://groups.google.com/g/bazel-discuss/c/6MNuZ5bKoa8?pli=1
> 
> the failure you see when building is:
> 
> In file included from src/event-parse.c:27:
> src/event-utils.h:13:10: error: 'event-parse.h' file not found with
> <angled> include; use "quotes" instead
> #include <event-parse.h>
>          ^~~~~~~~~~~~~~~
>          "event-parse.h"
> 
> Arguably -iquote is more correct within the library build, hence bazel
> using it. The libtraceevent Makefile is using -I, hence this not being
> a problem for others. We prefer not to carry internal patches, hence
> wanting to send this upstream.

OK, context is very important :-)

With the above explanation, it makes sense, and I'm OK with taking it.
Please resend with the above in the commit log.

Thanks!

-- Steve

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

* Re: [PATCH 1/2] libtraceevent: Add eof checks.
       [not found]     ` <CAFPGG2iQK8XMv6Z1-KurgjOnYuk=m=uWNWJXj6OEb_SBQkokZA@mail.gmail.com>
@ 2021-06-17 19:01       ` Steven Rostedt
  0 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2021-06-17 19:01 UTC (permalink / raw)
  To: Claire Jensen; +Cc: Ian Rogers, linux-trace-devel, Tzvetomir Stoyanov

On Thu, 17 Jun 2021 13:56:49 -0500
Claire Jensen <cjense@google.com> wrote:

> Hi Steve,
> 
> This issue was found while fuzz testing. One of the test cases created an
> infinite loop because __read_token had reached end of file. Checking was
> added to all cases where this may occur.
> 
> Sorry for the late reply, I just got an issue surrounding email permissions
> resolved so I was unable to respond earlier.
> 
>

Can you resend the patch with this information in the change log.

Thanks!

-- Steve

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

* [PATCH 2/2] libtraceevent: Changed angled brackets to double quotes.
  2021-06-17 19:43 [PATCH 1/2] libtraceevent: Add eof checks Claire Jensen
@ 2021-06-17 19:43 ` Claire Jensen
  0 siblings, 0 replies; 11+ messages in thread
From: Claire Jensen @ 2021-06-17 19:43 UTC (permalink / raw)
  To: eranian, irogers, tz.stoyanov, linux-trace-devel, rostedt; +Cc: Claire Jensen

Certain build systems, such as bazel, are picky about angled brackets vs
double quotes.

the failure you see when building is:

In file included from src/event-parse.c:27:
src/event-utils.h:13:10: error: 'event-parse.h' file not found with
<angled> include; use "quotes" instead
 #include <event-parse.h>
         ^~~~~~~~~~~~~~~
         "event-parse.h"

Arguably -iquote is more correct within the library build, hence bazel
using it. The libtraceevent Makefile is using -I, hence this not being
a problem for others.

Signed-off-by: Claire Jensen <cjense@google.com>
---
 src/event-utils.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/event-utils.h b/src/event-utils.h
index d377201..44f7968 100644
--- a/src/event-utils.h
+++ b/src/event-utils.h
@@ -10,7 +10,7 @@
 #include <stdarg.h>
 #include <stdbool.h>
 
-#include <event-parse.h>
+#include "event-parse.h"
 
 void tep_warning(const char *fmt, ...);
 void tep_info(const char *fmt, ...);
-- 
2.32.0.288.g62a8d224e6-goog


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

end of thread, other threads:[~2021-06-17 19:43 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-12  1:44 [PATCH 0/2] Build and fuzzing related fixes Ian Rogers
2021-06-12  1:45 ` [PATCH 1/2] libtraceevent: Add eof checks Ian Rogers
2021-06-13 23:30   ` Steven Rostedt
     [not found]     ` <CAFPGG2iQK8XMv6Z1-KurgjOnYuk=m=uWNWJXj6OEb_SBQkokZA@mail.gmail.com>
2021-06-17 19:01       ` Steven Rostedt
2021-06-12  1:45 ` [PATCH 2/2] libtraceevent: Changed angled brackets to double quotes Ian Rogers
2021-06-12 23:00   ` Steven Rostedt
2021-06-13 21:31     ` Ian Rogers
2021-06-13 23:29       ` Steven Rostedt
2021-06-14 16:14         ` Ian Rogers
2021-06-14 18:43           ` Steven Rostedt
2021-06-17 19:43 [PATCH 1/2] libtraceevent: Add eof checks Claire Jensen
2021-06-17 19:43 ` [PATCH 2/2] libtraceevent: Changed angled brackets to double quotes Claire Jensen

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).