linux-kernel - Re: [PATCH] KVM: VMX: fixes for vmentry_l1d

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: <CA+res+SLL5iQKXNyrEy75z=eUSW+D2Hsr+RMT7b9VPR-bffrrg@mail.gmail.com>
Date:   Wed, 22 Aug 2018 17:31:59 +0200
From:   Jack Wang <jack.wang.usish@...il.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org, bsd@...hat.com,
        stable@...r.kernel.org
Subject: Re: [PATCH] KVM: VMX: fixes for vmentry_l1d_flush module parameter

Paolo Bonzini <pbonzini@...hat.com> 于2018年8月22日周三 下午4:56写道：
>
> Two bug fixes:
>
> 1) missing entries in the l1d_param array; this can cause a host crash
> if an access attempts to reach the missing entry. Future-proof the get
> function against any overflows as well.  However, the two entries
> VMENTER_L1D_FLUSH_EPT_DISABLED and VMENTER_L1D_FLUSH_NOT_REQUIRED must
> not be accepted by the parse function, so disable them there.
>
> 2) invalid values must be rejected even if the CPU does not have the
> bug, so test for them before checking boot_cpu_has(X86_BUG_L1TF)
>
> ... and a small refactoring, since the .cmd field is redundant with
> the index in the array.
>
> Reported-by: Bandan Das <bsd@...hat.com>
> Cc: stable@...r.kernel.org
> Fixes: a7b9020b06ec6d7c3f3b0d4ef1a9eba12654f4f7
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> ---
>  arch/x86/kvm/vmx.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index c76ca8c4befa..8dae47e7267a 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -198,12 +198,14 @@
>
>  static const struct {
>         const char *option;
> -       enum vmx_l1d_flush_state cmd;
> +       bool for_parse;
>  } vmentry_l1d_param[] = {
> -       {"auto",        VMENTER_L1D_FLUSH_AUTO},
> -       {"never",       VMENTER_L1D_FLUSH_NEVER},
> -       {"cond",        VMENTER_L1D_FLUSH_COND},
> -       {"always",      VMENTER_L1D_FLUSH_ALWAYS},
> +       [VMENTER_L1D_FLUSH_AUTO]         = {"auto", true},
> +       [VMENTER_L1D_FLUSH_NEVER]        = {"never", true},
> +       [VMENTER_L1D_FLUSH_COND]         = {"cond", true},
> +       [VMENTER_L1D_FLUSH_ALWAYS]       = {"always", true},
> +       [VMENTER_L1D_FLUSH_EPT_DISABLED] = {"EPT disabled", false},
> +       [VMENTER_L1D_FLUSH_NOT_REQUIRED] = {"not required", false},
>  };
>
>  #define L1D_CACHE_ORDER 4
> @@ -287,8 +289,9 @@ static int vmentry_l1d_flush_parse(const char *s)
>
>         if (s) {
>                 for (i = 0; i < ARRAY_SIZE(vmentry_l1d_param); i++) {
> -                       if (sysfs_streq(s, vmentry_l1d_param[i].option))
> -                               return vmentry_l1d_param[i].cmd;
> +                       if (vmentry_l1d_param[i].for_parse &&
> +                           sysfs_streq(s, vmentry_l1d_param[i].option))
> +                               return i;
>                 }
>         }
>         return -EINVAL;
> @@ -298,13 +301,13 @@ static int vmentry_l1d_flush_set(const char *s, const struct kernel_param *kp)
>  {
>         int l1tf, ret;
>
> -       if (!boot_cpu_has(X86_BUG_L1TF))
> -               return 0;
> -
>         l1tf = vmentry_l1d_flush_parse(s);
>         if (l1tf < 0)
>                 return l1tf;
>
> +       if (!boot_cpu_has(X86_BUG_L1TF))
> +               return 0;
> +
>         /*
>          * Has vmx_init() run already? If not then this is the pre init
>          * parameter parsing. In that case just store the value and let
> @@ -324,6 +327,9 @@ static int vmentry_l1d_flush_set(const char *s, const struct kernel_param *kp)
>
>  static int vmentry_l1d_flush_get(char *s, const struct kernel_param *kp)
>  {
> +       if (WARN_ON_ONCE(l1tf_vmx_mitigation >= ARRAY_SIZE(vmentry_l1d_param)))
> +               return sprintf(s, "???\n");
> +
>         return sprintf(s, "%s\n", vmentry_l1d_param[l1tf_vmx_mitigation].option);
>  }
>
> --
> 1.8.3.1
>
Tested-by: Jack Wang <jinpu.wang@...fitbricks.com>
Thanks,

Jack