[PATCH v2] trace-cmd: prevent print_graph_duration buffer overflow

[PATCH v2] trace-cmd: prevent print_graph_duration buffer overflow

[Chase Douglas]

Passing n > sizeof(string) to snprintf can cause a glibc buffer overflow
condition. We know the exact size of nsecs_str, so use it along with the
the math to determine the longest string size we want.

Note that an overflow isn't really possible given the format of the
string. However, glibc would abort due to a runtime check.

Signed-off-by: Chase Douglas <chase.douglas@canonical.com>
---
 trace-ftrace.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/trace-ftrace.c b/trace-ftrace.c
index af9ac8d..181a00f 100644
--- a/trace-ftrace.c
+++ b/trace-ftrace.c
@@ -21,6 +21,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/param.h>
 
 #include "trace-cmd.h"
 
@@ -148,7 +149,7 @@ static void print_graph_duration(struct trace_seq *s, unsigned long long duratio
 
 	/* Print nsecs (we don't want to exceed 7 numbers) */
 	if ((s->len - len) < 7) {
-		snprintf(nsecs_str, 8 - (s->len - len), "%03lu", nsecs_rem);
+		snprintf(nsecs_str, MIN(sizeof(nsecs_str), 8 - len), "%03lu", nsecs_rem);
 		trace_seq_printf(s, ".%s", nsecs_str);
 	}
 
-- 
1.7.0.4

Re: [PATCH v2] trace-cmd: prevent print_graph_duration buffer overflow

[Steven Rostedt]

Quick note. Please send to my rostedt@goodmis.org address. I may not see
messages here for weeks at a time. I author patches with my RH account
just to "advertise" who I work for. But I sign-off-by with the account I
want people to send to.


On Tue, 2010-06-15 at 10:47 -0400, Chase Douglas wrote:
> Passing n > sizeof(string) to snprintf can cause a glibc buffer overflow
> condition. We know the exact size of nsecs_str, so use it along with the
> the math to determine the longest string size we want.
> 
> Note that an overflow isn't really possible given the format of the
> string. However, glibc would abort due to a runtime check.
> 
> Signed-off-by: Chase Douglas <chase.douglas@canonical.com>

Thanks!

Can you write a similar patch for the Linux kernel too. It may need to
go to stable as well.

-- Steve

> ---
>  trace-ftrace.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/trace-ftrace.c b/trace-ftrace.c
> index af9ac8d..181a00f 100644
> --- a/trace-ftrace.c
> +++ b/trace-ftrace.c
> @@ -21,6 +21,7 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <sys/param.h>
>  
>  #include "trace-cmd.h"
>  
> @@ -148,7 +149,7 @@ static void print_graph_duration(struct trace_seq *s, unsigned long long duratio
>  
>  	/* Print nsecs (we don't want to exceed 7 numbers) */
>  	if ((s->len - len) < 7) {
> -		snprintf(nsecs_str, 8 - (s->len - len), "%03lu", nsecs_rem);
> +		snprintf(nsecs_str, MIN(sizeof(nsecs_str), 8 - len), "%03lu", nsecs_rem);
>  		trace_seq_printf(s, ".%s", nsecs_str);
>  	}
>  

Re: [PATCH v2] trace-cmd: prevent print_graph_duration buffer overflow

[Chase Douglas]

On Tue, 2010-06-15 at 10:54 -0400, Steven Rostedt wrote:
> Quick note. Please send to my rostedt@goodmis.org address. I may not see
> messages here for weeks at a time. I author patches with my RH account
> just to "advertise" who I work for. But I sign-off-by with the account I
> want people to send to.

Ahh. I took the email address from the authors line like you said :). Is
this standard practice for others as well?

> On Tue, 2010-06-15 at 10:47 -0400, Chase Douglas wrote:
> > Passing n > sizeof(string) to snprintf can cause a glibc buffer overflow
> > condition. We know the exact size of nsecs_str, so use it along with the
> > the math to determine the longest string size we want.
> > 
> > Note that an overflow isn't really possible given the format of the
> > string. However, glibc would abort due to a runtime check.
> > 
> > Signed-off-by: Chase Douglas <chase.douglas@canonical.com>
> 
> Thanks!
> 
> Can you write a similar patch for the Linux kernel too. It may need to
> go to stable as well.

Sure

-- Chase

Re: [PATCH v2] trace-cmd: prevent print_graph_duration buffer overflow

[Steven Rostedt]

On Tue, 2010-06-15 at 11:21 -0400, Chase Douglas wrote:
> On Tue, 2010-06-15 at 10:54 -0400, Steven Rostedt wrote:
> > Quick note. Please send to my rostedt@goodmis.org address. I may not see
> > messages here for weeks at a time. I author patches with my RH account
> > just to "advertise" who I work for. But I sign-off-by with the account I
> > want people to send to.
> 
> Ahh. I took the email address from the authors line like you said :). Is
> this standard practice for others as well?

Probably not, but I like to confuse people ;-)

-- Steve