Re: [PATCH bpf-next 1/2] selftests/bpf: add helpers to manipulate ftrace

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 3178 bytes --]

> selftests/bpf: add helpers to manipulate ftrace

TL;DR: Restoring ftrace filters in trace_helpers.c may abort on a saved line
over 126 bytes (128-byte buffer), leaving the filter empty, so all functions
get traced and global ftrace state isn't restored.

> diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
> index 6d7a4b8c4ab8..1f26a057a810 100644
> --- a/tools/testing/selftests/bpf/trace_helpers.c
> +++ b/tools/testing/selftests/bpf/trace_helpers.c

[ ... ]

> +#define CHUNK_SIZE	128

[ ... ]

> +static const char *tracefs_next_line(const char *content, char *buf, size_t size,
> +				     int *err)
> +{
> +	const char *end;
> +	size_t len;
> +
> +	*err = 0;
> +	while (*content) {
> +		end = strchrnul(content, '\n');
> +		len = end - content;
> +		content = *end ? end + 1 : end;
> +
> +		if (!len || end[-len] == '#')
> +			continue;
> +
> +		if (len + 2 > size) {
> +			*err = -ENAMETOOLONG;
> +			return NULL;
> +		}
> +		memcpy(buf, end - len, len);
> +		buf[len++] = '\n';
> +		buf[len] = '\0';
> +		return content;
> +	}
> +
> +	return NULL;
> +}
> +
> +static int tracefs_restore_file(const struct tracefs_ctl_entry *entry)
> +{
> +	char buf[CHUNK_SIZE];
> +	const char *content = entry->old;
> +	int err = 0;
> +
> +	if (!content)
> +		return 0;
> +
> +	if (entry->kind == TRACEFS_CTL_FILTER) {
> +		err = tracefs_write_file(entry->path, "");
> +		if (err)
> +			return err;
> +	}
> +
> +	while ((content = tracefs_next_line(content, buf, sizeof(buf), &err))) {
> +		err = tracefs_write_file(entry->path, buf);
> +		if (err)
> +			return err;
> +	}
> +
> +	return err;
> +}

Can a saved line longer than 126 bytes abort the restore here?

tracefs_restore_file() splits the saved content into a buffer sized by the
read chunk constant:

	char buf[CHUNK_SIZE];     /* CHUNK_SIZE is 128 */

and tracefs_next_line() fails the whole restore with -ENAMETOOLONG when
len + 2 > size.  The kernel can print much longer entries than that.
print_rec() emits the full symbol (KSYM_NAME_LEN is 512) plus the
" [module]" suffix, so mangled Rust symbols or long C names with a module
suffix can exceed 126 bytes.

For TRACEFS_CTL_FILTER entries the file has already been emptied by
tracefs_write_file(entry->path, "") before the long line is reached:

	if (entry->kind == TRACEFS_CTL_FILTER) {
		err = tracefs_write_file(entry->path, "");
		...
	}
	while ((content = tracefs_next_line(content, buf, sizeof(buf), &err))) {

so the restore stops part-way, every entry after the long line is lost, and
if none were restored yet the filter is left empty.  For set_ftrace_filter
an empty filter means all functions are traced, which is the opposite of
the original restriction, and the global ftrace state is not restored.

Should the line buffer be sized to the actual line length (or should the
content be written straight from the saved string) instead of being tied to
CHUNK_SIZE?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/41113297251