diff mbox series

[v3,2/4] trace-cmd: Use the new trace buffers API

Message ID 20211111150805.86419-3-tz.stoyanov@gmail.com (mailing list archive)
State Accepted
Headers show
Series Refactor the logic for writing trace data in the file | expand

Commit Message

Tzvetomir Stoyanov (VMware) Nov. 11, 2021, 3:08 p.m. UTC 
When recording trace buffers, use the newly introduced library API for
describing the buffers.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 .../include/private/trace-cmd-private.h          |  2 --
 lib/trace-cmd/trace-output.c                     |  7 +++----
 tracecmd/trace-record.c                          | 16 +++++-----------
 3 files changed, 8 insertions(+), 17 deletions(-)

Comments

Steven Rostedt Nov. 24, 2021, 5:19 p.m. UTC | #1 
On Thu, 11 Nov 2021 17:08:03 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -4169,7 +4169,6 @@ static void touch_file(const char *file)
>  }
>  
>  static void append_buffer(struct tracecmd_output *handle,
> -			  struct tracecmd_option *buffer_option,
>  			  struct buffer_instance *instance,
>  			  char **temp_files)
>  {
> @@ -4197,7 +4196,7 @@ static void append_buffer(struct tracecmd_output *handle,
>  			touch_file(temp_files[i]);
>  	}
>  
> -	tracecmd_append_buffer_cpu_data(handle, buffer_option,
> +	tracecmd_append_buffer_cpu_data(handle, NULL,
>  					cpu_count, temp_files);
>  
>  	for (i = 0; i < instance->cpu_count; i++) {

Where we have:

int tracecmd_append_buffer_cpu_data(struct tracecmd_output *handle,
				    struct tracecmd_option *option,
				    int cpus, char * const *cpu_data_files)
{
	tsize_t offset;
	stsize_t ret;

	offset = lseek64(handle->fd, 0, SEEK_CUR);

	/* Go to the option data, where will write the offest */
	ret = lseek64(handle->fd, option->offset, SEEK_SET);


Now we dereference a NULL pointer because option is NULL here.

I know that this gets fixed in later patches, but we should not have a
known null pointer dereference in git history.

-- Steve
diff mbox series

Patch

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index c0a07045..47358c4b 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -290,8 +290,6 @@  struct tracecmd_option *
 tracecmd_add_option_v(struct tracecmd_output *handle,
 		      unsigned short id, const struct iovec *vector, int count);
 
-struct tracecmd_option *tracecmd_add_buffer_option(struct tracecmd_output *handle,
-						   const char *name, int cpus);
 int tracecmd_add_buffer_info(struct tracecmd_output *handle, const char *name, int cpus);
 int tracecmd_write_buffer_info(struct tracecmd_output *handle);
 
diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index f0f6aeed..1f144d28 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -1334,9 +1334,8 @@  int tracecmd_append_options(struct tracecmd_output *handle)
 	return 0;
 }
 
-struct tracecmd_option *
-tracecmd_add_buffer_option(struct tracecmd_output *handle, const char *name,
-			   int cpus)
+static struct tracecmd_option *
+add_buffer_option(struct tracecmd_output *handle, const char *name, int cpus)
 {
 	struct tracecmd_option *option;
 	char *buf;
@@ -1387,7 +1386,7 @@  int tracecmd_write_buffer_info(struct tracecmd_output *handle)
 	struct tracecmd_buffer *buf;
 
 	list_for_each_entry(buf, &handle->buffers, list) {
-		option = tracecmd_add_buffer_option(handle, buf->name, buf->cpus);
+		option = add_buffer_option(handle, buf->name, buf->cpus);
 		if (!option)
 			return -1;
 		buf->option = option;
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 15e07cf0..c96dcda0 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -4169,7 +4169,6 @@  static void touch_file(const char *file)
 }
 
 static void append_buffer(struct tracecmd_output *handle,
-			  struct tracecmd_option *buffer_option,
 			  struct buffer_instance *instance,
 			  char **temp_files)
 {
@@ -4197,7 +4196,7 @@  static void append_buffer(struct tracecmd_output *handle,
 			touch_file(temp_files[i]);
 	}
 
-	tracecmd_append_buffer_cpu_data(handle, buffer_option,
+	tracecmd_append_buffer_cpu_data(handle, NULL,
 					cpu_count, temp_files);
 
 	for (i = 0; i < instance->cpu_count; i++) {
@@ -4482,7 +4481,6 @@  error:
 
 static void record_data(struct common_record_context *ctx)
 {
-	struct tracecmd_option **buffer_options;
 	struct tracecmd_output *handle;
 	struct buffer_instance *instance;
 	bool local = false;
@@ -4551,9 +4549,6 @@  static void record_data(struct common_record_context *ctx)
 		}
 
 		if (buffers) {
-			buffer_options = malloc(sizeof(*buffer_options) * buffers);
-			if (!buffer_options)
-				die("Failed to allocate buffer options");
 			i = 0;
 			for_each_instance(instance) {
 				int cpus = instance->cpu_count != local_cpu_count ?
@@ -4561,10 +4556,9 @@  static void record_data(struct common_record_context *ctx)
 
 				if (instance->msg_handle)
 					continue;
-
-				buffer_options[i++] = tracecmd_add_buffer_option(handle,
-										 tracefs_instance_get_name(instance->tracefs),
-										 cpus);
+				tracecmd_add_buffer_info(handle,
+							tracefs_instance_get_name(instance->tracefs),
+							cpus);
 				add_buffer_stat(handle, instance);
 			}
 		}
@@ -4599,7 +4593,7 @@  static void record_data(struct common_record_context *ctx)
 				if (instance->msg_handle)
 					continue;
 				print_stat(instance);
-				append_buffer(handle, buffer_options[i++], instance, temp_files);
+				append_buffer(handle, instance, temp_files);
 			}
 		}