Re: [PATCH v2 07/15] tools/rv: Implement BPF monitor discovery and listing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] bpf: stack OOB access via unterminated strncpy()
- [Low] bpf: off-by-one snprintf check and file leak
--

commit 87f9013a78d7a34904987bd2e9122ad081e635b2
Author: Gabriele Monaco <gmonaco@redhat.com>

tools/rv: Implement BPF monitor discovery and listing

Implement the BPF monitor listing functionality that scans for compiled
BPF monitor object files and displays them in the monitor list.

> diff --git a/tools/verification/rv/src/bpf_monitor.c b/tools/verification/rv/src/bpf_monitor.c
> new file mode 100644
> index 0000000000000..a1de0c1577128
> --- /dev/null
> +++ b/tools/verification/rv/src/bpf_monitor.c
[ ... ]
> +static void bpf_object_iterate_path(const char *base_path, const char *subdir,
> +				    void (*action)(const char *name, struct bpf_object *obj))
> +{
> +	char path[MAX_PATH];
> +	struct dirent *entry;
> +	DIR *dir;
> +	char *ext;
> +
> +	snprintf(path, sizeof(path), "%s/%s", base_path, subdir);
> +	dir = opendir(path);
> +	if (!dir) {
> +		debug_msg("bpf: error opening directory: %s\n", path);
> +		return;
> +	}
> +
> +	while ((entry = readdir(dir)) != NULL) {
> +		size_t size;
> +		struct bpf_object *obj;
> +		char name[MAX_DA_NAME_LEN], obj_path[MAX_PATH];
> +
> +		if (entry->d_name[0] == '.')
> +			continue;
> +
> +		ext = strrchr(entry->d_name, '.');
> +		if (!ext || strcmp(ext, ".o") != 0)
> +			continue;
> +
> +		size = snprintf(obj_path, sizeof(obj_path), "%s/%s", path,
> +				entry->d_name);
> +		obj = bpf_object__open_file(obj_path, NULL);
> +		if (!obj || size > MAX_PATH) {

[Severity: Low]
Does this logic correctly handle path truncation?

The check size > MAX_PATH has an off-by-one error since snprintf() returns the
number of characters that would have been written excluding the null byte,
meaning truncation occurs if size >= MAX_PATH (or sizeof(obj_path)).

Also, bpf_object__open_file() is called before this check, potentially
operating on truncated paths. If open_file succeeds on a truncated string,
would the obj resource be leaked on the error path via the continue statement?

> +			err_msg("bpf: error opening object file %s: %s\n",
> +				obj_path, strerror(errno));
> +			continue;
> +		}
> +
> +		strncpy(name, entry->d_name, sizeof(name));
> +		ext = strrchr(name, '.');
> +		if (ext)
> +			*ext = '\0';

[Severity: Medium]
Could this sequence result in a stack out-of-bounds read and write?

If entry->d_name is exactly MAX_DA_NAME_LEN characters or longer, strncpy()
will leave the name array without a null terminator.

This causes strrchr() to scan adjacent stack memory (such as obj_path) to
find a '.', and potentially write a '\0' out of bounds if one is found. Can
this be triggered by a local user placing a file with a long name in the
bpf_monitors directory?

> +
> +		action(name, obj);
> +
> +		bpf_object__close(obj);
> +	}
> +
> +	closedir(dir);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001152042.124445-1-gmonaco@redhat.com?part=7