linux-kernel - Re: [PATCH] tracing: fix incorrect enabling of trace events by boot cmdline

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: <20150416093952.1dba3052@gandalf.local.home>
Date:	Thu, 16 Apr 2015 09:39:52 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Joonsoo Kim <iamjoonsoo.kim@....com>
Cc:	Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tracing: fix incorrect enabling of trace events by boot
 cmdline

On Thu, 16 Apr 2015 13:44:44 +0900
Joonsoo Kim <iamjoonsoo.kim@....com> wrote:

> There is a problem that trace events are not properly enabled with
> boot cmdline. Problem is that if we pass "trace_event=kmem:mm_page_alloc"
> to boot cmdline, it enables all kmem trace events.
> 
> It is caused by parsing mechanism. When we parse cmdline, buffer
> contents is modified due to tokenization. And, if we use this buffer
> again, we will get wrong result.
> 
> Unfortunately, this buffer should be accessed three times
> to set trace events properly in boot time. So, we need to handle
> this situation.
> 
> There is already handling code for ",", but, we need another for
> ":". This patch add it.

Thanks, but your patch has a bug in it. I'll fix it up.

> 
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@....com>
> ---
>  kernel/trace/trace_events.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index db54dda..ce5b194 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -565,6 +565,7 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
>  static int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
>  {
>  	char *event = NULL, *sub = NULL, *match;
> +	int ret;
>  
>  	/*
>  	 * The buf format can be <subsystem>:<event-name>
> @@ -590,7 +591,11 @@ static int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
>  			event = NULL;
>  	}
>  
> -	return __ftrace_set_clr_event(tr, match, sub, event, set);
> +	ret = __ftrace_set_clr_event(tr, match, sub, event, set);
> +
> +	/* Put back the colon to allow this to be called again */
> +	if (buf)
> +		*(buf - 1) = ':';

You forgot to add:

	return ret;

-- Steve

>  }
>  
>  /**

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/