Re: [cip-dev] New CVE entries this week

["Nobuhiro Iwamatsu" <nobuhiro1.iwamatsu@toshiba.co.jp>]

[-- Attachment #1: Type: text/plain, Size: 4467 bytes --]

Hi,

> -----Original Message-----
> From: cip-dev@lists.cip-project.org [mailto:cip-dev@lists.cip-project.org] On Behalf Of Pavel Machek
> Sent: Thursday, August 19, 2021 4:10 PM
> To: cip-dev@lists.cip-project.org
> Subject: Re: [cip-dev] New CVE entries this week
> 
> 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.

Thanks for your work. I just checked this issue.

This probably won't compile because the walk_nx_mask variable isn't well defined.
I think we need to backport a patch for this variable or create another patch.

> 
> Best regards,
> 								Pavel

Best regards,
  Nobuhiro

> 
> 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);
>  		}
> 
> 
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: Type: text/plain, Size: 429 bytes --]


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#6699): https://lists.cip-project.org/g/cip-dev/message/6699
Mute This Topic: https://lists.cip-project.org/mt/84986288/4520388
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/10495289/4520388/727948398/xyzzy [cip-dev@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-