linux-kernel - Re: [PATCH] tracing/boottime: Fix kprobe multiple events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3e6495df-065f-34c5-8e21-65764c79d6ca@gmail.com>
Date:   Wed, 17 Jun 2020 17:57:13 +0200
From:   Maximilian Werner <maximilian.werner96@...il.com>
To:     Steven Rostedt <rostedt@...dmis.org>,
        Sascha Ortmann <sascha.ortmann@...d.uni-hannover.de>
Cc:     linux-kernel@...r.kernel.org, mingo@...hat.com,
        linux-trace-devel@...r.kernel.org, linux-kernel@...cs.fau.de,
        Masami Hiramatsu <mhiramat@...nel.org>
Subject: Re: [PATCH] tracing/boottime: Fix kprobe multiple events

We are a group of students from Leibniz University Hannover and
this patch is part of a project of ours. That's why both of us
signed this off.

Should we have added Masami to Cc? He didn't appear in the
get_maintainer script.

-- Maximilian & Sascha

On 17.06.20 17:06, Steven Rostedt wrote:
> On Wed, 17 Jun 2020 11:05:21 -0400
> Steven Rostedt <rostedt@...dmis.org> wrote:
> 
>> On Wed, 17 Jun 2020 16:08:17 +0200
>> Sascha Ortmann <sascha.ortmann@...d.uni-hannover.de> wrote:
>>
>>> Fix boottime kprobe events to add multiple events even if one fails
>>> and report probe generation failures.
>>>
>>> As an example, when we try to set multiprobe kprobe events in
>>> bootconfig like this:
>>>
>>> ftrace.event.kprobes.vfsevents {
>>> 	probes = "vfs_read $arg1 $arg2,,
>>>                   !error! not reported;?", // leads to error
>>> 		 "vfs_write $arg1 $arg2"
>>> }
>>>
>>> this will not work like expected. After commit
>>> da0f1f4167e3af69e1d8b32d6d65195ddd2bfb64 ("tracing/boottime:
>>> Fix kprobe event API usage"), the function
>>> trace_boot_add_kprobe_event will not produce any error message,
>>> aborting the function and stopping subsequent probes from getting
>>> installed when adding a probe fails at kprobe_event_gen_cmd_start.
>>> Furthermore, probes continue when kprobe_event_gen_cmd_end fails
>>> (and kprobe_event_gen_cmd_start did not fail). In this case the
>>> function even returns successfully when the last call to
>>> kprobe_event_gen_cmd_end is successful.
>>>
>>> The behaviour of reporting and aborting after failures is not
>>> consistent.
>>>
>>> The function trace_boot_add_kprobe_event now continues even when
>>> one of the multiple events fails. Each failure is now reported
>>> individually. Since the function can only return one result to the
>>> caller, the function returns now the last failure (or none, if
>>> nothing fails).
>>>
>>> Cc: linux-kernel@...cs.fau.de
>>> Signed-off-by: Maximilian Werner <maximilian.werner96@...il.com>
>>> Signed-off-by: Sascha Ortmann <sascha.ortmann@...d.uni-hannover.de>
>>
>> Why the double signed off by?
>>
>> Masami, I'm fine with this, but needs your review.
> 
> [ It appears that Masami wasn't in the Cc ]
> 
> 
>>
>> -- Steve
>>
>>> ---
>>>   kernel/trace/trace_boot.c | 16 +++++++++++-----
>>>   1 file changed, 11 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
>>> index 9de29bb45a27..dbb50184e060 100644
>>> --- a/kernel/trace/trace_boot.c
>>> +++ b/kernel/trace/trace_boot.c
>>> @@ -95,18 +95,24 @@ trace_boot_add_kprobe_event(struct xbc_node *node, const char *event)
>>>   	struct xbc_node *anode;
>>>   	char buf[MAX_BUF_LEN];
>>>   	const char *val;
>>> +	int error = 0;
>>>   	int ret = 0;
>>>   
>>>   	xbc_node_for_each_array_value(node, "probes", anode, val) {
>>>   		kprobe_event_cmd_init(&cmd, buf, MAX_BUF_LEN);
>>>   
>>> -		ret = kprobe_event_gen_cmd_start(&cmd, event, val);
>>> -		if (ret)
>>> -			break;
>>> +		error = kprobe_event_gen_cmd_start(&cmd, event, val);
>>> +		if (error) {
>>> +			pr_err("Failed to generate probe: %s\n", buf);
>>> +			ret = error;
>>> +			continue;
>>> +		}
>>>   
>>> -		ret = kprobe_event_gen_cmd_end(&cmd);
>>> -		if (ret)
>>> +		error = kprobe_event_gen_cmd_end(&cmd);
>>> +		if (error) {
>>>   			pr_err("Failed to add probe: %s\n", buf);
>>> +			ret = error;
>>> +		}
>>>   	}
>>>   
>>>   	return ret;
>>
>