| Message ID | 20220111171048.3545974-14-peter.maydell@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm gicv3 ITS: Various bug fixes and refactorings | expand |
Peter Maydell <peter.maydell@linaro.org> writes: > In process_its_cmd(), we read an ICID out of the interrupt table > entry, and then use it as an index into the collection table. Add a > check that it is within range for the collection table first. > > This check is not strictly necessary, because: > * we range check the ICID from the guest before writing it into > the interrupt table entry, so the the only way to get an > out of range ICID in process_its_cmd() is if a badly-behaved > guest is writing directly to the interrupt table memory > * the collection table is in guest memory, so QEMU won't fall > over if we read off the end of it > > However, it seems clearer to include the check. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
On 1/12/22 04:10, Peter Maydell wrote: > In process_its_cmd(), we read an ICID out of the interrupt table > entry, and then use it as an index into the collection table. Add a > check that it is within range for the collection table first. > > This check is not strictly necessary, because: > * we range check the ICID from the guest before writing it into > the interrupt table entry, so the the only way to get an > out of range ICID in process_its_cmd() is if a badly-behaved > guest is writing directly to the interrupt table memory > * the collection table is in guest memory, so QEMU won't fall > over if we read off the end of it > > However, it seems clearer to include the check. > > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > hw/intc/arm_gicv3_its.c | 7 +++++++ > 1 file changed, 7 insertions(+) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index ef6c0f55ff9..b2f6a8c7f00 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -299,6 +299,13 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, return CMD_CONTINUE; } + if (icid >= s->ct.num_ids) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid ICID 0x%x in ITE (table corrupted?)\n", + __func__, icid); + return CMD_CONTINUE; + } + cte_valid = get_cte(s, icid, &cte, &res); if (res != MEMTX_OK) { return CMD_STALL;
In process_its_cmd(), we read an ICID out of the interrupt table entry, and then use it as an index into the collection table. Add a check that it is within range for the collection table first. This check is not strictly necessary, because: * we range check the ICID from the guest before writing it into the interrupt table entry, so the the only way to get an out of range ICID in process_its_cmd() is if a badly-behaved guest is writing directly to the interrupt table memory * the collection table is in guest memory, so QEMU won't fall over if we read off the end of it However, it seems clearer to include the check. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_its.c | 7 +++++++ 1 file changed, 7 insertions(+)