lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-Id: <20250610-jk-nouveua-drm-bl-snprintf-fix-v2-1-7fdd4b84b48e@intel.com> Date: Tue, 10 Jun 2025 14:54:51 -0700 From: Jacob Keller <jacob.e.keller@...el.com> To: Lyude Paul <lyude@...hat.com>, Danilo Krummrich <dakr@...nel.org>, David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Ben Skeggs <bskeggs@...hat.com>, Pierre Moreau <pierre.morrow@...e.fr>, Christophe JAILLET <christophe.jaillet@...adoo.fr>, Philip Li <philip.li@...el.com> Cc: dri-devel@...ts.freedesktop.org, nouveau@...ts.freedesktop.org, linux-kernel@...r.kernel.org, kernel test robot <lkp@...el.com>, Timur Tabi <ttabi@...dia.com>, Jacob Keller <jacob.e.keller@...el.com> Subject: [PATCH v2] drm/nouveau/bl: increase buffer size to avoid truncate warning 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@...el.com> Closes: https://lore.kernel.org/oe-kbuild-all/202312050324.0kv4PnfZ-lkp@intel.com/ Suggested-by: Timur Tabi <ttabi@...dia.com> Signed-off-by: Jacob Keller <jacob.e.keller@...el.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@...el.com>
Powered by blists - more mailing lists