From: Seyediman Seyedarab ImanDevel@gmail.com
[ Upstream commit 75c02a037609f34db17e91be195cedb33b61bae0 ]
snprintf() returns the number of bytes that would have been written, not the number actually written. Using this for offset tracking can cause buffer overruns if truncation occurs.
Replace snprintf() with scnprintf() to ensure the offset stays within bounds.
Since scnprintf() never returns a negative value, and zero is not possible in this context because 'bytes' starts at 0 and 'size - bytes' is DEBUG_BUFFER_SIZE in the first call, which is large enough to hold the string literals used, the return value is always positive. An integer overflow is also completely out of reach here due to the small and fixed buffer size. The error check in latency_show_one() is therefore unnecessary. Remove it and make dmar_latency_snapshot() return void.
Signed-off-by: Seyediman Seyedarab ImanDevel@gmail.com Link: https://lore.kernel.org/r/20250731225048.131364-1-ImanDevel@gmail.com Signed-off-by: Lu Baolu baolu.lu@linux.intel.com Signed-off-by: Joerg Roedel joerg.roedel@amd.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES – This fixes a real memory-corruption bug in the Intel VT-d debugfs latency snapshot and the change is tight, self-contained, and low risk for stable.
- `drivers/iommu/intel/perf.c:116-160` now uses `scnprintf` for every append into `debug_buf`. Previously `snprintf` advanced the `bytes` cursor by the would-have-been length; once the counters grew large enough to truncate a write, `bytes` could exceed `size`, and the next `snprintf(str + bytes, size - bytes, …)` would wrap the length argument and scribble past the 1 KB buffer. That overflow is a latent kernel memory corruption reachable from the `dmar_perf_latency` debugfs file. With `scnprintf`, the offset can no longer run past the remaining space, eliminating the corruption risk. - `drivers/iommu/intel/debugfs.c:659-666` drops the dead `< 0` error handling and simply prints the buffer, matching the new `void` return semantics and avoiding bogus “Failed to get latency snapshot” messages. - `drivers/iommu/intel/perf.h:37-70` updates the prototype and the `!CONFIG_DMAR_PERF` stub accordingly so all callers build cleanly; no other interfaces or architectures are touched.
The bug exists in all trees that have Intel IOMMU debugfs (`CONFIG_INTEL_IOMMU_DEBUGFS` selects this code) and can be triggered by routine use once latency counters accumulate large values. The fix is entirely in debug/perf code, introduces no behavioural changes beyond removing the overflow, and carries negligible regression risk. Backporting is recommended.
drivers/iommu/intel/debugfs.c | 10 ++-------- drivers/iommu/intel/perf.c | 10 ++++------ drivers/iommu/intel/perf.h | 5 ++--- 3 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c index 5aa7f46a420b5..38790ff50977c 100644 --- a/drivers/iommu/intel/debugfs.c +++ b/drivers/iommu/intel/debugfs.c @@ -661,17 +661,11 @@ DEFINE_SHOW_ATTRIBUTE(ir_translation_struct); static void latency_show_one(struct seq_file *m, struct intel_iommu *iommu, struct dmar_drhd_unit *drhd) { - int ret; - seq_printf(m, "IOMMU: %s Register Base Address: %llx\n", iommu->name, drhd->reg_base_addr);
- ret = dmar_latency_snapshot(iommu, debug_buf, DEBUG_BUFFER_SIZE); - if (ret < 0) - seq_puts(m, "Failed to get latency snapshot"); - else - seq_puts(m, debug_buf); - seq_puts(m, "\n"); + dmar_latency_snapshot(iommu, debug_buf, DEBUG_BUFFER_SIZE); + seq_printf(m, "%s\n", debug_buf); }
static int latency_show(struct seq_file *m, void *v) diff --git a/drivers/iommu/intel/perf.c b/drivers/iommu/intel/perf.c index adc4de6bbd88e..dceeadc3ee7cd 100644 --- a/drivers/iommu/intel/perf.c +++ b/drivers/iommu/intel/perf.c @@ -113,7 +113,7 @@ static char *latency_type_names[] = { " svm_prq" };
-int dmar_latency_snapshot(struct intel_iommu *iommu, char *str, size_t size) +void dmar_latency_snapshot(struct intel_iommu *iommu, char *str, size_t size) { struct latency_statistic *lstat = iommu->perf_statistic; unsigned long flags; @@ -122,7 +122,7 @@ int dmar_latency_snapshot(struct intel_iommu *iommu, char *str, size_t size) memset(str, 0, size);
for (i = 0; i < COUNTS_NUM; i++) - bytes += snprintf(str + bytes, size - bytes, + bytes += scnprintf(str + bytes, size - bytes, "%s", latency_counter_names[i]);
spin_lock_irqsave(&latency_lock, flags); @@ -130,7 +130,7 @@ int dmar_latency_snapshot(struct intel_iommu *iommu, char *str, size_t size) if (!dmar_latency_enabled(iommu, i)) continue;
- bytes += snprintf(str + bytes, size - bytes, + bytes += scnprintf(str + bytes, size - bytes, "\n%s", latency_type_names[i]);
for (j = 0; j < COUNTS_NUM; j++) { @@ -156,11 +156,9 @@ int dmar_latency_snapshot(struct intel_iommu *iommu, char *str, size_t size) break; }
- bytes += snprintf(str + bytes, size - bytes, + bytes += scnprintf(str + bytes, size - bytes, "%12lld", val); } } spin_unlock_irqrestore(&latency_lock, flags); - - return bytes; } diff --git a/drivers/iommu/intel/perf.h b/drivers/iommu/intel/perf.h index df9a36942d643..1d4baad7e852e 100644 --- a/drivers/iommu/intel/perf.h +++ b/drivers/iommu/intel/perf.h @@ -40,7 +40,7 @@ void dmar_latency_disable(struct intel_iommu *iommu, enum latency_type type); bool dmar_latency_enabled(struct intel_iommu *iommu, enum latency_type type); void dmar_latency_update(struct intel_iommu *iommu, enum latency_type type, u64 latency); -int dmar_latency_snapshot(struct intel_iommu *iommu, char *str, size_t size); +void dmar_latency_snapshot(struct intel_iommu *iommu, char *str, size_t size); #else static inline int dmar_latency_enable(struct intel_iommu *iommu, enum latency_type type) @@ -64,9 +64,8 @@ dmar_latency_update(struct intel_iommu *iommu, enum latency_type type, u64 laten { }
-static inline int +static inline void dmar_latency_snapshot(struct intel_iommu *iommu, char *str, size_t size) { - return 0; } #endif /* CONFIG_DMAR_PERF */