[PATCH v2 0/5] SMMUV3: STE lookup fixes

[PATCH v2 0/5] SMMUV3: STE lookup fixes

[Eric Auger]

The current code fails to sanitize SMMU_S_STRTAB_BASE_CFG
LOG2SIZE and SPLIT which are used in the STE lookup. This
could potentially lead to wrong shifts/masks in
smmu_find_ste() and does not fully comply with the spec.

Also the series fixes different issues related to strtab base
address alignment computations: off-by-one mask, unchecked span,
missing L2ptr base address alignment. Those issues
were not visible because the guest kernel does what it should
but better comply with the spec.

Best Regards

Eric

Eric Auger (5):
  hw/arm/smmuv3: Fix off-by-one bug in alignment strtab mask
  hw/arm/smmuv3: Sanitize SMMU_S_STRTAB_BASE_CFG.SPLIT
  hw/arm/smmuv3: Sanitize SMMU_S_STRTAB_BASE_CFG.LOG2SIZE
  hw/arm/smmuv3: Check L1STD.SPAN
  hw/arm/smmuv3: Enforce alignment of L2Ptr according to the span

 hw/arm/smmuv3.c | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

-- 
2.53.0

[PATCH v2 1/5] hw/arm/smmuv3: Fix off-by-one bug in alignment strtab mask

[Eric Auger]

The stream table base address needs to be aligned to its size.

With FMT == 0, the spec says the linear stream table base address
musst have ADDR[LOG2SIZE + 5:0] = 0. STE are 64B

With FMT == 1, ADDR[MAX(5, (LOG2SIZE - SPLIT - 1 + 3)):0] = 0.
L1 descriptors are 8B.

MAKE_64BIT_MASK() second argument is a size and not a shift, so
there is an off-by-one computation.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/arm/smmuv3.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 5e5a6a960c9..38aba9c0af9 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -664,7 +664,7 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, SMMUEventInfo *event)
 {
     dma_addr_t addr, strtab_base;
     uint32_t log2size;
-    int strtab_size_shift;
+    int strtab_size;
     int ret;
 
     trace_smmuv3_find_ste(sid, s->features, s->sid_split);
@@ -685,9 +685,9 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, SMMUEventInfo *event)
          * Align strtab base address to table size. For this purpose, assume it
          * is not bounded by SMMU_IDR1_SIDSIZE.
          */
-        strtab_size_shift = MAX(5, (int)log2size - s->sid_split - 1 + 3);
+        strtab_size = MAX(6, (int)log2size - s->sid_split + 3);
         strtab_base = s->strtab_base & SMMU_BASE_ADDR_MASK &
-                      ~MAKE_64BIT_MASK(0, strtab_size_shift);
+                      ~MAKE_64BIT_MASK(0, strtab_size);
         l1_ste_offset = sid >> s->sid_split;
         l2_ste_offset = sid & ((1 << s->sid_split) - 1);
         l1ptr = (dma_addr_t)(strtab_base + l1_ste_offset * sizeof(l1std));
@@ -729,9 +729,9 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, SMMUEventInfo *event)
         }
         addr = l2ptr + l2_ste_offset * sizeof(*ste);
     } else {
-        strtab_size_shift = log2size + 5;
+        strtab_size = log2size + 6;
         strtab_base = s->strtab_base & SMMU_BASE_ADDR_MASK &
-                      ~MAKE_64BIT_MASK(0, strtab_size_shift);
+                      ~MAKE_64BIT_MASK(0, strtab_size);
         addr = strtab_base + sid * sizeof(*ste);
     }
 
-- 
2.53.0

[PATCH v2 2/5] hw/arm/smmuv3: Sanitize SMMU_S_STRTAB_BASE_CFG.SPLIT

[Eric Auger]

Currently the guest value for the SPLIT field is not checked.
Also the spec says that values different from 6, 8, 10, respectively
meaning 4KB, 16kB and 64kB leaf tables are reserved and behave as 6.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3632
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 hw/arm/smmuv3.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 38aba9c0af9..285e6164a07 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1688,6 +1688,13 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr offset,
         s->strtab_base_cfg = data;
         if (FIELD_EX32(data, STRTAB_BASE_CFG, FMT) == 1) {
             s->sid_split = FIELD_EX32(data, STRTAB_BASE_CFG, SPLIT);
+            if (s->sid_split != 6 && s->sid_split != 8 && s->sid_split != 10) {
+                /* Other values are reserved, behave as 6 */
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "Invalid STRTAB_BASE_CFG.SPLIT=%u, use 6 instead\n",
+                              s->sid_split);
+                s->sid_split = 6;
+            }
             s->features |= SMMU_FEATURE_2LVL_STE;
         }
         break;
