On 1/23/24 10:45, Andrea Bolognani wrote:
> On Fri, Jan 19, 2024 at 04:25:11PM +0100, Michal Privoznik wrote:
>> +++ b/src/conf/domain_validate.c
>> @@ -2315,7 +2316,10 @@ virDomainMemoryDefCheckConflict(const virDomainMemoryDef *mem,
>> if (thisStart == 0 || otherStart == 0)
>> continue;
>>
>> - if (thisStart <= otherStart && thisEnd > otherStart) {
>> + otherEnd = otherStart + other->size;
>
> Shouldn't you multiply other->size by 1024? That's what happens
> earlier with mem->size.
D'oh! Of course. Nice catch.
>
> I'm also curious about the zero check on thisStart and otherStart
> right above that. It looks like it would allow two overlapping memory
> devices to exists as long as either of them starts at zero, but I've
> certainly missed something in related code that makes that scenario
> impossible.
>
There are two ways in which thisStart/otherStart can be zero:
1) It's initialized to 0 and never changed => we're dealing with a
memory device model that doesn't support setting addresses (e.g. SGX), or
2) it was set to zero => it's basically a not-user-set default, i.e.
it's formatted into XML iff != zero (see virDomainDeviceInfoFormat(),
virDomainMemoryTargetDefFormat()).
IOW, this == 0 comparison checks if an address was set by user and only
then check is performed. But truth to be told - we don't deny those
values during parsing. They are silently ignored (and it's probably too
late to change that). But I guess nobody want's to map a memory onto 0x0
anyway.
Michal
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
