On Fri, Mar 05, 2021 at 11:56:33AM +0100, Jan Beulich wrote:
> On 04.03.2021 15:47, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/hvm/svm/svm.c
> > +++ b/xen/arch/x86/hvm/svm/svm.c
> > @@ -1795,6 +1795,7 @@ static int svm_msr_read_intercept(unsigned int msr,
> > uint64_t *msr_content)
> > const struct domain *d = v->domain;
> > struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
> > const struct nestedsvm *nsvm = &vcpu_nestedsvm(v);
> > + uint64_t tmp;
> >
> > switch ( msr )
> > {
> > @@ -1965,6 +1966,11 @@ static int svm_msr_read_intercept(unsigned int msr,
> > uint64_t *msr_content)
> > break;
> >
> > default:
> > + if ( d->arch.hvm.rdmsr_relaxed && !rdmsr_safe(msr, tmp) )
> > + {
> > + *msr_content = 0;
> > + break;
> > + }
>
> You don't really need "tmp" here, do you? You could as well read
> into *msr_content, as you're zapping the value afterwards anyway.
I also thought about doing this, but felt unease. I fear the code
might be changed in the future and maybe msr_content is not zapped
anymore, thus leaking the content. I feel it's safer to use a
temporary variable that will never be returned to the guest. Maybe
I'm just too paranoid.
Thanks, Roger.
