[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
- Re: [PATCH 18/26] hw/intc/arm_gicv3_its: Fix handling of process_its_cmd() return value, (continued)
- [PATCH 07/26] hw/intc/arm_gicv3_its: Correct setting of TableDesc entry_sz, Peter Maydell, 2021/12/11
- [PATCH 09/26] hw/intc/arm_gicv3_its: Correct handling of MAPI, Peter Maydell, 2021/12/11
- [PATCH 14/26] hw/intc/arm_gicv3_its: Fix various off-by-one errors, Peter Maydell, 2021/12/11
- Re: [PATCH 14/26] hw/intc/arm_gicv3_its: Fix various off-by-one errors,
Alex Bennée <=
- [PATCH 12/26] hw/intc/arm_gicv3_its: Correct comment about CTE RDBase field size, Peter Maydell, 2021/12/11
- [PATCH 17/26] hw/intc/arm_gicv3_its: Convert int ID check to num_intids convention, Peter Maydell, 2021/12/11
- [PATCH 23/26] hw/intc/arm_gicv3_its: Fix return codes in process_mapti(), Peter Maydell, 2021/12/11
- [PATCH 21/26] hw/intc/arm_gicv3_its: Fix return codes in process_its_cmd(), Peter Maydell, 2021/12/11