Re: [PATCH RESEND] KVM: x86/mmu: fix UAF in paging_update_accessed_dirty_bits

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 24, 2022, Paolo Bonzini wrote:
> On 1/24/22 18:26, Tadeusz Struk wrote:
> > Syzbot reported an use-after-free bug in update_accessed_dirty_bits().
> > Fix this by checking if the memremap'ed pointer is still valid.
> 
> access_ok only checks that the pointer is in the userspace range.  Is this
> correct?  And if so, what are the exact circumstances in which access_ok
> returns a non-NULL but also non-userspace address?

I "objected" to this patch in its initial posting[*].  AFAICT adding access_ok()
is just masking a more egregious bug where interpretation of vm_pgoff as a PFN
base is flat out wrong except for select backing stores that use VM_PFNMAP.  In
other words, the vm_pgoff hack works for the /dev/mem use case, but it is wrong
in general.

[*] https://lore.kernel.org/all/Ybp0naX%2FZTG9FNEa@xxxxxxxxxx
> 
> Thanks,
> 
> Paolo
> 
> > Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > Cc: Sean Christopherson <seanjc@xxxxxxxxxx>
> > Cc: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> > Cc: Wanpeng Li <wanpengli@xxxxxxxxxxx>
> > Cc: Jim Mattson <jmattson@xxxxxxxxxx>
> > Cc: Joerg Roedel <joro@xxxxxxxxxx>
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > Cc: Borislav Petkov <bp@xxxxxxxxx>
> > Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> > Cc: x86@xxxxxxxxxx
> > Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> > Cc: kvm@xxxxxxxxxxxxxxx
> > Cc: <stable@xxxxxxxxxxxxxxx>
> > Fixes: bd53cb35a3e9 ("X86/KVM: Handle PFNs outside of kernel reach when touching GPTEs")
> > Link: https://syzkaller.appspot.com/bug?id=6cb6102a0a7b0c52060753dd62d070a1d1e71347
> > Reported-by: syzbot+6cde2282daa792c49ab8@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Signed-off-by: Tadeusz Struk <tadeusz.struk@xxxxxxxxxx>
> > ---
> >   arch/x86/kvm/mmu/paging_tmpl.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
> > index 5b5bdac97c7b..d25b72d7b1b1 100644
> > --- a/arch/x86/kvm/mmu/paging_tmpl.h
> > +++ b/arch/x86/kvm/mmu/paging_tmpl.h
> > @@ -174,7 +174,7 @@ static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
> >   		pfn = ((vaddr - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff;
> >   		paddr = pfn << PAGE_SHIFT;
> >   		table = memremap(paddr, PAGE_SIZE, MEMREMAP_WB);
> > -		if (!table) {
> > +		if (!table || !access_ok(table, PAGE_SIZE)) {
> >   			mmap_read_unlock(current->mm);
> >   			return -EFAULT;
> >   		}
> 



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux