On Mon, 25 Sept 2023 at 20:41, Vladimir Sementsov-Ogievskiy
<vsement...@yandex-team.ru> wrote:
>
> Add a constant and clear assertion. The assertion also tells Coverity
> that we are not going to overflow the array.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
> ---
> hw/i386/intel_iommu.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index c0ce896668..2233dbe13a 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -1028,12 +1028,17 @@ static dma_addr_t
> vtd_get_iova_pgtbl_base(IntelIOMMUState *s,
> * vtd_spte_rsvd 4k pages
> * vtd_spte_rsvd_large large pages
> */
> -static uint64_t vtd_spte_rsvd[5];
> -static uint64_t vtd_spte_rsvd_large[5];
> +#define VTD_SPTE_RSVD_LEN 5
> +static uint64_t vtd_spte_rsvd[VTD_SPTE_RSVD_LEN];
> +static uint64_t vtd_spte_rsvd_large[VTD_SPTE_RSVD_LEN];
>
> static bool vtd_slpte_nonzero_rsvd(uint64_t slpte, uint32_t level)
> {
> - uint64_t rsvd_mask = vtd_spte_rsvd[level];
> + uint64_t rsvd_mask;
> +
> + assert(level < VTD_SPTE_RSVD_LEN);
> +
> + rsvd_mask = vtd_spte_rsvd[level];
Looking at the code it is not clear to me why this assertion is
valid. It looks like we are picking up fields from guest-set
configuration (probably in-memory data structures). So we can't
assert() here -- we need to do whatever the real hardware does
if these fields are set to an incorrect value, or at least something
sensible that doesn't crash QEMU.
thanks
-- PMM
