* [PATCH v2] drm/msm: Replace unsafe snprintf usage with scnprintf
@ 2025-12-24 12:44 veygax
2025-12-24 15:37 ` Dmitry Baryshkov
2025-12-24 15:55 ` Dmitry Baryshkov
0 siblings, 2 replies; 3+ messages in thread
From: veygax @ 2025-12-24 12:44 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark
Cc: Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, linux-arm-msm, dri-devel, freedreno,
linux-kernel, Evan Lambert
From: Evan Lambert <veyga@veygax.dev>
The refill_buf function uses snprintf to append to a fixed-size buffer.
snprintf returns the length that would have been written, which can
exceed the remaining buffer size. If this happens, ptr advances beyond
the buffer and rem becomes negative. In the 2nd iteration, rem is
treated as a large unsigned integer, causing snprintf to write oob.
While this behavior is technically mitigated by num_perfcntrs being
locked at 5, it's still unsafe if num_perfcntrs were ever to change/a
second source was added.
Signed-off-by: Evan Lambert <veyga@veygax.dev>
---
v2: Use real name in Signed-off-by as requested by Dmitry Baryshkov.
drivers/gpu/drm/msm/msm_perf.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_perf.c b/drivers/gpu/drm/msm/msm_perf.c
index d3c7889aaf26..c369d4acc378 100644
--- a/drivers/gpu/drm/msm/msm_perf.c
+++ b/drivers/gpu/drm/msm/msm_perf.c
@@ -65,13 +65,13 @@ static int refill_buf(struct msm_perf_state *perf)
if ((perf->cnt++ % 32) == 0) {
/* Header line: */
- n = snprintf(ptr, rem, "%%BUSY");
+ n = scnprintf(ptr, rem, "%%BUSY");
ptr += n;
rem -= n;
for (i = 0; i < gpu->num_perfcntrs; i++) {
const struct msm_gpu_perfcntr *perfcntr = &gpu->perfcntrs[i];
- n = snprintf(ptr, rem, "\t%s", perfcntr->name);
+ n = scnprintf(ptr, rem, "\t%s", perfcntr->name);
ptr += n;
rem -= n;
}
@@ -93,21 +93,21 @@ static int refill_buf(struct msm_perf_state *perf)
return ret;
val = totaltime ? 1000 * activetime / totaltime : 0;
- n = snprintf(ptr, rem, "%3d.%d%%", val / 10, val % 10);
+ n = scnprintf(ptr, rem, "%3d.%d%%", val / 10, val % 10);
ptr += n;
rem -= n;
for (i = 0; i < ret; i++) {
/* cycle counters (I think).. convert to MHz.. */
val = cntrs[i] / 10000;
- n = snprintf(ptr, rem, "\t%5d.%02d",
+ n = scnprintf(ptr, rem, "\t%5d.%02d",
val / 100, val % 100);
ptr += n;
rem -= n;
}
}
- n = snprintf(ptr, rem, "\n");
+ n = scnprintf(ptr, rem, "\n");
ptr += n;
rem -= n;
--
2.52.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] drm/msm: Replace unsafe snprintf usage with scnprintf
2025-12-24 12:44 [PATCH v2] drm/msm: Replace unsafe snprintf usage with scnprintf veygax
@ 2025-12-24 15:37 ` Dmitry Baryshkov
2025-12-24 15:55 ` Dmitry Baryshkov
1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Baryshkov @ 2025-12-24 15:37 UTC (permalink / raw)
To: veygax
Cc: Dmitry Baryshkov, Rob Clark, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
linux-arm-msm, dri-devel, freedreno, linux-kernel
On Wed, Dec 24, 2025 at 12:44:22PM +0000, veygax wrote:
> From: Evan Lambert <veyga@veygax.dev>
>
> The refill_buf function uses snprintf to append to a fixed-size buffer.
> snprintf returns the length that would have been written, which can
> exceed the remaining buffer size. If this happens, ptr advances beyond
> the buffer and rem becomes negative. In the 2nd iteration, rem is
> treated as a large unsigned integer, causing snprintf to write oob.
>
> While this behavior is technically mitigated by num_perfcntrs being
> locked at 5, it's still unsafe if num_perfcntrs were ever to change/a
> second source was added.
>
> Signed-off-by: Evan Lambert <veyga@veygax.dev>
> ---
> v2: Use real name in Signed-off-by as requested by Dmitry Baryshkov.
Thanks!
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>
> drivers/gpu/drm/msm/msm_perf.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] drm/msm: Replace unsafe snprintf usage with scnprintf
2025-12-24 12:44 [PATCH v2] drm/msm: Replace unsafe snprintf usage with scnprintf veygax
2025-12-24 15:37 ` Dmitry Baryshkov
@ 2025-12-24 15:55 ` Dmitry Baryshkov
1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Baryshkov @ 2025-12-24 15:55 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, veygax
Cc: Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, linux-arm-msm, dri-devel, freedreno,
linux-kernel
On Wed, 24 Dec 2025 12:44:22 +0000, veygax wrote:
> The refill_buf function uses snprintf to append to a fixed-size buffer.
> snprintf returns the length that would have been written, which can
> exceed the remaining buffer size. If this happens, ptr advances beyond
> the buffer and rem becomes negative. In the 2nd iteration, rem is
> treated as a large unsigned integer, causing snprintf to write oob.
>
> While this behavior is technically mitigated by num_perfcntrs being
> locked at 5, it's still unsafe if num_perfcntrs were ever to change/a
> second source was added.
>
> [...]
Applied to msm-fixes, thanks!
[1/1] drm/msm: Replace unsafe snprintf usage with scnprintf
https://gitlab.freedesktop.org/lumag/msm/-/commit/66691e272e40
Best regards,
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-12-24 15:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-24 12:44 [PATCH v2] drm/msm: Replace unsafe snprintf usage with scnprintf veygax
2025-12-24 15:37 ` Dmitry Baryshkov
2025-12-24 15:55 ` Dmitry Baryshkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®