linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] drm/nouveau/bl: increase buffer size to avoid truncate warning
@ 2025-06-10 21:54 Jacob Keller
  2025-06-13 15:46 ` Danilo Krummrich
  0 siblings, 1 reply; 2+ messages in thread
From: Jacob Keller @ 2025-06-10 21:54 UTC (permalink / raw)
  To: Lyude Paul, Danilo Krummrich, David Airlie, Simona Vetter,
	Ben Skeggs, Pierre Moreau, Christophe JAILLET, Philip Li
  Cc: dri-devel, nouveau, linux-kernel, kernel test robot, Timur Tabi,
	Jacob Keller

The nouveau_get_backlight_name() function generates a unique name for the
backlight interface, appending an id from 1 to 99 for all backlight devices
after the first.

GCC 15 (and likely other compilers) produce the following
-Wformat-truncation warning:

nouveau_backlight.c: In function ‘nouveau_backlight_init’:
nouveau_backlight.c:56:69: error: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 3 [-Werror=format-truncation=]
   56 |                 snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb);
      |                                                                     ^~
In function ‘nouveau_get_backlight_name’,
    inlined from ‘nouveau_backlight_init’ at nouveau_backlight.c:351:7:
nouveau_backlight.c:56:56: note: directive argument in the range [1, 2147483647]
   56 |                 snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb);
      |                                                        ^~~~~~~~~~~~~~~~
nouveau_backlight.c:56:17: note: ‘snprintf’ output between 14 and 23 bytes into a destination of size 15
   56 |                 snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The warning started appearing after commit ab244be47a8f ("drm/nouveau:
Fix a potential theorical leak in nouveau_get_backlight_name()") This fix
for the ida usage removed the explicit value check for ids larger than 99.
The compiler is unable to intuit that the ida_alloc_max() limits the
returned value range between 0 and 99.

Because the compiler can no longer infer that the number ranges from 0 to
99, it thinks that it could use as many as 11 digits (10 + the potential -
sign for negative numbers).

The warning has gone unfixed for some time, with at least one kernel test
robot report. The code breaks W=1 builds, which is especially frustrating
with the introduction of CONFIG_WERROR.

The string is stored temporarily on the stack and then copied into the
device name. Its not a big deal to use 11 more bytes of stack rounding out
to an even 24 bytes. Increase BL_NAME_SIZE to 24 to avoid the truncation
warning. This fixes the W=1 builds that include this driver.

Compile tested only.

Fixes: ab244be47a8f ("drm/nouveau: Fix a potential theorical leak in nouveau_get_backlight_name()")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202312050324.0kv4PnfZ-lkp@intel.com/
Suggested-by: Timur Tabi <ttabi@nvidia.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Changes in v2:
- Just increase the buffer size
- Link to v1: https://lore.kernel.org/r/20250604-jk-nouveua-drm-bl-snprintf-fix-v1-1-79b1593ad664@intel.com
---
 drivers/gpu/drm/nouveau/nouveau_backlight.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index d47442125fa183146135f3725eae161c68e2a900..9aae26eb7d8fba54c8a989bfe7ecc2b10ccf7f61 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -42,7 +42,7 @@
 #include "nouveau_acpi.h"
 
 static struct ida bl_ida;
-#define BL_NAME_SIZE 15 // 12 for name + 2 for digits + 1 for '\0'
+#define BL_NAME_SIZE 24 // 12 for name + 11 for digits + 1 for '\0'
 
 static bool
 nouveau_get_backlight_name(char backlight_name[BL_NAME_SIZE],

---
base-commit: 90b83efa6701656e02c86e7df2cb1765ea602d07
change-id: 20250604-jk-nouveua-drm-bl-snprintf-fix-c2ca525a3d2f

Best regards,
-- 
Jacob Keller <jacob.e.keller@intel.com>


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] drm/nouveau/bl: increase buffer size to avoid truncate warning
  2025-06-10 21:54 [PATCH v2] drm/nouveau/bl: increase buffer size to avoid truncate warning Jacob Keller
@ 2025-06-13 15:46 ` Danilo Krummrich
  0 siblings, 0 replies; 2+ messages in thread
From: Danilo Krummrich @ 2025-06-13 15:46 UTC (permalink / raw)
  To: Jacob Keller
  Cc: Lyude Paul, David Airlie, Simona Vetter, Ben Skeggs,
	Pierre Moreau, Christophe JAILLET, Philip Li, dri-devel, nouveau,
	linux-kernel, kernel test robot, Timur Tabi

On 6/10/25 11:54 PM, Jacob Keller wrote:
> The nouveau_get_backlight_name() function generates a unique name for the
> backlight interface, appending an id from 1 to 99 for all backlight devices
> after the first.
> 
> GCC 15 (and likely other compilers) produce the following
> -Wformat-truncation warning:
> 
> nouveau_backlight.c: In function ‘nouveau_backlight_init’:
> nouveau_backlight.c:56:69: error: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 3 [-Werror=format-truncation=]
>     56 |                 snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb);
>        |                                                                     ^~
> In function ‘nouveau_get_backlight_name’,
>      inlined from ‘nouveau_backlight_init’ at nouveau_backlight.c:351:7:
> nouveau_backlight.c:56:56: note: directive argument in the range [1, 2147483647]
>     56 |                 snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb);
>        |                                                        ^~~~~~~~~~~~~~~~
> nouveau_backlight.c:56:17: note: ‘snprintf’ output between 14 and 23 bytes into a destination of size 15
>     56 |                 snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb);
>        |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> The warning started appearing after commit ab244be47a8f ("drm/nouveau:
> Fix a potential theorical leak in nouveau_get_backlight_name()") This fix
> for the ida usage removed the explicit value check for ids larger than 99.
> The compiler is unable to intuit that the ida_alloc_max() limits the
> returned value range between 0 and 99.
> 
> Because the compiler can no longer infer that the number ranges from 0 to
> 99, it thinks that it could use as many as 11 digits (10 + the potential -
> sign for negative numbers).
> 
> The warning has gone unfixed for some time, with at least one kernel test
> robot report. The code breaks W=1 builds, which is especially frustrating
> with the introduction of CONFIG_WERROR.
> 
> The string is stored temporarily on the stack and then copied into the
> device name. Its not a big deal to use 11 more bytes of stack rounding out
> to an even 24 bytes. Increase BL_NAME_SIZE to 24 to avoid the truncation
> warning. This fixes the W=1 builds that include this driver.
> 
> Compile tested only.
> 
> Fixes: ab244be47a8f ("drm/nouveau: Fix a potential theorical leak in nouveau_get_backlight_name()")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202312050324.0kv4PnfZ-lkp@intel.com/
> Suggested-by: Timur Tabi <ttabi@nvidia.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Applied to drm-misc-fixes, thanks!

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-06-13 15:46 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-10 21:54 [PATCH v2] drm/nouveau/bl: increase buffer size to avoid truncate warning Jacob Keller
2025-06-13 15:46 ` Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).