>>> On 11.03.19 at 11:11, <[email protected]> wrote:
>> -----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.
I don't follow:
static struct hvm_mmio_cache *hvmemul_find_mmio_cache(
struct hvm_vcpu_io *vio, unsigned long gla, uint8_t dir)
{
unsigned int i;
struct hvm_mmio_cache *cache;
for ( i = 0; i < vio->mmio_cache_count; i ++ )
This iterates up to (but not including) the recorded count of
populated cache entries.
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
