Re: [PATCH 09/13] tracing: use trace_seq_init to initize seq fieldin trace_iterator

[Steven Rostedt]

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@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/