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