[PATCH] tracing: use correct string size for snprintf

[PATCH] tracing: use correct string size for snprintf

[Chase Douglas]

The nsecs_str string is a local variable defined as:

char nsecs_str[5];

It is possible for the snprintf call to use a size value larger than the
size of the string. This should not cause a buffer overrun as it is
written now due to the maximum size of the string format. However, this
change makes it correct. By making the size correct we guard against
potential future changes that could actually cause a buffer overrun.

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

diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 79f4bac..73d6bd1 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -641,7 +641,8 @@ trace_print_graph_duration(unsigned long long duration, struct trace_seq *s)
 
 	/* Print nsecs (we don't want to exceed 7 numbers) */
 	if (len < 7) {
-		snprintf(nsecs_str, 8 - len, "%03lu", nsecs_rem);
+		snprintf(nsecs_str, min(sizeof(nsecs_str), 8 - len), "%03lu",
+			 nsecs_rem);
 		ret = trace_seq_printf(s, ".%s", nsecs_str);
 		if (!ret)
 			return TRACE_TYPE_PARTIAL_LINE;
-- 
1.7.0.4

Re: [PATCH] tracing: use correct string size for snprintf

[Steven Rostedt]

On Tue, 2010-06-15 at 12:29 -0400, Chase Douglas wrote:
> The nsecs_str string is a local variable defined as:
> 
> char nsecs_str[5];
> 
> It is possible for the snprintf call to use a size value larger than the
> size of the string. This should not cause a buffer overrun as it is
> written now due to the maximum size of the string format. However, this
> change makes it correct. By making the size correct we guard against
> potential future changes that could actually cause a buffer overrun.
> 
> Cc: stable@kernel.org

I see that because nsecs_str is based off of nsecs_rem which is the
remainder of a division of a 1000, that it wont ever be more than 4 in
length. For this, I'll remove the "Cc: stable" part and queue it for 36.
No urgent need for now.

But I still think it should be added, just because it looks to fragile
to have what is in the kernel now.

Thanks!

-- Steve

> Signed-off-by: Chase Douglas <chase.douglas@canonical.com>
> ---
>  kernel/trace/trace_functions_graph.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index 79f4bac..73d6bd1 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -641,7 +641,8 @@ trace_print_graph_duration(unsigned long long duration, struct trace_seq *s)
>  
>  	/* Print nsecs (we don't want to exceed 7 numbers) */
>  	if (len < 7) {
> -		snprintf(nsecs_str, 8 - len, "%03lu", nsecs_rem);
> +		snprintf(nsecs_str, min(sizeof(nsecs_str), 8 - len), "%03lu",
> +			 nsecs_rem);
>  		ret = trace_seq_printf(s, ".%s", nsecs_str);
>  		if (!ret)
>  			return TRACE_TYPE_PARTIAL_LINE;

[tip:perf/core] tracing/function-graph: Use correct string size for snprintf

[tip-bot for Chase Douglas]

Commit-ID:  d62f85d1e22e537192ce494c89540e1ac0d8bfc7
Gitweb:     http://git.kernel.org/tip/d62f85d1e22e537192ce494c89540e1ac0d8bfc7
Author:     Chase Douglas <chase.douglas@canonical.com>
AuthorDate: Tue, 15 Jun 2010 12:29:15 -0400
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Mon, 28 Jun 2010 21:11:39 -0400

tracing/function-graph: Use correct string size for snprintf

The nsecs_str string is a local variable defined as:

char nsecs_str[5];

It is possible for the snprintf call to use a size value larger than the
size of the string. This should not cause a buffer overrun as it is
written now due to the value for the string format "%03lu" can not be
larger than 1000. However, this change makes it correct. By making the
size correct we guard against potential future changes that could actually
cause a buffer overrun.

Signed-off-by: Chase Douglas <chase.douglas@canonical.com>
LKML-Reference: <1276619355-18116-1-git-send-email-chase.douglas@canonical.com>

[ added 'UL' to number 8 to fix gcc warning comparing it to sizeof() ]

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_functions_graph.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 79f4bac..6bff236 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -641,7 +641,8 @@ trace_print_graph_duration(unsigned long long duration, struct trace_seq *s)
 
 	/* Print nsecs (we don't want to exceed 7 numbers) */
 	if (len < 7) {
-		snprintf(nsecs_str, 8 - len, "%03lu", nsecs_rem);
+		snprintf(nsecs_str, min(sizeof(nsecs_str), 8UL - len), "%03lu",
+			 nsecs_rem);
 		ret = trace_seq_printf(s, ".%s", nsecs_str);
 		if (!ret)
 			return TRACE_TYPE_PARTIAL_LINE;