[PATCH] libtracecmd: Support changing /proc/kallsyms

[PATCH] libtracecmd: Support changing /proc/kallsyms

[Ilya Leoshkevich]

Running BPF selftests under trace-cmd intermittently fails with:

    error in size of file '/proc/kallsyms'

This is because these selftests load and unload BPF programs.
bpf_prog_put() uses workqueues and RCU, so these programs disappear
from /proc/kallsyms after a delay.

trace-cmd reads /proc/kallsyms twice: the first time to compute its
size, and the second time to copy it into the trace file. If the
resulting sizes don't match, which is what happens in this case,
recording fails.

Fix by first copying /proc/kallsyms into a temporary file, and then
into the trace file. An alternative would be to read it into a
malloc()-ed buffer, but this would increase trace-cmd memory usage,
since /proc/kallsyms can be a few dozen megabytes large. In case
/tmp is tmpfs, both solutions are almost equivalent.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 lib/trace-cmd/trace-output.c | 66 ++++++++++++++++++++++++++++--------
 1 file changed, 52 insertions(+), 14 deletions(-)

diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index 00b87a19..d85021be 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -1129,14 +1129,52 @@ err:
 		tracecmd_warning("can't set kptr_restrict");
 }
 
+static int copy_to_temp_fd(const char *path, tsize_t *size)
+{
+	char temp_path[] = "/tmp/trace_XXXXXX";
+	char buf[BUFSIZ];
+	int fd, temp_fd;
+	stsize_t r;
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		goto fail;
+
+	temp_fd = mkstemp(temp_path);
+	if (temp_fd < 0)
+		goto fail_close;
+	unlink(temp_path);
+
+	*size = 0;
+	for (;;) {
+		r = read(fd, buf, sizeof(buf));
+		if (r == 0)
+			break;
+		if (r < 0 || __do_write_check(temp_fd, buf, r))
+			goto fail_close_temp;
+		*size += r;
+	}
+
+	if (lseek(temp_fd, 0, SEEK_SET) == (off_t)-1)
+		goto fail_close_temp;
+
+	close(fd);
+	return temp_fd;
+
+fail_close_temp:
+	close(temp_fd);
+fail_close:
+	close(fd);
+fail:
+	return -1;
+}
+
 static int read_proc_kallsyms(struct tracecmd_output *handle, bool compress)
 {
 	enum tracecmd_section_flags flags = 0;
-	unsigned int size, check_size, endian4;
 	const char *path = "/proc/kallsyms";
-	tsize_t offset;
-	struct stat st;
-	int ret;
+	tsize_t check_size, offset, size;
+	int endian4, fd, ret;
 
 	if (!tcmd_check_out_state(handle, TRACECMD_FILE_KALLSYMS)) {
 		tracecmd_warning("Cannot read kallsyms, unexpected state 0x%X",
@@ -1155,36 +1193,36 @@ static int read_proc_kallsyms(struct tracecmd_output *handle, bool compress)
 		return -1;
 
 	tcmd_out_compression_start(handle, compress);
-	ret = stat(path, &st);
-	if (ret < 0) {
+	set_proc_kptr_restrict(0);
+	fd = copy_to_temp_fd(path, &size);
+	set_proc_kptr_restrict(1);
+	if (fd < 0) {
 		/* not found */
 		size = 0;
 		endian4 = convert_endian_4(handle, size);
 		ret = tcmd_do_write_check(handle, &endian4, 4);
 		goto out;
 	}
-	size = get_size(path);
 	endian4 = convert_endian_4(handle, size);
 	ret = tcmd_do_write_check(handle, &endian4, 4);
 	if (ret)
-		goto out;
+		goto out_close;
 
-	set_proc_kptr_restrict(0);
-	check_size = copy_file(handle, path);
+	check_size = copy_file_fd(handle, fd, 0);
 	if (size != check_size) {
 		errno = EINVAL;
 		tracecmd_warning("error in size of file '%s'", path);
-		set_proc_kptr_restrict(1);
 		ret = -1;
-		goto out;
+		goto out_close;
 	}
-	set_proc_kptr_restrict(1);
 
 	ret = tcmd_out_compression_end(handle, compress);
 	if (ret)
-		goto out;
+		goto out_close;
 
 	ret = tcmd_out_update_section_header(handle, offset);
+out_close:
+	close(fd);
 out:
 	if (!ret)
 		handle->file_state = TRACECMD_FILE_KALLSYMS;
-- 
2.49.0

Re: [PATCH] libtracecmd: Support changing /proc/kallsyms

[Ilya Leoshkevich]

On Thu, 2025-04-17 at 01:13 +0200, Ilya Leoshkevich wrote:
> Running BPF selftests under trace-cmd intermittently fails with:
> 
>     error in size of file '/proc/kallsyms'
> 
> This is because these selftests load and unload BPF programs.
> bpf_prog_put() uses workqueues and RCU, so these programs disappear
> from /proc/kallsyms after a delay.
> 
> trace-cmd reads /proc/kallsyms twice: the first time to compute its
> size, and the second time to copy it into the trace file. If the
> resulting sizes don't match, which is what happens in this case,
> recording fails.
> 
> Fix by first copying /proc/kallsyms into a temporary file, and then
> into the trace file. An alternative would be to read it into a
> malloc()-ed buffer, but this would increase trace-cmd memory usage,
> since /proc/kallsyms can be a few dozen megabytes large. In case
> /tmp is tmpfs, both solutions are almost equivalent.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  lib/trace-cmd/trace-output.c | 66 ++++++++++++++++++++++++++++------
> --
>  1 file changed, 52 insertions(+), 14 deletions(-)

Gentle ping.

Re: [PATCH] libtracecmd: Support changing /proc/kallsyms

[Steven Rostedt]

On Wed, 14 May 2025 09:51:16 +0200
Ilya Leoshkevich <iii@linux.ibm.com> wrote:

> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >  lib/trace-cmd/trace-output.c | 66 ++++++++++++++++++++++++++++------
> > --
> >  1 file changed, 52 insertions(+), 14 deletions(-)  
> 
> Gentle ping.

Thanks for the reminder. This came in just as I finished working on the
libraries, which I try to spend a little time on every month.

I'm about to leave for a short vacation, but hopefully I'll be able to
get to this next week.

If you don't see anything by the end of next week, please send me
another gentle ping ;-)

If I can catch up on everything I need to get done today, maybe I'll be
able to look at it today. But I don't want to get your hopes up.

-- Steve

Re: [PATCH] libtracecmd: Support changing /proc/kallsyms

[Steven Rostedt]

On Thu, 17 Apr 2025 01:13:00 +0200
Ilya Leoshkevich <iii@linux.ibm.com> wrote:

I finally got some time to look at this. Sorry for the delay.

> Running BPF selftests under trace-cmd intermittently fails with:
> 
>     error in size of file '/proc/kallsyms'
> 
> This is because these selftests load and unload BPF programs.
> bpf_prog_put() uses workqueues and RCU, so these programs disappear
> from /proc/kallsyms after a delay.
> 
> trace-cmd reads /proc/kallsyms twice: the first time to compute its
> size, and the second time to copy it into the trace file. If the
> resulting sizes don't match, which is what happens in this case,
> recording fails.
> 
> Fix by first copying /proc/kallsyms into a temporary file, and then
> into the trace file. An alternative would be to read it into a
> malloc()-ed buffer, but this would increase trace-cmd memory usage,
> since /proc/kallsyms can be a few dozen megabytes large. In case
> /tmp is tmpfs, both solutions are almost equivalent.

Actually, when compression is set, the file is already read into memory,
and then it is compressed. In fact, it's allocated twice!

The size is copied into this temp buffer. I wonder if we could just update
the size if it is different?

__hidden long long
tcmd_do_write_check(struct tracecmd_output *handle, const void *data, long long size)
{
	if (handle->do_compress)
		return tracecmd_compress_buffer_write(handle->compress, data, size);

	if (handle->msg_handle)
		return tracecmd_msg_data_send(handle->msg_handle, data, size);

	return __do_write_check(handle->fd, data, size);
}

Now for the "do_compress" we can just update the handle->buffer[] as the
size is the first thing written into it.

For the __do_write_check() we can save the file descriptor location and go
back and update what was written.

For the "->msg_handle", for now just error out. Unless you are running bpf
programs while using trace-cmd on guests or sending off the network?

-- Steve

Re: [PATCH] libtracecmd: Support changing /proc/kallsyms

[Ilya Leoshkevich]

On 2025-05-30 22:45, Steven Rostedt wrote:
> On Thu, 17 Apr 2025 01:13:00 +0200
> Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> 
> I finally got some time to look at this. Sorry for the delay.
> 
>> Running BPF selftests under trace-cmd intermittently fails with:
>> 
>>     error in size of file '/proc/kallsyms'
>> 
>> This is because these selftests load and unload BPF programs.
>> bpf_prog_put() uses workqueues and RCU, so these programs disappear
>> from /proc/kallsyms after a delay.
>> 
>> trace-cmd reads /proc/kallsyms twice: the first time to compute its
>> size, and the second time to copy it into the trace file. If the
>> resulting sizes don't match, which is what happens in this case,
>> recording fails.
>> 
>> Fix by first copying /proc/kallsyms into a temporary file, and then
>> into the trace file. An alternative would be to read it into a
>> malloc()-ed buffer, but this would increase trace-cmd memory usage,
>> since /proc/kallsyms can be a few dozen megabytes large. In case
>> /tmp is tmpfs, both solutions are almost equivalent.
> 
> Actually, when compression is set, the file is already read into 
> memory,
> and then it is compressed. In fact, it's allocated twice!
> 
> The size is copied into this temp buffer. I wonder if we could just 
> update
> the size if it is different?
> 
> __hidden long long
> tcmd_do_write_check(struct tracecmd_output *handle, const void *data,
> long long size)
> {
> 	if (handle->do_compress)
> 		return tracecmd_compress_buffer_write(handle->compress, data, size);
> 
> 	if (handle->msg_handle)
> 		return tracecmd_msg_data_send(handle->msg_handle, data, size);
> 
> 	return __do_write_check(handle->fd, data, size);
> }
> 
> Now for the "do_compress" we can just update the handle->buffer[] as 
> the
> size is the first thing written into it.
> 
> For the __do_write_check() we can save the file descriptor location and 
> go
> back and update what was written.
> 
> For the "->msg_handle", for now just error out. Unless you are running 
> bpf
> programs while using trace-cmd on guests or sending off the network?
> 
> -- Steve

Thanks for taking a look.
What you propose should work for me.
Would you mind if I make do_lseek() non-static for this?
And maybe rename it to tcmd_do_lseek().
Touching do_compress/msg_handle/pointer/fd directly feels hackish.

Re: [PATCH] libtracecmd: Support changing /proc/kallsyms

[Steven Rostedt]

On Mon, 02 Jun 2025 17:05:13 +0200
Ilya Leoshkevich <iii@linux.ibm.com> wrote:

> Thanks for taking a look.
> What you propose should work for me.
> Would you mind if I make do_lseek() non-static for this?
> And maybe rename it to tcmd_do_lseek().
> Touching do_compress/msg_handle/pointer/fd directly feels hackish.

Sure, I thought it was a bit hacky too.

-- Steve