drivers/gpu/drm/i915/intel_memory_region.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
From: Ard Biesheuvel <ardb@kernel.org>
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.
drivers/gpu/drm/i915/intel_memory_region.c: In function ‘intel_memory_region_create’:
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@kernel.org>
---
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tursulin@ursulin.net>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
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);
mutex_init(&mem->objects.lock);
INIT_LIST_HEAD(&mem->objects.list);
--
2.51.2.1041.gc1ab5b90ca-googOn 07/11/2025 16:42, Ard Biesheuvel wrote: > From: Ard Biesheuvel <ardb@kernel.org> > > 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’: > 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@kernel.org> > --- > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Tvrtko Ursulin <tursulin@ursulin.net> > Cc: David Airlie <airlied@gmail.com> > Cc: Simona Vetter <simona@ffwll.ch> > Cc: intel-gfx@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > > 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); It's a theoretical issue only since there is no hardware with a double digit number of instances. But I guess much prettier fix would be to simply grow the buffer. Also, hm, how come gcc does not find the mem->name vsnprintf from intel_memory_region_set_name? Regards, Tvrtko > > mutex_init(&mem->objects.lock); > INIT_LIST_HEAD(&mem->objects.list);
On Sat, 8 Nov 2025 at 01:27, Tvrtko Ursulin <tursulin@ursulin.net> wrote: > > > On 07/11/2025 16:42, Ard Biesheuvel wrote: > > From: Ard Biesheuvel <ardb@kernel.org> > > > > 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’: > > 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@kernel.org> > > --- > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Cc: Tvrtko Ursulin <tursulin@ursulin.net> > > Cc: David Airlie <airlied@gmail.com> > > Cc: Simona Vetter <simona@ffwll.ch> > > Cc: intel-gfx@lists.freedesktop.org > > Cc: dri-devel@lists.freedesktop.org > > > > 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); > It's a theoretical issue only since there is no hardware with a double > digit number of instances. > > But I guess much prettier fix would be to simply grow the buffer. > Whatever works for you - I don't really understand this code anyway. > Also, hm, how come gcc does not find the mem->name vsnprintf from > intel_memory_region_set_name? > The optimizer works in mysterious ways, I guess. I cannot explain why I am the only one seeing this in the first place, but the warning seems legit to me.
On Sun, 9 Nov 2025 at 19:00, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Sat, 8 Nov 2025 at 01:27, Tvrtko Ursulin <tursulin@ursulin.net> wrote:
> >
> >
> > On 07/11/2025 16:42, Ard Biesheuvel wrote:
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > >
> > > 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’:
> > > 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@kernel.org>
> > > ---
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Tvrtko Ursulin <tursulin@ursulin.net>
> > > Cc: David Airlie <airlied@gmail.com>
> > > Cc: Simona Vetter <simona@ffwll.ch>
> > > Cc: intel-gfx@lists.freedesktop.org
> > > Cc: dri-devel@lists.freedesktop.org
> > >
> > > 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);
> > It's a theoretical issue only since there is no hardware with a double
> > 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 {
> > 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.
On Fri, 5 Dec 2025 11:48:08 +0100
Ard Biesheuvel <ardb@kernel.org> wrote:
> On Sun, 9 Nov 2025 at 19:00, Ard Biesheuvel <ardb@kernel.org> wrote:
...
> > > 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];
The observant will notice the 7 bytes of padding following 'private',
and another 7 a little later on.
(I' pretty sure 'bool' is u8).
So extending the buffer doesn't even grow the structure.
The string is only used when printing some stats.
I got lost in a list of #defines and function pointers trying to find
the actual function that did the 'printf'.
David
> bool private; /* not for userspace */
>
> struct {
>
>
>
> > > 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.
>
On 05/12/2025 19:28, David Laight wrote:
> On Fri, 5 Dec 2025 11:48:08 +0100
> Ard Biesheuvel <ardb@kernel.org> wrote:
>
>> On Sun, 9 Nov 2025 at 19:00, Ard Biesheuvel <ardb@kernel.org> wrote:
> ...
>>>> 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];
> The observant will notice the 7 bytes of padding following 'private',
> and another 7 a little later on.
> (I' pretty sure 'bool' is u8).
Oh well, them holes love to be added over time.
Anyway, I have pushed this patch to drm-intel-gt-next so it will appear
in 6.20. Only now I realised I could have suggested to add some Fixes:
tag to it, so it would get automatically picked for 6.19.
My colleagues who are handling drm-intel-next-fixes for 6.19 could
perhaps manually pick it up.
Tvrtko
>
> So extending the buffer doesn't even grow the structure.
> The string is only used when printing some stats.
> I got lost in a list of #defines and function pointers trying to find
> the actual function that did the 'printf'.
>
> David
>
>> bool private; /* not for userspace */
>>
>> struct {
>>
>>
>>
>>>> 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.
>>
On Mon, 08 Dec 2025, Tvrtko Ursulin <tursulin@ursulin.net> wrote:
> Anyway, I have pushed this patch to drm-intel-gt-next so it will appear
> in 6.20. Only now I realised I could have suggested to add some Fixes:
> tag to it, so it would get automatically picked for 6.19.
>
> My colleagues who are handling drm-intel-next-fixes for 6.19 could
> perhaps manually pick it up.
Picked up with
Fixes: 3b38d3515753 ("drm/i915: Add stable memory region names")
Cc: <stable@vger.kernel.org> # v6.8+
added.
BR,
Jani.
--
Jani Nikula, Intel
On 05/12/2025 10:48, Ard Biesheuvel wrote:
> On Sun, 9 Nov 2025 at 19:00, Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Sat, 8 Nov 2025 at 01:27, Tvrtko Ursulin <tursulin@ursulin.net> wrote:
>>>
>>>
>>> On 07/11/2025 16:42, Ard Biesheuvel wrote:
>>>> From: Ard Biesheuvel <ardb@kernel.org>
>>>>
>>>> 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’:
>>>> 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@kernel.org>
>>>> ---
>>>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>> Cc: Tvrtko Ursulin <tursulin@ursulin.net>
>>>> Cc: David Airlie <airlied@gmail.com>
>>>> Cc: Simona Vetter <simona@ffwll.ch>
>>>> Cc: intel-gfx@lists.freedesktop.org
>>>> Cc: dri-devel@lists.freedesktop.org
>>>>
>>>> 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);
>>> It's a theoretical issue only since there is no hardware with a double
>>> 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
On Fri, 05 Dec 2025, Tvrtko Ursulin <tursulin@ursulin.net> wrote:
> On 05/12/2025 10:48, Ard Biesheuvel wrote:
>> 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.
For v2, please also fix the subject prefix: drm/i915.
BR,
Jani.
--
Jani Nikula, Intel
© 2016 - 2025 Red Hat, Inc.