Re: [PATCH 11/12] tracing/probes: Validate command prefix in kprobe, uprobe, and eprobe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] tracing/uprobes: memory leak in __trace_uprobe_create() [1]
--

commit d4f1d831d5c45144f78113e0b3ef37e7d6dc9844
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

tracing/probes: Validate command prefix in kprobe, uprobe, and eprobe

This commit validates trailing characters after the command prefix in
kprobe, uprobe, and eprobe creation, rejecting invalid characters with
BAD_MAXACT instead of silently ignoring them.

> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 14ac52aebe346..3745eb07bd14e 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -616,9 +616,6 @@ static int __trace_uprobe_create(int argc, const char **argv)
>  		return -E2BIG;
>  	}
>  
> -	if (argv[0][1] == ':')
> -		event = &argv[0][2];
> -
>  	if (!strchr(argv[1], '/'))
>  		return -ECANCELED;
>  
> @@ -631,6 +628,15 @@ static int __trace_uprobe_create(int argc, const char **argv)
>  	if (!arg || !isdigit(arg[1]))
>  		return -ECANCELED;
>  
> +	if (argv[0][1] != '\0') {
> +		if (argv[0][1] != ':') {
> +			trace_probe_log_set_index(0);
> +			trace_probe_log_err(1, BAD_MAXACT);
> +			return -EINVAL;
> +		}
> +		event = &argv[0][2];
> +	}

[Severity: Medium]
This problem wasn't introduced by this patch, but can __trace_uprobe_create()
leak tail [1] when generating a default event name?

kernel/trace/trace_uprobe.c:__trace_uprobe_create() {
	...
	if (!event) {
		char *tail;
		...
		tail = kstrdup(kbasename(filename), GFP_KERNEL);
		if (!tail)
			return -ENOMEM;
		...
		buf = kmalloc(MAX_EVENT_NAME_LEN, GFP_KERNEL);
		if (!buf)
			return -ENOMEM;
		...
		kfree(tail);
	}
	...
}

When event is NULL and allocating buf fails, __trace_uprobe_create() returns
-ENOMEM without freeing tail (which is not declared with __free(kfree)).

[1] https://sashiko.dev/#/bug/linux-160fa01f-1d70-4f17-984f-1bbf039dd3df

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179418408457.227715.1485179201014819821.stgit@devnote2?part=11