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: <20120524121657.GC1775@m.brq.redhat.com>
Date:	Thu, 24 May 2012 14:16:57 +0200
From:	Jiri Olsa <jolsa@...hat.com>
To:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc:	acme@...hat.com, mingo@...e.hu, paulus@...ba.org,
	cjashfor@...ux.vnet.ibm.com, fweisbec@...il.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

SNIP

> > +               /* 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?
hm, I dont think we care.. seems like this can go out


> > +       if (sample_type & PERF_SAMPLE_STACK) {
> > +               u64 mode = event->attr.sample_stack;
> > +
> > +               if (mode & PERF_SAMPLE_STACK_USER) {
> > +                       u64 dump_size = event->attr.sample_stack_user;
> > +
> > +                       perf_output_sample_ustack(handle, dump_size,
> > +                                                 data->regs_user);
> 
> OK, so that function is called _ustack() I read that as userstack, so
> why this strange split up?
ook


> 
> > +               }
> > +       }
> >  }
> >  
> >  void perf_prepare_sample(struct perf_event_header *header,
> > @@ -4135,6 +4185,39 @@ void perf_prepare_sample(struct perf_event_header *header,
> >  
> >                 header->size += size;
> >         }
> > +
> > +       if (sample_type & PERF_SAMPLE_STACK) {
> > +               u64 mode = event->attr.sample_stack;
> > +               int size = 0;
> > +
> > +               if (mode & PERF_SAMPLE_STACK_USER) {
> 
> This is very much similar to ->sample_stack_user, since a non-zero size
> usually means you want something.

ok, same case as for the regs bitmask stuf then..
we can use the size to check the presence in sample

> 
> > +                       if (!data->regs_user)
> > +                               data->regs_user = perf_sample_regs_user(regs);
> > +
> > +                       /*
> > +                        * A first field that tells the _static_ size of the
> > +                        * dump. 0 if there is nothing to dump (ie: we are in
> > +                        * a kernel thread) otherwise the requested size.
> > +                        */
> > +                       size += sizeof(u64);
> > +
> > +                       /*
> > +                        * If there is something to dump, add space for the
> > +                        * dump itself and for the field that tells the
> > +                        * dynamic size, which is how many have been actually
> > +                        * dumped. What couldn't be dumped will be zero-padded.
> > +                        */
> > +                       if (data->regs_user) {
> > +                               u64 user_size = event->attr.sample_stack_user;
> > +
> > +                               user_size = round_up(user_size, sizeof(u64));
> 
> Right, and here we go again.. so how about you either reject sizes that
> aren't properly aligned in perf_copy_attr() or just fix it up there.

right, we can do that in the attr check
--
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/