> -----Original Message----- > From: Jan Beulich [mailto:[email protected]] > Sent: 11 March 2019 09:56 > To: xen-devel <[email protected]> > Cc: Andrew Cooper <[email protected]>; Paul Durrant > <[email protected]>; Roger Pau Monne > <[email protected]>; Wei Liu <[email protected]> > 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 <[email protected]> > > --- 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 [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
