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
| ||
|
Date: Sat, 30 Jan 2010 22:26:54 +0100 From: Frederic Weisbecker <fweisbec@...il.com> To: Lai Jiangshan <laijs@...fujitsu.com> Cc: Steven Rostedt <rostedt@...dmis.org>, linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>, Andrew Morton <akpm@...ux-foundation.org> Subject: Re: [PATCH 1/6] : bug fix, remove partial zero out On Tue, Jan 26, 2010 at 11:31:58AM +0800, Lai Jiangshan wrote: > Frederic Weisbecker wrote: > > On Tue, Jan 19, 2010 at 03:33:56PM +0800, Lai Jiangshan wrote: > >> partial-zero-out a struct is very dangerous, we should zero out > >> field by field directly when need. > >> > >> partial-zero-out for struct trace_iterator exists when ftrace > >> was first introduced into mainline kernel. But in this few years, > >> the code of ftrace is changed a lot, and: > >> > >> 1) partial-zero-out for struct trace_iterator has a bug now, > >> cpumask_var_t started should not be zeroed out. > >> > >> 2) I viewed the codes and found that fields below > >> "/* The below is zeroed out in pipe_read */" > >> don't need to be zeroed out or initialized now. > >> > >> So, we remove the code of "partial zero out" > >> > >> Signed-off-by: Lai Jiangshan <laijs@...fujitsu.com> > >> --- > >> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h > >> index 3ca9485..c6d0e1a 100644 > >> --- a/include/linux/ftrace_event.h > >> +++ b/include/linux/ftrace_event.h > >> @@ -54,7 +54,6 @@ struct trace_iterator { > >> struct ring_buffer_iter *buffer_iter[NR_CPUS]; > >> unsigned long iter_flags; > >> > >> - /* The below is zeroed out in pipe_read */ > >> struct trace_seq seq; > >> struct trace_entry *ent; > >> int leftover; > >> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > >> index 5314c90..27fecf8 100644 > >> --- a/kernel/trace/trace.c > >> +++ b/kernel/trace/trace.c > >> @@ -3124,12 +3124,6 @@ waitagain: > >> if (cnt >= PAGE_SIZE) > >> cnt = PAGE_SIZE - 1; > >> > >> - /* reset all but tr, trace, and overruns */ > >> - memset(&iter->seq, 0, > >> - sizeof(struct trace_iterator) - > >> - offsetof(struct trace_iterator, seq)); > >> - iter->pos = -1; > >> - > > > > > > > > I'm not sure exaclty why we needed to zero the seq here. > > We already reset it in trace_seq_init(). > > > > We might do it again on waitagain. I lost track how we could > > ever need to goto waitagain. It was about a tricky bug to fix > > but I'm don't remember exactly the details. > > > > That said, if trace_seq_to_user returns -EBUSY, we > > re-init the seq buffer, so it should be fine I guess. > > Yes, -EBUSY is strange here. > but any way, trace_seq_init() is called. > > > > > But concerning the need of setting iter->pos to -1, I'm not > > sure we need to remove it. Shouldn't it be set to 0 btw? > > > > ->pos is not used here, ->idx is just increased here, > so we don't need to initialize them. Ok. -- 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