Re: [PATCH 14/26] hw/intc/arm_gicv3_its: Fix various off-by-one errors

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> The ITS code has to check whether various parameters passed in
> commands are in-bounds, where the limit is defined in terms of the
> number of bits that are available for the parameter.  (For example,
> the GITS_TYPER.Devbits ID register field specifies the number of
> DeviceID bits minus 1, and device IDs passed in the MAPTI and MAPD
> command packets must fit in that many bits.)
>
> Currently we have off-by-one bugs in many of these bounds checks.
> The typical problem is that we define a max_foo as 1 << n. In
> the Devbits example, we set
>   s->dt.max_ids = 1UL << (GITS_TYPER.Devbits + 1).
> However later when we do the bounds check we write
>   if (devid > s->dt.max_ids) { /* command error */ }
> which incorrectly permits a devid of 1 << n.
>
> These bugs will not cause QEMU crashes because the ID values being
> checked are only used for accesses into tables held in guest memory
> which we access with address_space_*() functions, but they are
> incorrect behaviour of our emulation.
>
> Fix them by standardizing on this pattern:
>  * bounds limits are named num_foos and are the 2^n value
>    (equal to the number of valid foo values)
>  * bounds checks are either
>    if (fooid < num_foos) { good }
>    or
>    if (fooid >= num_foos) { bad }
>
> In this commit we fix the handling of the number of IDs
> in the device table and the collection table, and the number
> of commands that will fit in the command queue.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée