Re: Re: [PATCH] domain_validate: Check for domain address conflicts fully

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.