Re: [PATCH v1 2/2] perf/core: Fake regs for leaked kernel samples

Jin, Yao
2020-08-07T05:23:15Z

Hi Peter,

On 8/6/2020 5:18 PM, peterz@infradead.org wrote:
> On Thu, Aug 06, 2020 at 10:26:29AM +0800, Jin, Yao wrote:
>
>>> +static struct pt_regs *sanitize_sample_regs(struct perf_event *event, struct pt_regs *regs)
>>> +{
>>> + struct pt_regs *sample_regs = regs;
>>> +
>>> + /* user only */
>>> + if (!event->attr.exclude_kernel || !event->attr.exclude_hv ||
>>> + !event->attr.exclude_host || !event->attr.exclude_guest)
>>> + return sample_regs;
>>> +
>>
>> Is this condition correct?
>>
>> Say counting user event on host, exclude_kernel = 1 and exclude_host = 0. It
>> will go "return sample_regs" path.
>
> I'm not sure, I'm terminally confused on virt stuff.
>
> Suppose we have nested virt:
>
> L0-hv
> |
> G0/L1-hv
> |
> G1
>
> And we're running in G0, then:
>
> - 'exclude_hv' would exclude L0 events
> - 'exclude_host' would ... exclude L1-hv events?

I think the exclude_host is generally set by guest (/arch/x86/kvm/pmu.c, pmc_reprogram_counter).

If G0 is a host, if we set exclude_host in G0, I think we will not be able to count the events on G0.

The appropriate usage is, G1 sets the exclude_host, then the events on G0 will not be collected by
guest G1.

That's my understanding for the usage of exclude_host.

> - 'exclude_guest' would ... exclude G1 events?
>

Similarly, the appropriate usage is, the host (G0) sets the exclude_guest, then the events on G1
will not be collected by host G0.

If G1 sets exclude_guest, since no guest is under G1, that's ineffective.

> Then the next question is, if G0 is a host, does the L1-hv run in
> G0 userspace or G0 kernel space?
>

I'm not very sure. Maybe some in kernel, some in userspace(qemu)? Maybe some KVM experts can help to
answer this question.

> I was assuming G0 userspace would not include anything L1 (kvm is a
> kernel module after all), but what do I know.
>

I have tested following conditions in native environment (not in KVM guests), the result is not
expected.

/* user only */
if (!event->attr.exclude_kernel || !event->attr.exclude_hv ||
!event->attr.exclude_host || !event->attr.exclude_guest)
return sample_regs;

perf record -e cycles:u ./div
perf report --stdio

# Overhead Command Shared Object Symbol
# ........ ....... ................ .......................
#
49.51% div libc-2.27.so [.] __random_r
33.93% div libc-2.27.so [.] __random
8.13% div libc-2.27.so [.] rand
4.29% div div [.] main
4.14% div div [.] rand@plt
0.00% div [unknown] [k] 0xffffffffbd600cb0
0.00% div [unknown] [k] 0xffffffffbd600df0
0.00% div ld-2.27.so [.] _dl_relocate_object
0.00% div ld-2.27.so [.] _dl_start
0.00% div ld-2.27.so [.] _start

0xffffffffbd600cb0 and 0xffffffffbd600df0 are leaked kernel addresses.

From debug, I can see:

[ 6272.320258] jinyao: sanitize_sample_regs: event->attr.exclude_kernel = 1, event->attr.exclude_hv
= 1, event->attr.exclude_host = 0, event->attr.exclude_guest = 0

So it goes "return sample_regs;" path.

>>> @@ -11609,7 +11636,8 @@ SYSCALL_DEFINE5(perf_event_open,
>>> if (err)
>>> return err;
>>> - if (!attr.exclude_kernel) {
>>> + if (!attr.exclude_kernel || !attr.exclude_callchain_kernel ||
>>> + !attr.exclude_hv || !attr.exclude_host || !attr.exclude_guest) {
>>> err = perf_allow_kernel(&attr);
>>> if (err)
>>> return err;
>>>
>>
>> I can understand the conditions "!attr.exclude_kernel || !attr.exclude_callchain_kernel".
>>
>> But I'm not very sure about the "!attr.exclude_hv || !attr.exclude_host || !attr.exclude_guest".
>
> Well, I'm very sure G0 userspace should never see L0 or G1 state, so
> exclude_hv and exclude_guest had better be true.
>
>> On host, exclude_hv = 1, exclude_guest = 1 and exclude_host = 0, right?
>
> Same as above, is G0 host state G0 userspace?
>
>> So even exclude_kernel = 1 but exclude_host = 0, we will still go
>> perf_allow_kernel path. Please correct me if my understanding is wrong.
>
> Yes, because with those permission checks in place it means you have
> permission to see kernel bits.
>

At the syscall entry, I also added some printk.

Aug 7 03:37:40 kbl-ppc kernel: [ 854.688045] syscall: attr.exclude_kernel = 1,
attr.exclude_callchain_kernel = 0, attr.exclude_hv = 0, attr.exclude_host = 0, attr.exclude_guest = 0

For my test case ("perf record -e cycles:u ./div"), the perf_allow_kernel() is also executed.

Thanks
Jin Yao