[22/38] trace-cmd dump: prevent buffer overrun in dump_clock()

Commit Message

Jerome Marchand June 5, 2024, 1:40 p.m. UTC

The clock isn't big enough to hold the string with the null
terminating character. Worse, clock[size], which is out of range, is
set to 0. Allocate a big enough buffer.

Fixes an OVERRUN error (CWE-119)

Signed-off-by: Jerome Marchand <[e-mail]>
---
 tracecmd/trace-dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Steven Rostedt July 17, 2024, 10:55 p.m. UTC | #1

Note, please start the subject with a capital letter:

  trace-cmd dump: Prevent buffer overrun in dump_clock()

On Wed,  5 Jun 2024 15:40:37 +0200
"Jerome Marchand" <[e-mail]> wrote:

> The clock isn't big enough to hold the string with the null
> terminating character. Worse, clock[size], which is out of range, is
> set to 0. Allocate a big enough buffer.
> 
> Fixes an OVERRUN error (CWE-119)
> 
> Signed-off-by: Jerome Marchand <[e-mail]>
> ---
>  tracecmd/trace-dump.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tracecmd/trace-dump.c b/tracecmd/trace-dump.c
> index 11c1baf1..c0a282c9 100644
> --- a/tracecmd/trace-dump.c
> +++ b/tracecmd/trace-dump.c
> @@ -961,7 +961,7 @@ static void dump_clock(int fd)
>  	}
>  	if (read_file_number(fd, &size, 8))
>  		die("cannot read clock size");
> -	clock = calloc(1, size);
> +	clock = calloc(1, size+1);

Also we follow the Linux kernel syntax. Please add spaces.

	clock = calloc(1, size + 1);

Care to resend. I'll skip this patch as well.

Thanks,

-- Steve



>  	if (!clock)
>  		die("cannot allocate clock %lld bytes", size);
>

Jerome Marchand Oct. 29, 2024, 6:31 a.m. UTC | #2

On 18/07/2024 00:55, Steven Rostedt wrote:
> 
> Note, please start the subject with a capital letter:
> 
>    trace-cmd dump: Prevent buffer overrun in dump_clock()
> 
> On Wed,  5 Jun 2024 15:40:37 +0200
> "Jerome Marchand" <[e-mail]> wrote:
> 
>> The clock isn't big enough to hold the string with the null
>> terminating character. Worse, clock[size], which is out of range, is
>> set to 0. Allocate a big enough buffer.
>>
>> Fixes an OVERRUN error (CWE-119)
>>
>> Signed-off-by: Jerome Marchand <[e-mail]>
>> ---
>>   tracecmd/trace-dump.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tracecmd/trace-dump.c b/tracecmd/trace-dump.c
>> index 11c1baf1..c0a282c9 100644
>> --- a/tracecmd/trace-dump.c
>> +++ b/tracecmd/trace-dump.c
>> @@ -961,7 +961,7 @@ static void dump_clock(int fd)
>>   	}
>>   	if (read_file_number(fd, &size, 8))
>>   		die("cannot read clock size");
>> -	clock = calloc(1, size);
>> +	clock = calloc(1, size+1);
> 
> Also we follow the Linux kernel syntax. Please add spaces.
> 
> 	clock = calloc(1, size + 1);
> 
> Care to resend. I'll skip this patch as well.

Will do.

Jerome

> 
> Thanks,
> 
> -- Steve
> 
> 
> 
>>   	if (!clock)
>>   		die("cannot allocate clock %lld bytes", size);
>>

Patch

diff --git a/tracecmd/trace-dump.c b/tracecmd/trace-dump.c
index 11c1baf1..c0a282c9 100644
--- a/tracecmd/trace-dump.c
+++ b/tracecmd/trace-dump.c
@@ -961,7 +961,7 @@  static void dump_clock(int fd)
 	}
 	if (read_file_number(fd, &size, 8))
 		die("cannot read clock size");
-	clock = calloc(1, size);
+	clock = calloc(1, size+1);
 	if (!clock)
 		die("cannot allocate clock %lld bytes", size);