-- 
2.53.0

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

[Eric Auger]

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();
+            }
         }
         break;
     case A_CMDQ_BASE: /* 64b */
-- 
2.53.0

[PATCH v2 4/5] hw/arm/smmuv3: Check L1STD.SPAN

[Eric Auger]

Span values above 11 are reserved and behave as 0.

Also span must be within the range of 0 to (SMMU_STRTAB_BASE_CFG.SPLIT + 1),
ie. it must stay within the bounds of the stream table split point.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/arm/smmuv3.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 5ec3700d0d5..f694f832598 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -707,7 +707,7 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, SMMUEventInfo *event)
 
         span = L1STD_SPAN(&l1std);
 
-        if (!span) {
+        if (!span || span > 11) {
             /* l2ptr is not valid */
             if (!event->inval_ste_allowed) {
                 qemu_log_mask(LOG_GUEST_ERROR,
@@ -716,6 +716,16 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, SMMUEventInfo *event)
             event->type = SMMU_EVT_C_BAD_STREAMID;
             return -EINVAL;
         }
+
+        if (span > s->sid_split + 1) {
+            if (!event->inval_ste_allowed) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "invalid span (0x%x)\n", span);
+            }
+            event->type = SMMU_EVT_C_BAD_STREAMID;
+            return -EINVAL;
+        }
+
         max_l2_ste = (1 << span) - 1;
         l2ptr = l1std_l2ptr(&l1std);
         trace_smmuv3_find_ste_2lvl(s->strtab_base, l1ptr, l1_ste_offset,
-- 
2.53.0

[PATCH v2 5/5] hw/arm/smmuv3: Enforce alignment of L2Ptr according to the span

[Eric Auger]

Spec says: Bits L2Ptr[N:0] are treated as 0 by the SMMU, where
N == 5 + (Span - 1).

Let's enforce this alignment.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/arm/smmuv3.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index f694f832598..7eb2c59d9eb 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -728,6 +728,8 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, SMMUEventInfo *event)
 
         max_l2_ste = (1 << span) - 1;
         l2ptr = l1std_l2ptr(&l1std);
+
+        l2ptr &= ~MAKE_64BIT_MASK(0, 6 + (span - 1));
         trace_smmuv3_find_ste_2lvl(s->strtab_base, l1ptr, l1_ste_offset,
                                    l2ptr, l2_ste_offset, max_l2_ste);
         if (l2_ste_offset > max_l2_ste) {
-- 
2.53.0

RE: [PATCH v2 1/5] hw/arm/smmuv3: Fix off-by-one bug in alignment strtab mask

[Shameer Kolothum Thodi]

> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: 01 July 2026 10:12
> To: eric.auger.pro@gmail.com; eric.auger@redhat.com; qemu-
> devel@nongnu.org; qemu-arm@nongnu.org; peter.maydell@linaro.org;
> berrange@redhat.com; Shameer Kolothum Thodi
> <skolothumtho@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; Nathan
> Chen <nathanc@nvidia.com>
> Subject: [PATCH v2 1/5] hw/arm/smmuv3: Fix off-by-one bug in alignment
> strtab mask
> 
> External email: Use caution opening links or attachments
> 
> 
> The stream table base address needs to be aligned to its size.
> 
> With FMT == 0, the spec says the linear stream table base address
> musst have ADDR[LOG2SIZE + 5:0] = 0. STE are 64B
> 
> With FMT == 1, ADDR[MAX(5, (LOG2SIZE - SPLIT - 1 + 3)):0] = 0.
> L1 descriptors are 8B.
> 
> MAKE_64BIT_MASK() second argument is a size and not a shift, so
> there is an off-by-one computation.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  hw/arm/smmuv3.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 5e5a6a960c9..38aba9c0af9 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -664,7 +664,7 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE
> *ste, SMMUEventInfo *event)
>  {
>      dma_addr_t addr, strtab_base;
>      uint32_t log2size;
> -    int strtab_size_shift;
> +    int strtab_size;
>      int ret;
> 
>      trace_smmuv3_find_ste(sid, s->features, s->sid_split);
> @@ -685,9 +685,9 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE
> *ste, SMMUEventInfo *event)
>           * Align strtab base address to table size. For this purpose, assume it
>           * is not bounded by SMMU_IDR1_SIDSIZE.
>           */
> -        strtab_size_shift = MAX(5, (int)log2size - s->sid_split - 1 + 3);
> +        strtab_size = MAX(6, (int)log2size - s->sid_split + 3);
>          strtab_base = s->strtab_base & SMMU_BASE_ADDR_MASK &
> -                      ~MAKE_64BIT_MASK(0, strtab_size_shift);
> +                      ~MAKE_64BIT_MASK(0, strtab_size);
>          l1_ste_offset = sid >> s->sid_split;
>          l2_ste_offset = sid & ((1 << s->sid_split) - 1);
>          l1ptr = (dma_addr_t)(strtab_base + l1_ste_offset * sizeof(l1std));
> @@ -729,9 +729,9 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE
> *ste, SMMUEventInfo *event)
>          }
>          addr = l2ptr + l2_ste_offset * sizeof(*ste);
>      } else {
> -        strtab_size_shift = log2size + 5;
> +        strtab_size = log2size + 6;
>          strtab_base = s->strtab_base & SMMU_BASE_ADDR_MASK &
> -                      ~MAKE_64BIT_MASK(0, strtab_size_shift);
> +                      ~MAKE_64BIT_MASK(0, strtab_size);

Should we cap the strtab_size to 64 here? 

A log2size=59 will make strtab_size=65 and MAKE_64BIT_MASK
may return invalid in that case.

Thanks,
Shameer

Re: [PATCH v2 1/5] hw/arm/smmuv3: Fix off-by-one bug in alignment strtab mask

[Philippe Mathieu-Daudé]

Hi Eric,

On 1/7/26 11:11, Eric Auger wrote:
> The stream table base address needs to be aligned to its size.
> 
> With FMT == 0, the spec says the linear stream table base address
> musst have ADDR[LOG2SIZE + 5:0] = 0. STE are 64B
> 
> With FMT == 1, ADDR[MAX(5, (LOG2SIZE - SPLIT - 1 + 3)):0] = 0.
> L1 descriptors are 8B.
> 
> MAKE_64BIT_MASK() second argument is a size and not a shift, so
> there is an off-by-one computation.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   hw/arm/smmuv3.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 5e5a6a960c9..38aba9c0af9 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -664,7 +664,7 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, SMMUEventInfo *event)
>   {
>       dma_addr_t addr, strtab_base;
>       uint32_t log2size;
> -    int strtab_size_shift;
> +    int strtab_size;
>       int ret;
>   
>       trace_smmuv3_find_ste(sid, s->features, s->sid_split);
> @@ -685,9 +685,9 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, SMMUEventInfo *event)
>            * Align strtab base address to table size. For this purpose, assume it
>            * is not bounded by SMMU_IDR1_SIDSIZE.
>            */
> -        strtab_size_shift = MAX(5, (int)log2size - s->sid_split - 1 + 3);
> +        strtab_size = MAX(6, (int)log2size - s->sid_split + 3);
>           strtab_base = s->strtab_base & SMMU_BASE_ADDR_MASK &
> -                      ~MAKE_64BIT_MASK(0, strtab_size_shift);
> +                      ~MAKE_64BIT_MASK(0, strtab_size);
>           l1_ste_offset = sid >> s->sid_split;
>           l2_ste_offset = sid & ((1 << s->sid_split) - 1);
>           l1ptr = (dma_addr_t)(strtab_base + l1_ste_offset * sizeof(l1std));
> @@ -729,9 +729,9 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, SMMUEventInfo *event)
>           }
>           addr = l2ptr + l2_ste_offset * sizeof(*ste);
>       } else {
> -        strtab_size_shift = log2size + 5;
> +        strtab_size = log2size + 6;
>           strtab_base = s->strtab_base & SMMU_BASE_ADDR_MASK &
> -                      ~MAKE_64BIT_MASK(0, strtab_size_shift);
> +                      ~MAKE_64BIT_MASK(0, strtab_size);
>           addr = strtab_base + sid * sizeof(*ste);
>       }
>   

