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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