Re: [PATCH v2 3/5] hw/arm/smmuv3: Sanitize SMMU_S_STRTAB_BASE_CFG.LOG2SIZE

[Eric Auger <eric.auger@redhat.com>]

Hi Shameer, Peter,
On 7/3/26 2:54 PM, Peter Maydell wrote:
> On Wed, 1 Jul 2026 at 10:12, Eric Auger <eric.auger@redhat.com> wrote:
>> STRTAB_BASE_CFG.LOG2SIZE is programmed by the guest through the
>> emulated SMMUv3 MMIO register interface. Currently the value is
>> not checked and used directly in smmu_find_ste() for strtab_base
>> alignment computation with risk that MAKE_64BIT_MASK() runs out
>> of bounds. On FMT==1 the strtab_size cannot be greater than 64 after
>> the SPLIT being at minimum 6 (log2size is max 64).
>>
>> However on FMT=0 path, strtab_size= log2size + 6 can potentially
>> exceed 64. Let's abort in that case.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3632
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>  hw/arm/smmuv3.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>> index 285e6164a07..5ec3700d0d5 100644
>> --- a/hw/arm/smmuv3.c
>> +++ b/hw/arm/smmuv3.c
>> @@ -1696,6 +1696,14 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr offset,
>>                  s->sid_split = 6;
>>              }
>>              s->features |= SMMU_FEATURE_2LVL_STE;
>> +        } else {
>> +            uint32_t log2size = FIELD_EX32(data, STRTAB_BASE_CFG, LOG2SIZE);
>> +
>> +            if (log2size + 6 > 64) {
>> +                qemu_log_mask(LOG_GUEST_ERROR,
>> +                              "Invalid STRTAB_BASE_CFG.LOG2SIZE: %d", log2size);
>> +                g_assert_not_reached();
> This doesn't look right. Either:
>  (a) the guest can do something silly that gets us to this code path:
>      in that case we mustn't assert, but must continue (doing whatever
>      the spec permits and that is reasonably straightforward to implement)

Sorry for the delay

OK. Then I will just follow Shameer's suggestion and cap strtab_size to 64.

Thanks!

Eric
>  (b) we can't actually get here in practice: in that case the logging
>      isn't needed and we could just assert(log2size + 6 <= 64);
>
> It sounds like we're in case (a) here.
>
> thanks
> -- PMM
>