Re: [PATCH] vmx: add missing entries in the l1d_param array

Re: [PATCH] vmx: add missing entries in the l1d_param array

[Paolo Bonzini]

On 21/08/2018 23:38, Bandan Das wrote:
> 
> 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.
> 
> Signed-off-by: Bandan Das <bsd@redhat.com>
> ---
>  arch/x86/kvm/vmx.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 46b428c0990e..42c27406b63e 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -203,6 +203,8 @@ static const struct {
>  	{"never",	VMENTER_L1D_FLUSH_NEVER},
>  	{"cond",	VMENTER_L1D_FLUSH_COND},
>  	{"always",	VMENTER_L1D_FLUSH_ALWAYS},
> +	{"disabled",    VMENTER_L1D_FLUSH_EPT_DISABLED},
> +	{"not required", VMENTER_L1D_FLUSH_NOT_REQUIRED},
>  };
>  
>  #define L1D_CACHE_ORDER 4
> @@ -323,7 +325,17 @@ 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)
>  {
> -	return sprintf(s, "%s\n", vmentry_l1d_param[l1tf_vmx_mitigation].option);
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(vmentry_l1d_param); i++) {
> +		if (vmentry_l1d_param[i].cmd == l1tf_vmx_mitigation)
> +			break;
> +	}
> +
> +	if (i == ARRAY_SIZE(vmentry_l1d_param))
> +		return -EINVAL;
> +
> +	return sprintf(s, "%s\n", vmentry_l1d_param[i].option);
>  }
>  
>  static const struct kernel_param_ops vmentry_l1d_flush_ops = {
> 

Queued, thanks.

Paolo

Re: [PATCH] vmx: add missing entries in the l1d_param array

[Paolo Bonzini]

On 21/08/2018 23:38, Bandan Das wrote:
> 
> 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.
> 
> Signed-off-by: Bandan Das <bsd@redhat.com>
> ---
>  arch/x86/kvm/vmx.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 46b428c0990e..42c27406b63e 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -203,6 +203,8 @@ static const struct {
>  	{"never",	VMENTER_L1D_FLUSH_NEVER},
>  	{"cond",	VMENTER_L1D_FLUSH_COND},
>  	{"always",	VMENTER_L1D_FLUSH_ALWAYS},
> +	{"disabled",    VMENTER_L1D_FLUSH_EPT_DISABLED},
> +	{"not required", VMENTER_L1D_FLUSH_NOT_REQUIRED},
>  };
>  
>  #define L1D_CACHE_ORDER 4
> @@ -323,7 +325,17 @@ 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)
>  {
> -	return sprintf(s, "%s\n", vmentry_l1d_param[l1tf_vmx_mitigation].option);
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(vmentry_l1d_param); i++) {
> +		if (vmentry_l1d_param[i].cmd == l1tf_vmx_mitigation)
> +			break;
> +	}
> +
> +	if (i == ARRAY_SIZE(vmentry_l1d_param))
> +		return -EINVAL;
> +
> +	return sprintf(s, "%s\n", vmentry_l1d_param[i].option);
>  }
>  
>  static const struct kernel_param_ops vmentry_l1d_flush_ops = {
> 

Hmm, actually the two missing entries must be rejected by
vmentry_l1d_flush_parse.  I've fixed up the patch since I'm going to
send a pull request to Linus today.

Paolo