From: Seyediman Seyedarab imandevel@gmail.com
[ Upstream commit 6510b62fe9303aaf48ff136ff69186bcfc32172d ]
snprintf() returns the number of characters that *would* have been written, which can overestimate how much you actually wrote to the buffer in case of truncation. That leads to 'data += this' advancing the pointer past the end of the buffer and size going negative.
Switching to scnprintf() prevents potential buffer overflows and ensures consistent behavior when building the output string.
Signed-off-by: Seyediman Seyedarab ImanDevel@gmail.com Link: https://lore.kernel.org/r/20250724195913.60742-1-ImanDevel@gmail.com Signed-off-by: Danilo Krummrich dakr@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
Summary - Replaces snprintf with scnprintf in nvkm_snprintbf to prevent pointer over-advancement and out-of-bounds NUL writes on truncation. - Small, contained, behavior-preserving fix across widely used debug/interrupt logging paths in Nouveau.
Technical Analysis - Problematic code path: - In nvkm_snprintbf, the loop builds a space-separated list of bit names: drivers/gpu/drm/nouveau/nvkm/core/enum.c:45–55. - It uses snprintf to append: drivers/gpu/drm/nouveau/nvkm/core/enum.c:47–50. - After each append, it advances the pointer by the return value and reduces size: drivers/gpu/drm/nouveau/nvkm/core/enum.c:49–50. - On exit, it writes a trailing NUL at the current pointer: drivers/gpu/drm/nouveau/nvkm/core/enum.c:55. - Bug mechanism: - snprintf returns the number of characters that would have been written (excluding NUL) even when truncated. - If the buffer is near full (e.g., size == 1), snprintf returns a value > 0, causing size to go negative and data to advance past the end of the buffer, so the final data[0] = '\0' writes out-of-bounds (drivers/gpu/drm/nouveau/nvkm/core/enum.c:55). - Fix rationale: - scnprintf returns the number of characters actually written into the buffer, bounded by size-1 and always consistent with the pointer/data movement. - Replacing snprintf with scnprintf at drivers/gpu/drm/nouveau/nvkm/core/enum.c:47 guarantees that size tracking and pointer advancement remain in-bounds and that the final NUL write is safe. - API availability: - scnprintf is a long-standing kernel helper declared in include/linux/sprintf.h:15, so it exists across stable series.
Impact and Usage Context - nvkm_snprintbf is used widely to format error/interrupt bitfields into human-readable strings (numerous call sites): - Example: drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c:1239 (char error[128];) and subsequent use of nvkm_snprintbf(error, sizeof(error), ...) to log errors. - Other examples include: drivers/gpu/drm/nouveau/nvkm/engine/gr/nv50.c:272, drivers/gpu/drm/nouveau/nvkm/engine/gr/nv40.c:271–273, drivers/gpu/drm/nouveau/nvkm/subdev/ltc/gf100.c:103, drivers/gpu/drm/nouveau/nvkm/subdev/ltc/gm107.c:82. - While buffers are typically 64–128 bytes, concatenating multiple bit names can still reach truncation boundaries. The current code can then over-advance and perform an OOB NUL write. This is memory corruption (albeit a single byte) on the kernel stack and should be fixed.
Risk/Regression Assessment - Change scope is a one-line replacement local to the function; no API/ABI change. - scnprintf semantics match the intended logic of “advance by what we wrote,” preventing negative size and pointer overflow. - Behavior under truncation improves: instead of corrupting memory, the function simply yields a properly NUL-terminated string with as much content as fits. - No architectural changes; only string formatting mechanics inside a helper used for logging and diagnostics.
Stable Backport Criteria - Fixes a real bug that can corrupt memory (stack OOB write) in common code paths. - Minimal, self-contained, and low-risk change. - Not a feature addition; purely correctness/safety. - Touches only the Nouveau driver helper; broad benefit across many call sites without side effects. - No special dependencies; scnprintf is present across stable kernels.
Backport Notes - Function prototype remains unchanged: drivers/gpu/drm/nouveau/include/nvkm/core/enum.h:21. - Ensure include paths bring in scnprintf (declared in include/linux/sprintf.h:15). Nouveau’s headers already include standard Linux headers via drivers/gpu/drm/nouveau/include/nvif/os.h which pulls core kernel headers, so no additional includes are needed.
Conclusion - This is a classic correctness/safety fix that prevents a subtle but real OOB write. It is small, contained, and aligns with stable policy. Backporting is recommended.
drivers/gpu/drm/nouveau/nvkm/core/enum.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/core/enum.c b/drivers/gpu/drm/nouveau/nvkm/core/enum.c index b9581feb24ccb..a23b40b27b81b 100644 --- a/drivers/gpu/drm/nouveau/nvkm/core/enum.c +++ b/drivers/gpu/drm/nouveau/nvkm/core/enum.c @@ -44,7 +44,7 @@ nvkm_snprintbf(char *data, int size, const struct nvkm_bitfield *bf, u32 value) bool space = false; while (size >= 1 && bf->name) { if (value & bf->mask) { - int this = snprintf(data, size, "%s%s", + int this = scnprintf(data, size, "%s%s", space ? " " : "", bf->name); size -= this; data += this;