Re: [PATCH] drm/i195: Fix format string truncation warning
From: Tvrtko Ursulin
Date: Fri Dec 05 2025 - 06:13:18 EST
On 05/12/2025 10:48, Ard Biesheuvel wrote:
On Sun, 9 Nov 2025 at 19:00, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
On Sat, 8 Nov 2025 at 01:27, Tvrtko Ursulin <tursulin@xxxxxxxxxxx> wrote:
On 07/11/2025 16:42, Ard Biesheuvel wrote:
From: Ard Biesheuvel <ardb@xxxxxxxxxx>
GCC notices that the 16-byte uabi_name field could theoretically be too
small for the formatted string if the instance number exceeds 100.
Given that there are apparently ABI concerns here, this is the minimal
fix that shuts up the compiler without changing the output or the
maximum length for existing values < 100.
What would be those ABI concerns? I don't immediately see any.
drivers/gpu/drm/i915/intel_memory_region.c: In function ‘intel_memory_region_create’:It's a theoretical issue only since there is no hardware with a double
drivers/gpu/drm/i915/intel_memory_region.c:273:61: error: ‘%u’ directive output may be truncated writing between 1 and 5 bytes into a region of size between 3 and 11 [-Werror=format-truncation=]
273 | snprintf(mem->uabi_name, sizeof(mem->uabi_name), "%s%u",
| ^~
drivers/gpu/drm/i915/intel_memory_region.c:273:58: note: directive argument in the range [0, 65535]
273 | snprintf(mem->uabi_name, sizeof(mem->uabi_name), "%s%u",
| ^~~~~~
drivers/gpu/drm/i915/intel_memory_region.c:273:9: note: ‘snprintf’ output between 7 and 19 bytes into a destination of size 16
273 | snprintf(mem->uabi_name, sizeof(mem->uabi_name), "%s%u",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
274 | intel_memory_type_str(type), instance);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
---
Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Cc: Tvrtko Ursulin <tursulin@xxxxxxxxxxx>
Cc: David Airlie <airlied@xxxxxxxxx>
Cc: Simona Vetter <simona@xxxxxxxx>
Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
This is unlikely to be the right fix, but sending a wrong patch is
usually a better way to elicit a response than just sending a bug
report.
drivers/gpu/drm/i915/intel_memory_region.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c
index 59bd603e6deb..ad4afcf0c58a 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -271,7 +271,7 @@ intel_memory_region_create(struct drm_i915_private *i915,
mem->instance = instance;
snprintf(mem->uabi_name, sizeof(mem->uabi_name), "%s%u",
- intel_memory_type_str(type), instance);
+ intel_memory_type_str(type), instance % 100);
digit number of instances.
But I guess much prettier fix would be to simply grow the buffer.
OK, so something like
--- a/drivers/gpu/drm/i915/intel_memory_region.h
+++ b/drivers/gpu/drm/i915/intel_memory_region.h
@@ -72,7 +72,7 @@ struct intel_memory_region {
u16 instance;
enum intel_region_id id;
char name[16];
- char uabi_name[16];
+ char uabi_name[20];
bool private; /* not for userspace */
struct {
Yes please. There is only two of those objects at majority of systems, and 3-4 on a few discrete cards supported by i915, so no big deal to grow them a tiny bit.
Also, hm, how come gcc does not find the mem->name vsnprintf from
intel_memory_region_set_name?
AFAICT, intel_memory_region_set_name() is never called with a format
string that could produce more than 15/16 bytes of output.
Right, I reminded myself that the non-uabi visible name does not have the instance number in it.
Regards,
Tvrtko