Maybe in a preliminary patch extract this magic '5' value to a
self-describing #definition?

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

[Shameer Kolothum Thodi]

> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: 01 July 2026 10:12
> To: eric.auger.pro@gmail.com; eric.auger@redhat.com; qemu-
> devel@nongnu.org; qemu-arm@nongnu.org; peter.maydell@linaro.org;
> berrange@redhat.com; Shameer Kolothum Thodi
> <skolothumtho@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; Nathan
> Chen <nathanc@nvidia.com>
> Subject: [PATCH v2 3/5] hw/arm/smmuv3: Sanitize
> SMMU_S_STRTAB_BASE_CFG.LOG2SIZE
> 
> External email: Use caution opening links or attachments
> 
> 
> 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();

Ah..this will address the bound check issue I mentioned in patch #1 I guess.
But this will terminate the Guest. If we limit the strtab_size = MIN(strtab_size, 64)
in patch #1 and patch #2 sanitises the SPLIT, do we need this patch now?

Thanks,
Shameer

> +            }
>          }
>          break;
>      case A_CMDQ_BASE: /* 64b */
> --
> 2.53.0

RE: [PATCH v2 4/5] hw/arm/smmuv3: Check L1STD.SPAN

[Shameer Kolothum Thodi]

> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: 01 July 2026 10:12
> To: eric.auger.pro@gmail.com; eric.auger@redhat.com; qemu-
> devel@nongnu.org; qemu-arm@nongnu.org; peter.maydell@linaro.org;
> berrange@redhat.com; Shameer Kolothum Thodi
> <skolothumtho@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; Nathan
> Chen <nathanc@nvidia.com>
> Subject: [PATCH v2 4/5] hw/arm/smmuv3: Check L1STD.SPAN
> 
> External email: Use caution opening links or attachments
> 
> 
> Span values above 11 are reserved and behave as 0.
> 
> Also span must be within the range of 0 to (SMMU_STRTAB_BASE_CFG.SPLIT
> + 1),
> ie. it must stay within the bounds of the stream table split point.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>

