| Message ID | 20211211191135.1764649-8-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm gicv3 ITS: Various bug fixes and refactorings | expand |
On 12/11/21 11:11 AM, Peter Maydell wrote: > We set the TableDesc entry_sz field from the appropriate > GITS_BASER.ENTRYSIZE field. That ID register field specifies the > number of bytes per table entry minus one. However when we use > td->entry_sz we assume it to be the number of bytes per table entry > (for instance we calculate the number of entries in a page by > dividing the page size by the entry size). > > The effects of this bug are: > * we miscalculate the maximum number of entries in the table, > so our checks on guest index values are wrong (too lax) > * when looking up an entry in the second level of an indirect > table, we calculate an incorrect index into the L2 table. > Because we make the same incorrect calculation on both > reads and writes of the L2 table, the guest won't notice > unless it's unlucky enough to use an index value that > causes us to index off the end of the L2 table page and > cause guest memory corruption in whatever follows > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/intc/arm_gicv3_its.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Peter Maydell <peter.maydell@linaro.org> writes: > We set the TableDesc entry_sz field from the appropriate > GITS_BASER.ENTRYSIZE field. That ID register field specifies the > number of bytes per table entry minus one. However when we use > td->entry_sz we assume it to be the number of bytes per table entry > (for instance we calculate the number of entries in a page by > dividing the page size by the entry size). > > The effects of this bug are: > * we miscalculate the maximum number of entries in the table, > so our checks on guest index values are wrong (too lax) > * when looking up an entry in the second level of an indirect > table, we calculate an incorrect index into the L2 table. > Because we make the same incorrect calculation on both > reads and writes of the L2 table, the guest won't notice > unless it's unlucky enough to use an index value that > causes us to index off the end of the L2 table page and > cause guest memory corruption in whatever follows > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index 84808b1e298..88f4d730999 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -829,7 +829,7 @@ static void extract_table_params(GICv3ITSState *s) } td->page_sz = page_sz; td->indirect = FIELD_EX64(value, GITS_BASER, INDIRECT); - td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE); + td->entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE) + 1; td->base_addr = baser_base_addr(value, page_sz); if (!td->indirect) { td->max_entries = (num_pages * page_sz) / td->entry_sz;
We set the TableDesc entry_sz field from the appropriate GITS_BASER.ENTRYSIZE field. That ID register field specifies the number of bytes per table entry minus one. However when we use td->entry_sz we assume it to be the number of bytes per table entry (for instance we calculate the number of entries in a page by dividing the page size by the entry size). The effects of this bug are: * we miscalculate the maximum number of entries in the table, so our checks on guest index values are wrong (too lax) * when looking up an entry in the second level of an indirect table, we calculate an incorrect index into the L2 table. Because we make the same incorrect calculation on both reads and writes of the L2 table, the guest won't notice unless it's unlucky enough to use an index value that causes us to index off the end of the L2 table page and cause guest memory corruption in whatever follows Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_its.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)