linux-kernel - Re: [PATCH 04/16] perf: Add ability to attach user stack dump to sample

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: <20120607100752.GD19842@somewhere.redhat.com>
Date:	Thu, 7 Jun 2012 12:07:56 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc:	Jiri Olsa <jolsa@...hat.com>, acme@...hat.com, mingo@...e.hu,
	paulus@...ba.org, cjashfor@...ux.vnet.ibm.com, eranian@...gle.com,
	gorcunov@...nvz.org, tzanussi@...il.com, mhiramat@...hat.com,
	robert.richter@....com, fche@...hat.com,
	linux-kernel@...r.kernel.org, masami.hiramatsu.pt@...achi.com,
	drepper@...il.com, asharma@...com, benjamin.redelings@...cent.org
Subject: Re: [PATCH 04/16] perf: Add ability to attach user stack dump to
 sample

On Thu, May 24, 2012 at 12:51:20PM +0200, Peter Zijlstra wrote:
> On Wed, 2012-05-23 at 21:32 +0200, Jiri Olsa wrote:
> > +static void
> > +perf_output_sample_ustack(struct perf_output_handle *handle, u64 dump_size,
> > +                         struct pt_regs *regs)
> > +{
> > +       u64 size;
> > +
> > +       /* Case of a kernel thread, nothing to dump */
> > +       if (!regs) {
> > +               size = 0;
> > +               perf_output_put(handle, size);
> > +       } else {
> > +               unsigned long sp;
> > +               unsigned int rem;
> > +               u64 dyn_size;
> > +
> > +               /*
> > +                * Static size: we always dump the size
> > +                * requested by the user because most of the
> > +                * time, the top of the user stack is not
> > +                * paged out.
> > +                */
> > +               size = round_up(dump_size, sizeof(u64));
> 
> You also do this in the prepare thing..
> 
> > +               perf_output_put(handle, size);
> > +
> > +               sp = user_stack_pointer(regs);
> > +               rem = __output_copy_user(handle, (void *)sp, size);
> > +               dyn_size = size - rem;
> > +
> > +               /* What couldn't be dumped is zero padded */
> > +               while (rem--) {
> > +                       char zero = 0;
> > +                       perf_output_put(handle, zero);
> > +               }
> 
> Does this matter? If we don't write it the worst that can happen is that
> we leave previous ring-bugger content around, but since we already are
> privileged to read that (and very likely already have) there's no
> problem with that..
> 
> I know not zero-ing is ugly, but its also faster.. and do we care about
> them silly zeros?

The problem there is that the unwinder may then deal with random values
from previous records in the buffer and think it is some real stack values.
This may give strange results. At least zeroing was limiting this effect.

I think the best would be to do like you say: if we can't dump everything
then just leave the state of the buffer as is. But also output in the end
the effective size of the stack that has been dumped. So that we know what
we can give to the unwinder and what we can not.
--
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/