hw/intc/arm_gicv3_its: Fix various off-by-one errors
authorPeter Maydell <peter.maydell@linaro.org>
Fri, 7 Jan 2022 17:08:00 +0000 (17:08 +0000)
committerPeter Maydell <peter.maydell@linaro.org>
Fri, 7 Jan 2022 17:08:00 +0000 (17:08 +0000)
commit80dcd37feb3a249cdd6a96826836e267df5c7077
tree1260ca02682c7c64ba96247e6d567d8c468f2a37
parent437dc0ea982beb11cb9b4df82baf8aefe6af661c
hw/intc/arm_gicv3_its: Fix various off-by-one errors

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>
hw/intc/arm_gicv3_its.c
include/hw/intc/arm_gicv3_its_common.h