[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aH_RWQ_YqlydOkKH@willie-the-truck>
Date: Tue, 22 Jul 2025 18:58:49 +0100
From: Will Deacon <will@...nel.org>
To: Seyediman Seyedarab <imandevel@...il.com>
Cc: dwmw2@...radead.org, baolu.lu@...ux.intel.com, joro@...tes.org,
robin.murphy@....com, skhan@...uxfoundation.org,
linux-kernel-mentees@...ts.linux.dev, iommu@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iommu/vt-d: replace snprintf with scnprintf in
dmar_latency_snapshot()
On Tue, Jul 22, 2025 at 09:11:17AM -0400, Seyediman Seyedarab wrote:
> snprintf returns the number of bytes that would have been written,
> not the number actually written to the buffer. When accumulating
> the byte count with the return value of snprintf, this can cause
> the offset to exceed the actual buffer size if truncation occurs.
>
> The byte count is passed to seq_puts() in latency_show_one() with-
> out checking for truncation.
>
> Replace snprintf with scnprintf, ensuring the buffer offset stays
> within bound.
>
> Signed-off-by: Seyediman Seyedarab <ImanDevel@...il.com>
> ---
> drivers/iommu/intel/perf.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/intel/perf.c b/drivers/iommu/intel/perf.c
> index adc4de6bb..cee4821f4 100644
> --- a/drivers/iommu/intel/perf.c
> +++ b/drivers/iommu/intel/perf.c
> @@ -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,7 +156,7 @@ 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);
Should the check of the return value in latency_show_one() also be
adjusted so that 'ret <= 0' is an error? I couldn't convince myself
that the string in 'debug_buf' is always null-terminated if ret == 0.
Will
Powered by blists - more mailing lists