linux-kernel - Re: [PATCH v4 04/11] tracing: Use tracing error

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: <20190325111724.GB211529@google.com>
Date:   Mon, 25 Mar 2019 20:37:23 +0900
From:   Namhyung Kim <namhyung@...nel.org>
To:     Tom Zanussi <zanussi@...nel.org>
Cc:     rostedt@...dmis.org, tglx@...utronix.de, mhiramat@...nel.org,
        bigeasy@...utronix.de, joel@...lfernandes.org,
        linux-kernel@...r.kernel.org, linux-rt-users@...r.kernel.org
Subject: Re: [PATCH v4 04/11] tracing: Use tracing error_log with trace event
 filters

Hi Tom,

On Fri, Mar 22, 2019 at 10:34:30AM -0500, Tom Zanussi wrote:
> From: Tom Zanussi <tom.zanussi@...ux.intel.com>
> 
> Use tracing_log_err() from the new tracing error_log mechanism to send
> filter parse errors to tracing/error_log.
> 
> With this change, users will be able to see filter errors by looking
> at tracing/error_log.
> 
> The same errors will also be available in the filter file, as
> expected.
> 
> Signed-off-by: Tom Zanussi <tom.zanussi@...ux.intel.com>
> ---
>  kernel/trace/trace_events_filter.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index ade606c33231..3a1ed8631aa0 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -66,7 +66,8 @@ static const char * ops[] = { OPS };
>  	C(INVALID_FILTER,	"Meaningless filter expression"),	\
>  	C(IP_FIELD_ONLY,	"Only 'ip' field is supported for function trace"), \
>  	C(INVALID_VALUE,	"Invalid value (did you forget quotes)?"), \
> -	C(NO_FILTER,		"No filter found"),
> +	C(ERRNO,		"Error"),				\
> +	C(NO_FILTER,		"No filter found")
>  
>  #undef C
>  #define C(a, b)		FILT_ERR_##a
> @@ -76,7 +77,7 @@ enum { ERRORS };
>  #undef C
>  #define C(a, b)		b
>  
> -static char *err_text[] = { ERRORS };
> +static const char *err_text[] = { ERRORS };
>  
>  /* Called after a '!' character but "!=" and "!~" are not "not"s */
>  static bool is_not(const char *str)
> @@ -947,8 +948,10 @@ static void append_filter_err(struct filter_parse_error *pe,
>  	if (pe->lasterr > 0) {
>  		trace_seq_printf(s, "\n%*s", pos, "^");
>  		trace_seq_printf(s, "\nparse_error: %s\n", err_text[pe->lasterr]);
> +		tracing_log_err("event filter parse error", filter->filter_string, err_text, pe->lasterr, pe->lasterr_pos);
>  	} else {
>  		trace_seq_printf(s, "\nError: (%d)\n", pe->lasterr);
> +		tracing_log_err("event filter parse error", filter->filter_string, err_text, FILT_ERR_ERRNO, 0);

These lines are too long, could you please wrap?
Otherwise, the whole series look good to me.

Acked-by: Namhyung Kim <namhyung@...nel.org>

Thanks,
Namhyung


>  	}
>  	trace_seq_putc(s, 0);
>  	buf = kmemdup_nul(s->buffer, s->seq.len, GFP_KERNEL);
> -- 
> 2.14.1
>