linux-kernel - Re: [PATCH 09/13] tracing: use trace_seq_init to initize seq field in trace

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] [day] [month] [year] [list]

Message-ID: <1363011206.12608.10.camel@gandalf.local.home>
Date:	Mon, 11 Mar 2013 10:13:26 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	"zhangwei(Jovi)" <jovi.zhangwei@...wei.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH 09/13] tracing: use trace_seq_init to initize seq field
 in trace_iterator

On Mon, 2013-03-11 at 15:14 +0800, zhangwei(Jovi) wrote:
> the size of seq field in trace_iterator is more than PAGE_SIZE, so
> seq memset is expensive, try to use cheap seq init method: trace_seq_init

I have no problem with this change, except that it may need to be
audited. The seq code is sent out over the trace buffer, and even though
only root can enable tracing, I still don't want to allow uninitialized
memory to ever be seen by userspace.

I believe this code won't allow it, but that memset() was more of a
paranoid way to protect against uninitialized memory leaking to
userspace. I want to review the effect of this change everywhere before
I include it.

-- Steve

> 
> Signed-off-by: zhangwei(Jovi) <jovi.zhangwei@...wei.com>
> ---
>  kernel/trace/trace.c     |   10 ++++++----
>  kernel/trace/trace_kdb.c |    5 +++--
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 4cec7b8..c7cc915 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3551,9 +3551,10 @@ waitagain:
>  		cnt = PAGE_SIZE - 1;
> 
>  	/* reset all but tr, trace, and overruns */
> -	memset(&iter->seq, 0,
> +	trace_seq_init(&iter->seq);
> +	memset(&iter->ent, 0,
>  	       sizeof(struct trace_iterator) -
> -	       offsetof(struct trace_iterator, seq));
> +	       offsetof(struct trace_iterator, ent));
>  	iter->pos = -1;
> 
>  	trace_event_read_lock();
> @@ -5167,9 +5168,10 @@ __ftrace_dump(bool disable_tracing, enum ftrace_dump_mode oops_dump_mode)
>  		cnt++;
> 
>  		/* reset all but tr, trace, and overruns */
> -		memset(&iter.seq, 0,
> +		trace_seq_init(&iter.seq);
> +		memset(&iter.ent, 0,
>  		       sizeof(struct trace_iterator) -
> -		       offsetof(struct trace_iterator, seq));
> +		       offsetof(struct trace_iterator, ent));
>  		iter.iter_flags |= TRACE_FILE_LAT_FMT;
>  		iter.pos = -1;
> 
> diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c
> index 6489b2e..84f1584 100644
> --- a/kernel/trace/trace_kdb.c
> +++ b/kernel/trace/trace_kdb.c
> @@ -37,9 +37,10 @@ static void ftrace_dump_buf(int skip_lines, long cpu_file)
>  	kdb_printf("Dumping ftrace buffer:\n");
> 
>  	/* reset all but tr, trace, and overruns */
> -	memset(&iter.seq, 0,
> +	trace_seq_init(&iter.seq);
> +	memset(&iter.ent, 0,
>  		   sizeof(struct trace_iterator) -
> -		   offsetof(struct trace_iterator, seq));
> +		   offsetof(struct trace_iterator, ent));
>  	iter.iter_flags |= TRACE_FILE_LAT_FMT;
>  	iter.pos = -1;
> 


--
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/