> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 11 March 2019 09:56 > To: xen-devel <xen-devel@lists.xenproject.org> > Cc: Andrew Cooper <andrew.coop...@citrix.com>; Paul Durrant > <paul.durr...@citrix.com>; Roger Pau Monne > <roger....@citrix.com>; Wei Liu <wei.l...@citrix.com> > Subject: [PATCH] x86/HVM: don't crash guest in hvmemul_find_mmio_cache() > > Commit 35a61c05ea ("x86emul: adjust handling of AVX2 gathers") builds > upon the fact that the domain will actually survive running out of MMIO > result buffer space. Drop the domain_crash() invocation. Also delay > incrementing of the usage counter, such that the function can't possibly > use/return an out-of-bounds slot/pointer in case execution subsequently > makes it into the function again without a prior reset of state. > > Signed-off-by: Jan Beulich <jbeul...@suse.com> > > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -966,12 +966,11 @@ static struct hvm_mmio_cache *hvmemul_fi > return cache; > } > > - i = vio->mmio_cache_count++; > + i = vio->mmio_cache_count; > if( i == ARRAY_SIZE(vio->mmio_cache) ) > - { > - domain_crash(current->domain); > return NULL; > - } > + > + ++vio->mmio_cache_count;
AFAICT this isn't going to stop the for loop at the top of the function accessing one entry beyond the bounds of the array. If you're going to remove the domain_crash() then I think you also need to move the bounds check to the top of the function. Paul > > cache = &vio->mmio_cache[i]; > memset(cache, 0, sizeof (*cache)); > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
