Re: [PATCH v1 0/2] perf: Drop leaked kernel samples

From: Jin, Yao
Date: Fri Jun 15 2018 - 21:18:16 EST




On 6/16/2018 8:56 AM, Kyle Huey wrote:
On Fri, Jun 15, 2018 at 5:50 PM, Jin, Yao <yao.jin@xxxxxxxxxxxxxxx> wrote:
Hi All,

This patch raised many questions, I was prepared. :)

I'd like to try another proposal that it adds a special flag in the returned
perf_sample_data to indicate the perf binary that this sample is a leaked
sample.

For example, create a new PERF_SAMPLE_RETURN_LEAKAGE in
perf_event_sample_format.

In perf_prepare_sample(),

if (event->attr.exclude_kernel && !user_mode(regs))
data->type |= PERF_SAMPLE_RETURN_LEAKAGE;

Now all the samples are kept and the leaked kernel samples are tagged with
PERF_SAMPLE_RETURN_LEAKAGE.

In perf binary, it filters out the samples with PERF_SAMPLE_RETURN_LEAKAGE.
It needs perf binary modification but rr doesn't need to be changed.

I don't 0-stuffing some fields because:

1. Keeping the skid info should allow us to look at that if we have
interesting later.

2. Not sure if 0-stuffing some fields has potential conflicts with some
applications.

Is this proposal reasonable?

Thanks
Jin Yao


On 6/16/2018 1:34 AM, Robert O'Callahan wrote:

On Fri, Jun 15, 2018 at 10:16 AM, Kyle Huey <me@xxxxxxxxxxxx> wrote:


If you want a sysctl for your own reasons that's fine. But we don't
want a sysctl. We want to work without any further configuration.


Also toggling a sysctl would require root privileges, but rr does not
currently need to run as root. Thus rr users would have to either
permanently change their system configuration (and every extra
configuration step is a pain), or run rr as root so rr can toggle the
sysctl itself. Running rr as root is highly undesirable.

Rob



If the problem you're trying to fix is an inappropriate disclosure of
kernel-space information to user-space, how does filtering the samples
in user space solve anything?

- Kyle


In theory it is. But actually we just use perf tool to look at the sampling result.

And suppose a case, if we want to estimate the skid window, we still need the skid info.

Thanks
Jin Yao