LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Wei Wang <wei.w.wang@intel.com>
To: Eric Hankland <ehankland@google.com>,
	pbonzini@redhat.com, rkrcmar@redhat.com
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH v1] KVM: x86: PMU Whitelist
Date: Fri, 14 Jun 2019 17:26:27 +0800	[thread overview]
Message-ID: <5D036843.2010607@intel.com> (raw)
In-Reply-To: <CAOyeoRWfPNmaWY6Lifdkdj3KPPM654vzDO+s3oduEMCJP+Asow@mail.gmail.com>

On 05/23/2019 06:23 AM, Eric Hankland wrote:
> - Add a VCPU ioctl that can control which events the guest can monitor.
>
> Signed-off-by: ehankland <ehankland@google.com>
> ---
> Some events can provide a guest with information about other guests or the
> host (e.g. L3 cache stats); providing the capability to restrict access
> to a "safe" set of events would limit the potential for the PMU to be used
> in any side channel attacks. This change introduces a new vcpu ioctl that
> sets an event whitelist. If the guest attempts to program a counter for
> any unwhitelisted event, the kernel counter won't be created, so any
> RDPMC/RDMSR will show 0 instances of that event.
> ---
>   Documentation/virtual/kvm/api.txt | 16 +++++++++++
>   arch/x86/include/asm/kvm_host.h   |  1 +
>   arch/x86/include/uapi/asm/kvm.h   |  7 +++++
>   arch/x86/kvm/pmu.c                | 44 +++++++++++++++++++++++++++++++
>   arch/x86/kvm/pmu.h                |  3 +++
>   arch/x86/kvm/pmu_amd.c            | 16 +++++++++++
>   arch/x86/kvm/vmx/pmu_intel.c      | 16 +++++++++++
>   arch/x86/kvm/x86.c                |  7 +++++
>   include/uapi/linux/kvm.h          |  4 +++
>   9 files changed, 114 insertions(+)
>
> diff --git a/Documentation/virtual/kvm/api.txt
> b/Documentation/virtual/kvm/api.txt
> index ba6c42c576dd..79cbe7339145 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -4065,6 +4065,22 @@ KVM_ARM_VCPU_FINALIZE call.
>   See KVM_ARM_VCPU_INIT for details of vcpu features that require finalization
>   using this ioctl.
>
> +4.120 KVM_SET_PMU_WHITELIST
> +
> +Capability: KVM_CAP_PMU_WHITELIST
> +Architectures: x86
> +Type: vm ioctl
> +Parameters: struct kvm_pmu_whitelist (in)
> +Returns: 0 on success, -1 on error
> +
> +struct kvm_pmu_whitelist {
> +       __u64 event_mask;

Is this "ARCH_PERFMON_EVENTSEL_EVENT | ARCH_PERFMON_EVENTSEL_UMASK"?


> +       __u16 num_events;
> +       __u64 events[0];

Can this be __u16?
The lower 16 bits (umask+eventsel) already determines what the event is.


> +};
> +This ioctl restricts the set of PMU events that the guest can program to the
> +set of whitelisted events.
> +
>   5. The kvm_run structure
>   ------------------------
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 450d69a1e6fa..942647475999 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -477,6 +477,7 @@ struct kvm_pmu {
>          struct kvm_pmc fixed_counters[INTEL_PMC_MAX_FIXED];
>          struct irq_work irq_work;
>          u64 reprogram_pmi;
> +       struct kvm_pmu_whitelist *whitelist;

This could be per-VM and under rcu?

>   };
>
>   struct kvm_pmu_ops;
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index 7a0e64ccd6ff..2633b48b75cd 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -421,4 +421,11 @@ struct kvm_nested_state {
>          __u8 data[0];
>   };
>
> +/* for KVM_SET_PMU_WHITELIST */
> +struct kvm_pmu_whitelist {
> +       __u64 event_mask;
> +       __u16 num_events;
> +       __u64 events[0];
> +};
> +
>   #endif /* _ASM_X86_KVM_H */
> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> index e39741997893..d0d81cd3626d 100644
> --- a/arch/x86/kvm/pmu.c
> +++ b/arch/x86/kvm/pmu.c
> @@ -101,6 +101,9 @@ static void pmc_reprogram_counter(struct kvm_pmc
> *pmc, u32 type,
>                                    bool exclude_kernel, bool intr,
>                                    bool in_tx, bool in_tx_cp)
>   {
> +       struct kvm_pmu *pmu = pmc_to_pmu(pmc);
> +       int i;
> +       u64 event_config;
>          struct perf_event *event;
>          struct perf_event_attr attr = {
>                  .type = type,
> @@ -127,6 +130,19 @@ static void pmc_reprogram_counter(struct kvm_pmc
> *pmc, u32 type,
>                  attr.config |= HSW_IN_TX_CHECKPOINTED;
>          }
>
> +       if (pmu->whitelist) {

Why not moving this filter to reprogram_gp_counter?

You could directly compare "unit_mask, event_sel"  with whitelist->events[i]

> +               event_config = attr.config;
> +               if (type == PERF_TYPE_HARDWARE)
> +                       event_config = kvm_x86_ops->pmu_ops->get_event_code(
> +                               attr.config);
> +               event_config &= pmu->whitelist->event_mask;
> +               for (i = 0; i < pmu->whitelist->num_events; i++)
> +                       if (event_config == pmu->whitelist->events[i])
> +                               break;
> +               if (i == pmu->whitelist->num_events)
> +                       return;
> +       }
> +
>          event = perf_event_create_kernel_counter(&attr, -1, current,
>                                                   intr ? kvm_perf_overflow_intr :
>                                                   kvm_perf_overflow, pmc);
> @@ -244,6 +260,34 @@ int kvm_pmu_is_valid_msr_idx(struct kvm_vcpu
> *vcpu, unsigned idx)
>          return kvm_x86_ops->pmu_ops->is_valid_msr_idx(vcpu, idx);
>   }
>
> +int kvm_vcpu_ioctl_set_pmu_whitelist(struct kvm_vcpu *vcpu,
> +                                    struct kvm_pmu_whitelist __user *whtlst)
> +{
> +       struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> +       struct kvm_pmu_whitelist *old = pmu->whitelist;
> +       struct kvm_pmu_whitelist *new = NULL;
> +       struct kvm_pmu_whitelist tmp;
> +       int r;
> +       size_t size;
> +
> +       r = -EFAULT;
> +       if (copy_from_user(&tmp, whtlst, sizeof(struct kvm_pmu_whitelist)))
> +               goto err;

I would directly return -EFAULT here.

> +
> +       size = sizeof(tmp) + sizeof(tmp.events[0]) * tmp.num_events;
> +       new = kvzalloc(size, GFP_KERNEL_ACCOUNT);
> +       r = -ENOMEM;
> +       if (!new)
> +               goto err;

Same here.

> +       pmu->whitelist = new;

Forgot to copy the whitelist-ed events to new?


Best,
Wei

  parent reply	other threads:[~2019-06-14  9:21 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-22 22:23 Eric Hankland
2019-05-28  2:01 ` Wei Wang
2019-05-28 18:14   ` Eric Hankland
2019-05-29  7:54     ` Wei Wang
2019-05-29 17:11       ` Eric Hankland
2019-05-31  1:02         ` Wei Wang
2019-05-31 19:59           ` Eric Hankland
2019-06-01 10:55             ` Wei Wang
2019-06-03 17:30               ` Eric Hankland
2019-06-04  4:42                 ` Wei Wang
2019-06-04 15:56                   ` Eric Hankland
     [not found]                     ` <CAEU=KTHsVmrAHXUKdHu_OwcrZoy-hgV7pk4UymtchGE5bGdUGA@mail.gmail.com>
2019-06-05 21:35                       ` Eric Hankland
2019-06-06  7:36                         ` Wei Wang
2019-06-13 17:43                           ` Eric Hankland
2019-06-14  9:14                             ` Wei Wang
2019-06-14  9:26 ` Wei Wang [this message]
2019-06-25  0:32   ` Eric Hankland
2019-06-25  9:12     ` Wei Wang
2019-07-02 17:46       ` Eric Hankland
2019-07-03  9:06         ` Wei Wang
2019-06-20 18:05 ` Andi Kleen
2019-06-24 23:56   ` Eric Hankland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5D036843.2010607@intel.com \
    --to=wei.w.wang@intel.com \
    --cc=ehankland@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rkrcmar@redhat.com \
    --subject='Re: [PATCH v1] KVM: x86: PMU Whitelist' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).