Re: [PATCH v3] ioreq: Make sure ioreq is always in-bounds

[Jan Beulich <jbeulich@xxxxxxxx>]

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 <julian.vetter@xxxxxxxxxx>
> Signed-off-by: Teddy Astie <teddy.astie@xxxxxxxxxx>

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