On 17.11.2025 13:35, Teddy Astie wrote: > A 4K page appears to be able to hold 128 ioreq entries, which luckly > matches the current vCPU limit. However, if we decide to increase the > domain vCPU limit, that doesn't hold anymore and this function would now > silently create a out of bounds pointer leading to confusing problems. > > All architectures with ioreq support don't support 128 vCPU limit for > HVM guests, and have pages that are at least 4 KB large, so this case > doesn't occurs in with the current limits. > > For the time being, make sure we can't make a Xen build that can > accidentally make a out of bounds pointers here. > > No functional change. > > Reported-by: Julian Vetter <[email protected]> > Signed-off-by: Teddy Astie <[email protected]>
I was meaning to ack this, but ... > --- a/xen/common/ioreq.c > +++ b/xen/common/ioreq.c > @@ -99,6 +99,7 @@ static ioreq_t *get_ioreq(struct ioreq_server *s, struct > vcpu *v) > > ASSERT((v == current) || !vcpu_runnable(v)); > ASSERT(p != NULL); > + BUILD_BUG_ON(HVM_MAX_VCPUS > (PAGE_SIZE / sizeof(struct ioreq))); ... does this even build on e.g. Arm? IOREQ_SERVER is a setting which can be enabled (with EXPERT=y) also for non-x86. Yet HVM_MAX_VCPUS looks to be an x86-only thing. (I then also wonder about some of what the description says). Just to mention (no further change requested at this point, in this regard): HVM_MAX_VCPUS being part of the public interface, we'll need to see whether we can sensibly retain that identifier to carry changed meaning once we up the limit. The check here may therefore not trigger at that point; the hope then is that while making respective changes, people would at least stumble across it by e.g. seeing it in grep output. Jan
