Re: [PATCH 1/1] x86/ept: Fix buggy XSA-321 backport

["M. Vefa Bicakci" <m.v.b@xxxxxxxxxx>]

On 16/02/2021 07.48, M. Vefa Bicakci wrote:
> On 16/02/2021 04.20, Roger Pau Monné wrote:
> > On Mon, Feb 15, 2021 at 06:46:19PM -0500, M. Vefa Bicakci wrote:
[snipped by Vefa]
> >> In any case I think this is too much change, so I would go for a
> > smaller fix like my proposal below. Can you please test it?
> Thank you! I will test your patch later today, and I will report
> back by tomorrow.
[snipped by Vefa]
> > Here is my proposed fix, I think we could even do away with the else
> > branch, but if level is != 0 p2m_is_foreign should be false, so we
> > avoid an extra check.
> >
> > Thanks, Roger.
> I will test this. Thanks again! I really appreciate that you have
> have taken the time and effort.
>
> Vefa
Hello Roger,

I have tested your patch, and I am happy to confirm that it too resolves
the issue I have described in my original patch description. Thank you!

When I find some more time, I would like to prepare a GitHub pull request
for Qubes OS 4.0's version of Xen 4.8.5 with your patch so that other users
do not encounter the same issue. I would like to properly credit your
contribution. Would you be able to send a patch with a Signed-off-by tag
in its description?

Thanks again,

Vefa

> > ---8<---
> > diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> > index 036771f43c..086739ffdd 100644
> > --- a/xen/arch/x86/mm/p2m-ept.c
> > +++ b/xen/arch/x86/mm/p2m-ept.c
> > @@ -56,11 +56,8 @@ static int atomic_write_ept_entry(ept_entry_t *entryptr, 
> > ept_entry_t new,
> >       if ( level )
> >       {
> >           ASSERT(!is_epte_superpage(&new) || !p2m_is_foreign(new.sa_p2mt));
> > -        write_atomic(&entryptr->epte, new.epte);
> > -        return 0;
> >       }
> > -
> > -    if ( unlikely(p2m_is_foreign(new.sa_p2mt)) )
> > +    else if ( unlikely(p2m_is_foreign(new.sa_p2mt)) )
> >       {
> >           rc = -EINVAL;
> >           if ( !is_epte_present(&new) )