mod_devicetable: Enlarge the maximum platform_device_id name length
Subsystems (2)
PATCH 1/1 mod_devicetable: Enlarge the maximum platform_device_id name length
The 20 byte length of struct platform_device_id::name is not long enough for many devices (especially regulators), where the string initialization is getting truncated and missing the trailing NUL byte. This is seen with GCC 15's -Wunterminated-string-initialization option:
drivers/regulator/hi6421v530-regulator.c:189:19: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization]
189 | { .name = "hi6421v530-regulator" },
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/regulator/hi6421v600-regulator.c:278:19: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization]
278 | { .name = "hi6421v600-regulator" },
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/regulator/lp87565-regulator.c:233:11: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization]
233 | { "lp87565-q1-regulator", },
| ^~~~~~~~~~~~~~~~~~~~~~
sound/soc/fsl/imx-pcm-rpmsg.c:818:19: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization]
818 | { .name = "rpmsg-micfil-channel" },
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/iio/light/hid-sensor-als.c:457:25: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization]
457 | .name = "HID-SENSOR-LISS-0041",
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/iio/light/hid-sensor-prox.c:366:25: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization]
366 | .name = "HID-SENSOR-LISS-0226",
| ^~~~~~~~~~~~~~~~~~~~~~Increase the length to 24, slightly more than is currently being used by the affected drivers. The string is used in '%s' format strings and via the module code, which appears to do its own length encoding. This size was chosen because there was already a 4 byte hole in the structure:
struct platform_device_id {
char name[20]; /* 0 20 */
/* XXX 4 bytes hole, try to pack */
kernel_ulong_t driver_data; /* 24 8 */
/* size: 32, cachelines: 1, members: 2 */
/* sum members: 28, holes: 1, sum holes: 4 */
/* last cacheline: 32 bytes */
};---
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Nipun Gupta <[email protected]>
Cc: Nikhil Agarwal <[email protected]>
Cc: "Borislav Petkov (AMD)" <[email protected]>
Cc: Pieter Jansen van Vuuren <[email protected]>
Cc: Andy Shevchenko <[email protected]>
--- include/linux/mod_devicetable.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/mod_devicetable.h +1 -1
@@ -601,7 +601,7 @@ struct dmi_system_id {#define DMI_MATCH (a, b) { .slot = a, .substr = b }#define DMI_EXACT_MATCH (a, b) { .slot = a, .substr = b, .exact_match = 1 } -#define PLATFORM_NAME_SIZE 20 +#define PLATFORM_NAME_SIZE 24#define PLATFORM_MODULE_PREFIX "platform:"struct platform_device_id {
Signature · 1 line
2.34.1
Quoted 29 lines · Kees Cook
The 20 byte length of struct
platform_device_id::nameis not long enough for many devices (especially regulators), where the string initialization is getting truncated and missing the trailing NUL byte. This is seen with GCC 15's -Wunterminated-string-initialization option:
drivers/regulator/hi6421v530-regulator.c:189:19: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization]
189 | { .name = "hi6421v530-regulator" },| ^~~~~~~~~~~~~~~~~~~~~~ drivers/regulator/hi6421v600-regulator.c:278:19: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 278 | { .name = "hi6421v600-regulator" }, | ^~~~~~~~~~~~~~~~~~~~~~ drivers/regulator/lp87565-regulator.c:233:11: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 233 | { "lp87565-q1-regulator", }, | ^~~~~~~~~~~~~~~~~~~~~~ sound/soc/fsl/imx-pcm-rpmsg.c:818:19: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 818 | { .name = "rpmsg-micfil-channel" }, | ^~~~~~~~~~~~~~~~~~~~~~ drivers/iio/light/hid-sensor-als.c:457:25: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 457 | .name = "HID-SENSOR-LISS-0041", | ^~~~~~~~~~~~~~~~~~~~~~ drivers/iio/light/hid-sensor-prox.c:366:25: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 366 | .name = "HID-SENSOR-LISS-0226", | ^~~~~~~~~~~~~~~~~~~~~~Increase the length to 24, slightly more than is currently being used by the affected drivers. The string is used in '%s' format strings and via the module code, which appears to do its own length encoding. This size was chosen because there was already a 4 byte hole in the structure:
Quoted 11 lines
struct platform_device_id {
char name[20]; /* 0 20 */
/* XXX 4 bytes hole, try to pack */
kernel_ulong_t driver_data; /* 24 8 */
/* size: 32, cachelines: 1, members: 2 */
/* sum members: 28, holes: 1, sum holes: 4 */
/* last cacheline: 32 bytes */
};Since there is no even potential ABI breakage, I'm fine with the change.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Andy, Kees,
Quoted 37 lines · Andy Shevchenko
Quoted 36 lines · Kees Cook
The 20 byte length of struct
platform_device_id::nameis not long enough for many devices (especially regulators), where the string initialization is getting truncated and missing the trailing NUL byte. This is seen with GCC 15's -Wunterminated-string-initialization option:
drivers/regulator/hi6421v530-regulator.c:189:19: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization]
189 | { .name = "hi6421v530-regulator" },| ^~~~~~~~~~~~~~~~~~~~~~ drivers/regulator/hi6421v600-regulator.c:278:19: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 278 | { .name = "hi6421v600-regulator" }, | ^~~~~~~~~~~~~~~~~~~~~~ drivers/regulator/lp87565-regulator.c:233:11: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 233 | { "lp87565-q1-regulator", }, | ^~~~~~~~~~~~~~~~~~~~~~ sound/soc/fsl/imx-pcm-rpmsg.c:818:19: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 818 | { .name = "rpmsg-micfil-channel" }, | ^~~~~~~~~~~~~~~~~~~~~~ drivers/iio/light/hid-sensor-als.c:457:25: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 457 | .name = "HID-SENSOR-LISS-0041", | ^~~~~~~~~~~~~~~~~~~~~~ drivers/iio/light/hid-sensor-prox.c:366:25: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 366 | .name = "HID-SENSOR-LISS-0226", | ^~~~~~~~~~~~~~~~~~~~~~Increase the length to 24, slightly more than is currently being used by the affected drivers. The string is used in '%s' format strings and via the module code, which appears to do its own length encoding. This size was chosen because there was already a 4 byte hole in the structure:
struct
platform_device_id{
char name[20]; /* 0 20 *//* XXX 4 bytes hole, try to pack */
kernel_ulong_tdriver_data; /* 24 8 */
Quoted 7 lines
Quoted 4 lines
/* size: 32, cachelines: 1, members: 2 */ /* sum members: 28, holes: 1, sum holes: 4 */ /* last cacheline: 32 bytes */ };Since there is no even potential ABI breakage, I'm fine with the change.
Reviewed-by Andy Shevchenko <[email protected]>
This definitely breaks ABI on 32-bit architectures such as i586, because there is no gap from alignment. Perhaps, this also make the commit not suitable for backporting to stable branches?
I recently stumbled on build failure on v5.10.239 for i586:
make: Entering directory '/usr/src/kernel-source-5.10'
DEPMOD 5.10.239
depmod: FATAL: Module index: bad character '�'=0x80 - only 7-bit ASCII is supported:
platform:jsl_rt5682_max98360ax�
make: *** [Makefile:1786: modules_install] Error 1
make: Leaving directory '/usr/src/kernel-source-5.10'
With this patch not applied "jsl_rt5682_max98360a" have terminating '\0' truncated due to PLATFORM_NAME_SIZE being same as the string length and concatenated with the following binary data:
{
.name = "jsl_rt5682_max98360a",
.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |
SOF_RT5682_MCLK_24MHZ |
SOF_RT5682_SSP_CODEC(0) |
SOF_SPEAKER_AMP_PRESENT |
SOF_MAX98360A_SPEAKER_AMP_PRESENT |
SOF_RT5682_SSP_AMP(1)),
},modpost then interprets it as an asciiz string concatenating with driver_data resulting in bad characters.
static int do_platform_entry(const char *filename,
void *symval, char *alias)
{
DEF_FIELD_ADDR(symval, platform_device_id, name);
sprintf(alias, PLATFORM_MODULE_PREFIX "%s", *name);
return 1;}
creating in an incorrect alias, and this somehow breaks depmod in kmod 34.2 (maybe earlier).
Old kmod 30 successfully adds incorrect alias:
$ modinfo snd-soc-sof_rt5682.ko | grep jsl_rt5682_max98360a alias: platform:jsl_rt5682_max98360a alias: platform:jsl_rt5682_max98360ax�
and
modules.alias:alias platform:jsl_rt5682_max98360ax� snd_soc_sof_rt5682
Perhaps, scripts/mod/file2alias.c should be updated with:
- sprintf(alias, PLATFORM_MODULE_PREFIX "%s", *name);
+ sprintf(alias, PLATFORM_MODULE_PREFIX "%.*s", PLATFORM_NAME_SIZE, *name);
(Or even producing an error if more serious truncation occurs.)
Thanks,
Quoted 6 lines
Signature · 3 lines
-- With Best Regards, Andy Shevchenko
Kees Cook
AppliedQuoted 24 lines · Kees Cook
The 20 byte length of struct
platform_device_id::nameis not long enough for many devices (especially regulators), where the string initialization is getting truncated and missing the trailing NUL byte. This is seen with GCC 15's -Wunterminated-string-initialization option:
drivers/regulator/hi6421v530-regulator.c:189:19: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization]
189 | { .name = "hi6421v530-regulator" },| ^~~~~~~~~~~~~~~~~~~~~~ drivers/regulator/hi6421v600-regulator.c:278:19: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 278 | { .name = "hi6421v600-regulator" }, | ^~~~~~~~~~~~~~~~~~~~~~ drivers/regulator/lp87565-regulator.c:233:11: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 233 | { "lp87565-q1-regulator", }, | ^~~~~~~~~~~~~~~~~~~~~~ sound/soc/fsl/imx-pcm-rpmsg.c:818:19: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 818 | { .name = "rpmsg-micfil-channel" }, | ^~~~~~~~~~~~~~~~~~~~~~ drivers/iio/light/hid-sensor-als.c:457:25: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 457 | .name = "HID-SENSOR-LISS-0041", | ^~~~~~~~~~~~~~~~~~~~~~ drivers/iio/light/hid-sensor-prox.c:366:25: warning: initializer-string for array of 'char' truncates NUL terminator but destination lacks 'nonstring' attribute (21 chars into 20 available) [-Wunterminated-string-initialization] 366 | .name = "HID-SENSOR-LISS-0226", | ^~~~~~~~~~~~~~~~~~~~~~
Quoted 1 line
[...]
Applied to for-next/hardening, thanks!
[1/1] mod_devicetable: Enlarge the maximum platform_device_id name length
https://git.kernel.org/kees/c/94a821d9355c
Take care,
Signature · 1 line
Kees Cook
Sasha Levin
maintainer[ Sasha's backport helper bot ]
Hi,
Summary of potential issues:
⚠️ Found matching upstream commit but patch is missing proper reference to it
Found matching upstream commit: 655862865c97ff55e4f3f2aaa7708f42f0ea3bd8
Note: The patch differs from the upstream commit:
---
1: 655862865c97f ! 1: b545ae48a3db0 mod_devicetable: Enlarge the maximum platform_device_id name length
@@ Commit message
/* last cacheline: 32 bytes */
};
- Reviewed-by: Andy Shevchenko <[email protected]>
- Link: https://lore.kernel.org/r/[email protected]
## include/linux/mod_devicetable.h ##
---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.15.y | Success | Success | | stable/linux-6.12.y | Success | Success | | stable/linux-6.6.y | Success | Success | | stable/linux-6.1.y | Success | Success | | stable/linux-5.15.y | Success | Success | | stable/linux-5.10.y | Success | Success | | stable/linux-5.4.y | Success | Success |