On 10/4/2026 12:18 PM, Tan Chi wrote:
riscv_iommu_translate() allocates an IOATC entry before passing it to
riscv_iommu_iot_update(). The update helper normally transfers ownership
to the hash table.
When ioatc-limit is zero, however, the helper returns without inserting or
freeing the entry. Every cacheable non-identity translation therefore leaks
one RISCVIOMMUEntry.
Free the entry on the disabled-cache path so that
riscv_iommu_iot_update() consistently consumes the entry passed to it.
Fixes: 9d085a1c3cb2 ("hw/riscv/riscv-iommu: add Address Translation Cache
(IOATC)")
Cc: [email protected]
Signed-off-by: Tan Chi <[email protected]>
---
hw/riscv/riscv-iommu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
index 323a041b4a..ebf8d7006c 100644
--- a/hw/riscv/riscv-iommu.c
+++ b/hw/riscv/riscv-iommu.c
@@ -1700,6 +1700,7 @@ static void riscv_iommu_iot_update(RISCVIOMMUState *s,
GHashTable *iot_cache, RISCVIOMMUEntry *iot)
{
if (!s->iot_limit) {
+ g_free(iot);
return;
}
It is better if we could avoid g_freeing() stuff from other functions to avoid
cases where we need to fix freeing logic for a certain pointer all over the
code.
I see that riscv_iommu_iot_update() is being gated by these checks in
riscv_iommu_translate():
if (!fault && iotlb->translated_addr != iotlb->iova && enable_cache) {
The 'enable_cache' flag is a flag set by callers of riscv_iommu_translate() in
two
instances:
- in riscv_iommu_process_dbg(), always set to 'false':
int fault = riscv_iommu_translate(s, ctx, &iotlb, false);
- in riscv_iommu_memory_region_translate(), always set to 'true':
} else if (riscv_iommu_translate(as->iommu, ctx, &iotlb, true)) {
The use in riscv_iommu_process_dbg() is correct because we don't cache
debug messages. But the other one isn't - s->iot_limit is a property set
during init(), i.e. it won't change during the iommu lifecycle, and we
can't have cache if s->iot_limit is 0.
So I think we can do this instead:
- } else if (riscv_iommu_translate(as->iommu, ctx, &iotlb, true)) {
+ } else if (riscv_iommu_translate(as->iommu, ctx, &iotlb,
+ s->iot_limit > 0 ? true : false)) {
And we're fixing the leak you're detected while also making the logic more
sound,
if ever so slightly.
In fact I think there's more improvements to be made w.r.t s->iot_limit, i.e.
if it's a known zero why bother allocating s->iot_cache in the first place,
but that's something out of the scope for this patch.
Thanks,
Daniel