Thanks,
Shameer

> ---
>  hw/arm/smmuv3.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 5ec3700d0d5..f694f832598 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -707,7 +707,7 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid,
> STE *ste, SMMUEventInfo *event)
> 
>          span = L1STD_SPAN(&l1std);
> 
> -        if (!span) {
> +        if (!span || span > 11) {
>              /* l2ptr is not valid */
>              if (!event->inval_ste_allowed) {
>                  qemu_log_mask(LOG_GUEST_ERROR,
> @@ -716,6 +716,16 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid,
> STE *ste, SMMUEventInfo *event)
>              event->type = SMMU_EVT_C_BAD_STREAMID;
>              return -EINVAL;
>          }
> +
> +        if (span > s->sid_split + 1) {
> +            if (!event->inval_ste_allowed) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                              "invalid span (0x%x)\n", span);
> +            }
> +            event->type = SMMU_EVT_C_BAD_STREAMID;
> +            return -EINVAL;
> +        }
> +
>          max_l2_ste = (1 << span) - 1;
>          l2ptr = l1std_l2ptr(&l1std);
>          trace_smmuv3_find_ste_2lvl(s->strtab_base, l1ptr, l1_ste_offset,
> --
> 2.53.0

RE: [PATCH v2 5/5] hw/arm/smmuv3: Enforce alignment of L2Ptr according to the span

[Shameer Kolothum Thodi]

> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: 01 July 2026 10:12
> To: eric.auger.pro@gmail.com; eric.auger@redhat.com; qemu-
> devel@nongnu.org; qemu-arm@nongnu.org; peter.maydell@linaro.org;
> berrange@redhat.com; Shameer Kolothum Thodi
> <skolothumtho@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; Nathan
> Chen <nathanc@nvidia.com>
> Subject: [PATCH v2 5/5] hw/arm/smmuv3: Enforce alignment of L2Ptr
> according to the span
> 
> External email: Use caution opening links or attachments
> 
> 
> Spec says: Bits L2Ptr[N:0] are treated as 0 by the SMMU, where N == 5 + (Span
> - 1).
> 
> Let's enforce this alignment.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>

Thanks,
Shameer

> ---
>  hw/arm/smmuv3.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index
> f694f832598..7eb2c59d9eb 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -728,6 +728,8 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid,
> STE *ste, SMMUEventInfo *event)
> 
>          max_l2_ste = (1 << span) - 1;
>          l2ptr = l1std_l2ptr(&l1std);
> +
> +        l2ptr &= ~MAKE_64BIT_MASK(0, 6 + (span - 1));
>          trace_smmuv3_find_ste_2lvl(s->strtab_base, l1ptr, l1_ste_offset,
>                                     l2ptr, l2_ste_offset, max_l2_ste);
>          if (l2_ste_offset > max_l2_ste) {
> --
> 2.53.0

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

[Peter Maydell]

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)
 (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

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

[Eric Auger]

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
>