Hi ! On Thu, Aug 19, 2021 at 4:10 PM Pavel Machek wrote: > > Hi! > > > > CVE-2021-38198: KVM: X86: MMU: Use the correct inherited permissions > > to get shadow page > > > > This vulnerability has been introduced since 2.6.20-rc4 so 4.4 affects > > this CVE but patch didn't apply to 4.4 > > (https://lore.kernel.org/stable/162358450944186@kroah.com/). 4.19 also > > failed to apply this patch but backport patch has been merged > > recently(https://lore.kernel.org/stable/20210812174140.2370680-1-ovidiu.panait@windriver.com/). > > > > I tried to look at this, and it is rather non-trivial. In particular, > I'd not know how to test it. I ended up with this patch, but it is not > even compile-tested. > > Best regards, > Pavel > > diff --git a/Documentation/virtual/kvm/mmu.txt b/Documentation/virtual/kvm/mmu.txt > index b653641d4261..ee5bd16a0856 100644 > --- a/Documentation/virtual/kvm/mmu.txt > +++ b/Documentation/virtual/kvm/mmu.txt > @@ -152,8 +152,8 @@ Shadow pages contain the following information: > shadow pages) so role.quadrant takes values in the range 0..3. Each > quadrant maps 1GB virtual address space. > role.access: > - Inherited guest access permissions in the form uwx. Note execute > - permission is positive, not negative. > + Inherited guest access permissions from the parent ptes in the form uwx. > + Note execute permission is positive, not negative. > role.invalid: > The page is invalid and should not be used. It is a root page that is > currently pinned (by a cpu hardware register pointing to it); once it is > diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h > index 7be8a251363e..cebcf7b29b15 100644 > --- a/arch/x86/kvm/paging_tmpl.h > +++ b/arch/x86/kvm/paging_tmpl.h > @@ -100,8 +100,8 @@ struct guest_walker { > gpa_t pte_gpa[PT_MAX_FULL_LEVELS]; > pt_element_t __user *ptep_user[PT_MAX_FULL_LEVELS]; > bool pte_writable[PT_MAX_FULL_LEVELS]; > - unsigned pt_access; > - unsigned pte_access; > + unsigned int pt_access[PT_MAX_FULL_LEVELS]; > + unsigned int pte_access; > gfn_t gfn; > struct x86_exception fault; > }; > @@ -354,6 +354,9 @@ retry_walk: > pte_access = pt_access & FNAME(gpte_access)(vcpu, pte); > > walker->ptes[walker->level - 1] = pte; > + > + /* Convert to ACC_*_MASK flags for struct guest_walker. */ > + walker->pt_access[walker->level - 1] = FNAME(gpte_access)(pt_access ^ walk_nx_mask); > } while (!is_last_gpte(mmu, walker->level, pte)); > > if (unlikely(permission_fault(vcpu, mmu, pte_access, access))) { > @@ -392,10 +395,11 @@ retry_walk: > goto retry_walk; > } > > - walker->pt_access = pt_access; > - walker->pte_access = pte_access; > + walker->pt_access = FNAME(gpte_access)(pt_access ^ walk_nx_mask); > + walker->pte_access = FNAME(gpte_access)(pte_access ^ walk_nx_mask); > pgprintk("%s: pte %llx pte_access %x pt_access %x\n", > - __func__, (u64)pte, pte_access, pt_access); > + __func__, (u64)pte, walker->pte_access, > + walker->pt_access[walker->level - 1]); > return 1; > > error: > @@ -555,7 +559,7 @@ static int FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr, > { > struct kvm_mmu_page *sp = NULL; > struct kvm_shadow_walk_iterator it; > - unsigned direct_access, access = gw->pt_access; > + unsigned int direct_access, access; > int top_level, emulate = 0; > > direct_access = gw->pte_access; > @@ -586,6 +590,7 @@ static int FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr, > sp = NULL; > if (!is_shadow_present_pte(*it.sptep)) { > table_gfn = gw->table_gfn[it.level - 2]; > + access = gw->pt_access[it.level - 2]; > sp = kvm_mmu_get_page(vcpu, table_gfn, addr, it.level-1, > false, access, it.sptep); > } > > Thank you for the patch. I looked at both original patch(b1bd5cba3306691c771d558e94baa73e8b0b96b7) and your's. This patch looks good to me. > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > > > Regards, -- Masami Ichikawa Cybertrust Japan Co., Ltd. Email :masami.ichikawa@cybertrust.co.jp :masami.ichikawa@miraclelinux.com