[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Alex Bennée
Subject: Re: [PATCH 14/26] hw/intc/arm_gicv3_its: Fix various off-by-one errors
Date: Mon, 13 Dec 2021 13:35:01 +0000
User-agent: mu4e 1.7.5; emacs 28.0.90

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]