From 23cfbc6ec44e5e80d5522976ff45ffcdcddfb230 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 21 Apr 2022 17:29:04 +0200 Subject: firmware: Add the support for ZSTD-compressed firmware files As the growing demand on ZSTD compressions, there have been requests for the support of ZSTD-compressed firmware files, so here it is: this patch extends the firmware loader code to allow loading ZSTD files. The implementation is fairly straightforward, it just adds a ZSTD decompression routine for the file expander. (And the code is even simpler than XZ thanks to the ZSTD API that gives the original decompressed size from the header.) Link: https://lore.kernel.org/all/20210127154939.13288-1-tiwai@suse.de/ Tested-by: Piotr Gorski Signed-off-by: Takashi Iwai Link: https://lore.kernel.org/r/20220421152908.4718-2-tiwai@suse.de Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/Kconfig | 24 +++++++++--- drivers/base/firmware_loader/main.c | 76 ++++++++++++++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 9 deletions(-) diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig index 38f3b66bf52b..08bb50451a96 100644 --- a/drivers/base/firmware_loader/Kconfig +++ b/drivers/base/firmware_loader/Kconfig @@ -159,21 +159,33 @@ config FW_LOADER_USER_HELPER_FALLBACK config FW_LOADER_COMPRESS bool "Enable compressed firmware support" - select FW_LOADER_PAGED_BUF - select XZ_DEC help This option enables the support for loading compressed firmware files. The caller of firmware API receives the decompressed file content. The compressed file is loaded as a fallback, only after loading the raw file failed at first. - Currently only XZ-compressed files are supported, and they have to - be compressed with either none or crc32 integrity check type (pass - "-C crc32" option to xz command). - Compressed firmware support does not apply to firmware images that are built into the kernel image (CONFIG_EXTRA_FIRMWARE). +if FW_LOADER_COMPRESS +config FW_LOADER_COMPRESS_XZ + bool "Enable XZ-compressed firmware support" + select FW_LOADER_PAGED_BUF + select XZ_DEC + help + This option adds the support for XZ-compressed files. + The files have to be compressed with either none or crc32 + integrity check type (pass "-C crc32" option to xz command). + +config FW_LOADER_COMPRESS_ZSTD + bool "Enable ZSTD-compressed firmware support" + select ZSTD_DECOMPRESS + help + This option adds the support for ZSTD-compressed files. + +endif # FW_LOADER_COMPRESS + config FW_CACHE bool "Enable firmware caching during suspend" depends on PM_SLEEP diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 94d1789a233e..74830aeec7f6 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -304,10 +305,74 @@ int fw_map_paged_buf(struct fw_priv *fw_priv) } #endif +/* + * ZSTD-compressed firmware support + */ +#ifdef CONFIG_FW_LOADER_COMPRESS_ZSTD +static int fw_decompress_zstd(struct device *dev, struct fw_priv *fw_priv, + size_t in_size, const void *in_buffer) +{ + size_t len, out_size, workspace_size; + void *workspace, *out_buf; + zstd_dctx *ctx; + int err; + + if (fw_priv->allocated_size) { + out_size = fw_priv->allocated_size; + out_buf = fw_priv->data; + } else { + zstd_frame_header params; + + if (zstd_get_frame_header(¶ms, in_buffer, in_size) || + params.frameContentSize == ZSTD_CONTENTSIZE_UNKNOWN) { + dev_dbg(dev, "%s: invalid zstd header\n", __func__); + return -EINVAL; + } + out_size = params.frameContentSize; + out_buf = vzalloc(out_size); + if (!out_buf) + return -ENOMEM; + } + + workspace_size = zstd_dctx_workspace_bound(); + workspace = kvzalloc(workspace_size, GFP_KERNEL); + if (!workspace) { + err = -ENOMEM; + goto error; + } + + ctx = zstd_init_dctx(workspace, workspace_size); + if (!ctx) { + dev_dbg(dev, "%s: failed to initialize context\n", __func__); + err = -EINVAL; + goto error; + } + + len = zstd_decompress_dctx(ctx, out_buf, out_size, in_buffer, in_size); + if (zstd_is_error(len)) { + dev_dbg(dev, "%s: failed to decompress: %d\n", __func__, + zstd_get_error_code(len)); + err = -EINVAL; + goto error; + } + + if (!fw_priv->allocated_size) + fw_priv->data = out_buf; + fw_priv->size = len; + err = 0; + + error: + kvfree(workspace); + if (err && !fw_priv->allocated_size) + vfree(out_buf); + return err; +} +#endif /* CONFIG_FW_LOADER_COMPRESS_ZSTD */ + /* * XZ-compressed firmware support */ -#ifdef CONFIG_FW_LOADER_COMPRESS +#ifdef CONFIG_FW_LOADER_COMPRESS_XZ /* show an error and return the standard error code */ static int fw_decompress_xz_error(struct device *dev, enum xz_ret xz_ret) { @@ -401,7 +466,7 @@ static int fw_decompress_xz(struct device *dev, struct fw_priv *fw_priv, else return fw_decompress_xz_pages(dev, fw_priv, in_size, in_buffer); } -#endif /* CONFIG_FW_LOADER_COMPRESS */ +#endif /* CONFIG_FW_LOADER_COMPRESS_XZ */ /* direct firmware loading support */ static char fw_path_para[256]; @@ -757,7 +822,12 @@ _request_firmware(const struct firmware **firmware_p, const char *name, if (!(opt_flags & FW_OPT_PARTIAL)) nondirect = true; -#ifdef CONFIG_FW_LOADER_COMPRESS +#ifdef CONFIG_FW_LOADER_COMPRESS_ZSTD + if (ret == -ENOENT && nondirect) + ret = fw_get_filesystem_firmware(device, fw->priv, ".zst", + fw_decompress_zstd); +#endif +#ifdef CONFIG_FW_LOADER_COMPRESS_XZ if (ret == -ENOENT && nondirect) ret = fw_get_filesystem_firmware(device, fw->priv, ".xz", fw_decompress_xz); -- cgit v1.2.3 From b3625b1324a56ff1194734c9b84a51b05e14a419 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 21 Apr 2022 17:29:05 +0200 Subject: selftests: firmware: Use smaller dictionary for XZ compression The xz -9 option leads to an unnecessarily too large dictionary that isn't really suitable for the kernel firmware loader. Pass the dictionary size explicitly, instead. While we're at it, make the xz command call defined in $RUN_XZ for simplicity. Fixes: 108ae07c5036 ("selftests: firmware: Add compressed firmware tests") Signed-off-by: Takashi Iwai Link: https://lore.kernel.org/r/20220421152908.4718-3-tiwai@suse.de Signed-off-by: Greg Kroah-Hartman --- tools/testing/selftests/firmware/fw_filesystem.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh index c2a2a100114b..731f011def78 100755 --- a/tools/testing/selftests/firmware/fw_filesystem.sh +++ b/tools/testing/selftests/firmware/fw_filesystem.sh @@ -11,6 +11,8 @@ TEST_REQS_FW_SET_CUSTOM_PATH="yes" TEST_DIR=$(dirname $0) source $TEST_DIR/fw_lib.sh +RUN_XZ="xz -C crc32 --lzma2=dict=2MiB" + check_mods check_setup verify_reqs @@ -410,9 +412,9 @@ test_request_firmware_nowait_custom() RANDOM_FILE_PATH=$(setup_random_file) RANDOM_FILE="$(basename $RANDOM_FILE_PATH)" if [ "$2" = "both" ]; then - xz -9 -C crc32 -k $RANDOM_FILE_PATH + $RUN_XZ -k $RANDOM_FILE_PATH elif [ "$2" = "xzonly" ]; then - xz -9 -C crc32 $RANDOM_FILE_PATH + $RUN_XZ $RANDOM_FILE_PATH fi config_set_name $RANDOM_FILE config_trigger_async @@ -501,7 +503,7 @@ test_request_partial_firmware_into_buf_nofile 2 10 test "$HAS_FW_LOADER_COMPRESS" != "yes" && exit 0 # test with both files present -xz -9 -C crc32 -k $FW +$RUN_XZ -k $FW config_set_name $NAME echo echo "Testing with both plain and xz files present..." -- cgit v1.2.3 From 04c826d0726720bc69617a291fad6e482ecf83d6 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 21 Apr 2022 17:29:06 +0200 Subject: selftests: firmware: Fix the request_firmware_into_buf() test for XZ format The test uses a different firmware name, and we forgot to adapt for the XZ compressed file tests. https://lore.kernel.org/all/20210127154939.13288-1-tiwai@suse.de/ Fixes: 1798045900b7 ("selftests: firmware: Add request_firmware_into_buf tests") Signed-off-by: Takashi Iwai Link: https://lore.kernel.org/r/20220421152908.4718-4-tiwai@suse.de Signed-off-by: Greg Kroah-Hartman --- tools/testing/selftests/firmware/fw_filesystem.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh index 731f011def78..3ac09b401a83 100755 --- a/tools/testing/selftests/firmware/fw_filesystem.sh +++ b/tools/testing/selftests/firmware/fw_filesystem.sh @@ -504,6 +504,7 @@ test "$HAS_FW_LOADER_COMPRESS" != "yes" && exit 0 # test with both files present $RUN_XZ -k $FW +$RUN_XZ -k $FW_INTO_BUF config_set_name $NAME echo echo "Testing with both plain and xz files present..." @@ -529,6 +530,7 @@ done # test with only xz file present mv "$FW" "${FW}-orig" +mv "$FW_INTO_BUF" "${FW_INTO_BUF}-orig" echo echo "Testing with only xz file present..." for i in $(seq 1 5); do -- cgit v1.2.3 From f18b45ff9ac7ff05beeebd62d7c25d58f38b1410 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 21 Apr 2022 17:29:07 +0200 Subject: selftests: firmware: Simplify test patterns The test patterns are almost same in three sequential tests. Make the unified helper function for improving the readability. Link: https://lore.kernel.org/all/20210127154939.13288-1-tiwai@suse.de/ Signed-off-by: Takashi Iwai Link: https://lore.kernel.org/r/20220421152908.4718-5-tiwai@suse.de Signed-off-by: Greg Kroah-Hartman --- tools/testing/selftests/firmware/fw_filesystem.sh | 106 ++++++---------------- 1 file changed, 30 insertions(+), 76 deletions(-) diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh index 3ac09b401a83..4a574be8b862 100755 --- a/tools/testing/selftests/firmware/fw_filesystem.sh +++ b/tools/testing/selftests/firmware/fw_filesystem.sh @@ -437,6 +437,32 @@ test_request_partial_firmware_into_buf() echo "OK" } +do_tests () +{ + mode="$1" + suffix="$2" + + for i in $(seq 1 5); do + test_batched_request_firmware$suffix $i $mode + done + + for i in $(seq 1 5); do + test_batched_request_firmware_into_buf$suffix $i $mode + done + + for i in $(seq 1 5); do + test_batched_request_firmware_direct$suffix $i $mode + done + + for i in $(seq 1 5); do + test_request_firmware_nowait_uevent$suffix $i $mode + done + + for i in $(seq 1 5); do + test_request_firmware_nowait_custom$suffix $i $mode + done +} + # Only continue if batched request triggers are present on the # test-firmware driver test_config_present @@ -444,25 +470,7 @@ test_config_present # test with the file present echo echo "Testing with the file present..." -for i in $(seq 1 5); do - test_batched_request_firmware $i normal -done - -for i in $(seq 1 5); do - test_batched_request_firmware_into_buf $i normal -done - -for i in $(seq 1 5); do - test_batched_request_firmware_direct $i normal -done - -for i in $(seq 1 5); do - test_request_firmware_nowait_uevent $i normal -done - -for i in $(seq 1 5); do - test_request_firmware_nowait_custom $i normal -done +do_tests normal # Partial loads cannot use fallback, so do not repeat tests. test_request_partial_firmware_into_buf 0 10 @@ -474,25 +482,7 @@ test_request_partial_firmware_into_buf 2 10 # a hung task, which would require a hard reset. echo echo "Testing with the file missing..." -for i in $(seq 1 5); do - test_batched_request_firmware_nofile $i -done - -for i in $(seq 1 5); do - test_batched_request_firmware_into_buf_nofile $i -done - -for i in $(seq 1 5); do - test_batched_request_firmware_direct_nofile $i -done - -for i in $(seq 1 5); do - test_request_firmware_nowait_uevent_nofile $i -done - -for i in $(seq 1 5); do - test_request_firmware_nowait_custom_nofile $i -done +do_tests nofile _nofile # Partial loads cannot use fallback, so do not repeat tests. test_request_partial_firmware_into_buf_nofile 0 10 @@ -508,49 +498,13 @@ $RUN_XZ -k $FW_INTO_BUF config_set_name $NAME echo echo "Testing with both plain and xz files present..." -for i in $(seq 1 5); do - test_batched_request_firmware $i both -done - -for i in $(seq 1 5); do - test_batched_request_firmware_into_buf $i both -done - -for i in $(seq 1 5); do - test_batched_request_firmware_direct $i both -done - -for i in $(seq 1 5); do - test_request_firmware_nowait_uevent $i both -done - -for i in $(seq 1 5); do - test_request_firmware_nowait_custom $i both -done +do_tests both # test with only xz file present mv "$FW" "${FW}-orig" mv "$FW_INTO_BUF" "${FW_INTO_BUF}-orig" echo echo "Testing with only xz file present..." -for i in $(seq 1 5); do - test_batched_request_firmware $i xzonly -done - -for i in $(seq 1 5); do - test_batched_request_firmware_into_buf $i xzonly -done - -for i in $(seq 1 5); do - test_batched_request_firmware_direct $i xzonly -done - -for i in $(seq 1 5); do - test_request_firmware_nowait_uevent $i xzonly -done - -for i in $(seq 1 5); do - test_request_firmware_nowait_custom $i xzonly -done +do_tests xzonly exit 0 -- cgit v1.2.3 From bc67cac1032679ba5bcd8f2767f661248c2a0c9a Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 21 Apr 2022 17:29:08 +0200 Subject: selftests: firmware: Add ZSTD compressed file tests It's similar like XZ compressed files. For the simplicity, both XZ and ZSTD tests are done in a single function. The format is specified via $COMPRESS_FORMAT and the compression function is pre-defined. Link: https://lore.kernel.org/r/20210127154939.13288-5-tiwai@suse.de Signed-off-by: Takashi Iwai Link: https://lore.kernel.org/r/20220421152908.4718-6-tiwai@suse.de Signed-off-by: Greg Kroah-Hartman --- tools/testing/selftests/firmware/fw_filesystem.sh | 76 +++++++++++++++++------ tools/testing/selftests/firmware/fw_lib.sh | 12 +++- 2 files changed, 65 insertions(+), 23 deletions(-) diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh index 4a574be8b862..1a99aea0549e 100755 --- a/tools/testing/selftests/firmware/fw_filesystem.sh +++ b/tools/testing/selftests/firmware/fw_filesystem.sh @@ -12,6 +12,7 @@ TEST_DIR=$(dirname $0) source $TEST_DIR/fw_lib.sh RUN_XZ="xz -C crc32 --lzma2=dict=2MiB" +RUN_ZSTD="zstd -q" check_mods check_setup @@ -213,7 +214,7 @@ read_firmwares() else fwfile="$FW" fi - if [ "$1" = "xzonly" ]; then + if [ "$1" = "componly" ]; then fwfile="${fwfile}-orig" fi for i in $(seq 0 3); do @@ -237,7 +238,7 @@ read_partial_firmwares() fwfile="${FW}" fi - if [ "$1" = "xzonly" ]; then + if [ "$1" = "componly" ]; then fwfile="${fwfile}-orig" fi @@ -411,10 +412,8 @@ test_request_firmware_nowait_custom() config_unset_uevent RANDOM_FILE_PATH=$(setup_random_file) RANDOM_FILE="$(basename $RANDOM_FILE_PATH)" - if [ "$2" = "both" ]; then - $RUN_XZ -k $RANDOM_FILE_PATH - elif [ "$2" = "xzonly" ]; then - $RUN_XZ $RANDOM_FILE_PATH + if [ -n "$2" -a "$2" != "normal" ]; then + compress_"$2"_"$COMPRESS_FORMAT" $RANDOM_FILE_PATH fi config_set_name $RANDOM_FILE config_trigger_async @@ -490,21 +489,58 @@ test_request_partial_firmware_into_buf_nofile 0 5 test_request_partial_firmware_into_buf_nofile 1 6 test_request_partial_firmware_into_buf_nofile 2 10 -test "$HAS_FW_LOADER_COMPRESS" != "yes" && exit 0 +test_request_firmware_compressed () +{ + export COMPRESS_FORMAT="$1" -# test with both files present -$RUN_XZ -k $FW -$RUN_XZ -k $FW_INTO_BUF -config_set_name $NAME -echo -echo "Testing with both plain and xz files present..." -do_tests both + # test with both files present + compress_both_"$COMPRESS_FORMAT" $FW + compress_both_"$COMPRESS_FORMAT" $FW_INTO_BUF -# test with only xz file present -mv "$FW" "${FW}-orig" -mv "$FW_INTO_BUF" "${FW_INTO_BUF}-orig" -echo -echo "Testing with only xz file present..." -do_tests xzonly + config_set_name $NAME + echo + echo "Testing with both plain and $COMPRESS_FORMAT files present..." + do_tests both + + # test with only compressed file present + mv "$FW" "${FW}-orig" + mv "$FW_INTO_BUF" "${FW_INTO_BUF}-orig" + + config_set_name $NAME + echo + echo "Testing with only $COMPRESS_FORMAT file present..." + do_tests componly + + mv "${FW}-orig" "$FW" + mv "${FW_INTO_BUF}-orig" "$FW_INTO_BUF" +} + +compress_both_XZ () +{ + $RUN_XZ -k "$@" +} + +compress_componly_XZ () +{ + $RUN_XZ "$@" +} + +compress_both_ZSTD () +{ + $RUN_ZSTD -k "$@" +} + +compress_componly_ZSTD () +{ + $RUN_ZSTD --rm "$@" +} + +if test "$HAS_FW_LOADER_COMPRESS_XZ" = "yes"; then + test_request_firmware_compressed XZ +fi + +if test "$HAS_FW_LOADER_COMPRESS_ZSTD" = "yes"; then + test_request_firmware_compressed ZSTD +fi exit 0 diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh index 5b8c0fedee76..3fa8282b053b 100755 --- a/tools/testing/selftests/firmware/fw_lib.sh +++ b/tools/testing/selftests/firmware/fw_lib.sh @@ -62,7 +62,8 @@ check_setup() { HAS_FW_LOADER_USER_HELPER="$(kconfig_has CONFIG_FW_LOADER_USER_HELPER=y)" HAS_FW_LOADER_USER_HELPER_FALLBACK="$(kconfig_has CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y)" - HAS_FW_LOADER_COMPRESS="$(kconfig_has CONFIG_FW_LOADER_COMPRESS=y)" + HAS_FW_LOADER_COMPRESS_XZ="$(kconfig_has CONFIG_FW_LOADER_COMPRESS_XZ=y)" + HAS_FW_LOADER_COMPRESS_ZSTD="$(kconfig_has CONFIG_FW_LOADER_COMPRESS_ZSTD=y)" PROC_FW_IGNORE_SYSFS_FALLBACK="0" PROC_FW_FORCE_SYSFS_FALLBACK="0" @@ -98,9 +99,14 @@ check_setup() OLD_FWPATH="$(cat /sys/module/firmware_class/parameters/path)" - if [ "$HAS_FW_LOADER_COMPRESS" = "yes" ]; then + if [ "$HAS_FW_LOADER_COMPRESS_XZ" = "yes" ]; then if ! which xz 2> /dev/null > /dev/null; then - HAS_FW_LOADER_COMPRESS="" + HAS_FW_LOADER_COMPRESS_XZ="" + fi + fi + if [ "$HAS_FW_LOADER_COMPRESS_ZSTD" = "yes" ]; then + if ! which zstd 2> /dev/null > /dev/null; then + HAS_FW_LOADER_COMPRESS_ZSTD="" fi fi } -- cgit v1.2.3 From 6c2f421174273de8f83cde4286d1c076d43a2d35 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 19 Apr 2022 13:34:24 +0200 Subject: driver: platform: Add helper for safer setting of driver_override Several core drivers and buses expect that driver_override is a dynamically allocated memory thus later they can kfree() it. However such assumption is not documented, there were in the past and there are already users setting it to a string literal. This leads to kfree() of static memory during device release (e.g. in error paths or during unbind): kernel BUG at ../mm/slub.c:3960! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM ... (kfree) from [] (platform_device_release+0x88/0xb4) (platform_device_release) from [] (device_release+0x2c/0x90) (device_release) from [] (kobject_put+0xec/0x20c) (kobject_put) from [] (exynos5_clk_probe+0x154/0x18c) (exynos5_clk_probe) from [] (platform_drv_probe+0x6c/0xa4) (platform_drv_probe) from [] (really_probe+0x280/0x414) (really_probe) from [] (driver_probe_device+0x78/0x1c4) (driver_probe_device) from [] (bus_for_each_drv+0x74/0xb8) (bus_for_each_drv) from [] (__device_attach+0xd4/0x16c) (__device_attach) from [] (bus_probe_device+0x88/0x90) (bus_probe_device) from [] (device_add+0x3dc/0x62c) (device_add) from [] (of_platform_device_create_pdata+0x94/0xbc) (of_platform_device_create_pdata) from [] (of_platform_bus_create+0x1a8/0x4fc) (of_platform_bus_create) from [] (of_platform_bus_create+0x20c/0x4fc) (of_platform_bus_create) from [] (of_platform_populate+0x84/0x118) (of_platform_populate) from [] (of_platform_default_populate_init+0xa0/0xb8) (of_platform_default_populate_init) from [] (do_one_initcall+0x8c/0x404) Provide a helper which clearly documents the usage of driver_override. This will allow later to reuse the helper and reduce the amount of duplicated code. Convert the platform driver to use a new helper and make the driver_override field const char (it is not modified by the core). Reviewed-by: Rafael J. Wysocki Acked-by: Rafael J. Wysocki Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20220419113435.246203-2-krzysztof.kozlowski@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/base/driver.c | 69 +++++++++++++++++++++++++++++++++++++++++ drivers/base/platform.c | 28 +++-------------- include/linux/device/driver.h | 2 ++ include/linux/platform_device.h | 6 +++- 4 files changed, 80 insertions(+), 25 deletions(-) diff --git a/drivers/base/driver.c b/drivers/base/driver.c index 8c0d33e182fd..1b9d47b10bd0 100644 --- a/drivers/base/driver.c +++ b/drivers/base/driver.c @@ -30,6 +30,75 @@ static struct device *next_device(struct klist_iter *i) return dev; } +/** + * driver_set_override() - Helper to set or clear driver override. + * @dev: Device to change + * @override: Address of string to change (e.g. &device->driver_override); + * The contents will be freed and hold newly allocated override. + * @s: NUL-terminated string, new driver name to force a match, pass empty + * string to clear it ("" or "\n", where the latter is only for sysfs + * interface). + * @len: length of @s + * + * Helper to set or clear driver override in a device, intended for the cases + * when the driver_override field is allocated by driver/bus code. + * + * Returns: 0 on success or a negative error code on failure. + */ +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len) +{ + const char *new, *old; + char *cp; + + if (!override || !s) + return -EINVAL; + + /* + * The stored value will be used in sysfs show callback (sysfs_emit()), + * which has a length limit of PAGE_SIZE and adds a trailing newline. + * Thus we can store one character less to avoid truncation during sysfs + * show. + */ + if (len >= (PAGE_SIZE - 1)) + return -EINVAL; + + if (!len) { + /* Empty string passed - clear override */ + device_lock(dev); + old = *override; + *override = NULL; + device_unlock(dev); + kfree(old); + + return 0; + } + + cp = strnchr(s, len, '\n'); + if (cp) + len = cp - s; + + new = kstrndup(s, len, GFP_KERNEL); + if (!new) + return -ENOMEM; + + device_lock(dev); + old = *override; + if (cp != s) { + *override = new; + } else { + /* "\n" passed - clear override */ + kfree(new); + *override = NULL; + } + device_unlock(dev); + + kfree(old); + + return 0; +} +EXPORT_SYMBOL_GPL(driver_set_override); + /** * driver_for_each_device - Iterator for devices bound to a driver. * @drv: Driver we're iterating. diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 8cc272fd5c99..b684157b7f2f 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -1275,31 +1275,11 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct platform_device *pdev = to_platform_device(dev); - char *driver_override, *old, *cp; - - /* We need to keep extra room for a newline */ - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(dev); - old = pdev->driver_override; - if (strlen(driver_override)) { - pdev->driver_override = driver_override; - } else { - kfree(driver_override); - pdev->driver_override = NULL; - } - device_unlock(dev); + int ret; - kfree(old); + ret = driver_set_override(dev, &pdev->driver_override, buf, count); + if (ret) + return ret; return count; } diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h index 15e7c5e15d62..700453017e1c 100644 --- a/include/linux/device/driver.h +++ b/include/linux/device/driver.h @@ -151,6 +151,8 @@ extern int __must_check driver_create_file(struct device_driver *driver, extern void driver_remove_file(struct device_driver *driver, const struct driver_attribute *attr); +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len); extern int __must_check driver_for_each_device(struct device_driver *drv, struct device *start, void *data, diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 7c96f169d274..582d83ed9a91 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -31,7 +31,11 @@ struct platform_device { struct resource *resource; const struct platform_device_id *id_entry; - char *driver_override; /* Driver name to force a match */ + /* + * Driver name to force a match. Do not set directly, because core + * frees it. Use driver_set_override() to set or clear it. + */ + const char *driver_override; /* MFD cell pointer */ struct mfd_cell *mfd_cell; -- cgit v1.2.3 From 6e67955087e72f1a5f64dd8014c27e1d9317fbb4 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 19 Apr 2022 13:34:25 +0200 Subject: amba: Use driver_set_override() instead of open-coding Use a helper to set driver_override to reduce the amount of duplicated code. Make the driver_override field const char, because it is not modified by the core and it matches other subsystems. Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20220419113435.246203-3-krzysztof.kozlowski@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/amba/bus.c | 28 ++++------------------------ include/linux/amba/bus.h | 6 +++++- 2 files changed, 9 insertions(+), 25 deletions(-) diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c index d3bd14aaabf6..f3d26d698b77 100644 --- a/drivers/amba/bus.c +++ b/drivers/amba/bus.c @@ -94,31 +94,11 @@ static ssize_t driver_override_store(struct device *_dev, const char *buf, size_t count) { struct amba_device *dev = to_amba_device(_dev); - char *driver_override, *old, *cp; - - /* We need to keep extra room for a newline */ - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(_dev); - old = dev->driver_override; - if (strlen(driver_override)) { - dev->driver_override = driver_override; - } else { - kfree(driver_override); - dev->driver_override = NULL; - } - device_unlock(_dev); + int ret; - kfree(old); + ret = driver_set_override(_dev, &dev->driver_override, buf, count); + if (ret) + return ret; return count; } diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h index 6562f543c3e0..93799a72ff82 100644 --- a/include/linux/amba/bus.h +++ b/include/linux/amba/bus.h @@ -70,7 +70,11 @@ struct amba_device { unsigned int cid; struct amba_cs_uci_id uci; unsigned int irq[AMBA_NR_IRQS]; - char *driver_override; + /* + * Driver name to force a match. Do not set directly, because core + * frees it. Use driver_set_override() to set or clear it. + */ + const char *driver_override; }; struct amba_driver { -- cgit v1.2.3 From 5688f212e98a2469583a067fa5da4312ddc4e357 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 19 Apr 2022 13:34:26 +0200 Subject: fsl-mc: Use driver_set_override() instead of open-coding Use a helper to set driver_override to reduce the amount of duplicated code. Make the driver_override field const char, because it is not modified by the core and it matches other subsystems. Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20220419113435.246203-4-krzysztof.kozlowski@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/bus/fsl-mc/fsl-mc-bus.c | 25 ++++--------------------- include/linux/fsl/mc.h | 6 ++++-- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c index 8fd4a356a86e..ba01c7f4de92 100644 --- a/drivers/bus/fsl-mc/fsl-mc-bus.c +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c @@ -166,31 +166,14 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev); - char *driver_override, *old = mc_dev->driver_override; - char *cp; + int ret; if (WARN_ON(dev->bus != &fsl_mc_bus_type)) return -EINVAL; - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - if (strlen(driver_override)) { - mc_dev->driver_override = driver_override; - } else { - kfree(driver_override); - mc_dev->driver_override = NULL; - } - - kfree(old); + ret = driver_set_override(dev, &mc_dev->driver_override, buf, count); + if (ret) + return ret; return count; } diff --git a/include/linux/fsl/mc.h b/include/linux/fsl/mc.h index 7b6c42bfb660..7a87ab9eba99 100644 --- a/include/linux/fsl/mc.h +++ b/include/linux/fsl/mc.h @@ -170,7 +170,9 @@ struct fsl_mc_obj_desc { * @regions: pointer to array of MMIO region entries * @irqs: pointer to array of pointers to interrupts allocated to this device * @resource: generic resource associated with this MC object device, if any. - * @driver_override: driver name to force a match + * @driver_override: driver name to force a match; do not set directly, + * because core frees it; use driver_set_override() to + * set or clear it. * * Generic device object for MC object devices that are "attached" to a * MC bus. @@ -204,7 +206,7 @@ struct fsl_mc_device { struct fsl_mc_device_irq **irqs; struct fsl_mc_resource *resource; struct device_link *consumer_link; - char *driver_override; + const char *driver_override; }; #define to_fsl_mc_device(_dev) \ -- cgit v1.2.3 From 01ed100276bdac259f1b418b998904a7486b0b68 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 19 Apr 2022 13:34:27 +0200 Subject: hv: Use driver_set_override() instead of open-coding Use a helper to set driver_override to the reduce amount of duplicated code. Make the driver_override field const char, because it is not modified by the core and it matches other subsystems. Reviewed-by: Michael Kelley Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20220419113435.246203-5-krzysztof.kozlowski@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/hv/vmbus_drv.c | 28 ++++------------------------ include/linux/hyperv.h | 6 +++++- 2 files changed, 9 insertions(+), 25 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 14de17087864..607e40aba18e 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -575,31 +575,11 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct hv_device *hv_dev = device_to_hv_device(dev); - char *driver_override, *old, *cp; - - /* We need to keep extra room for a newline */ - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(dev); - old = hv_dev->driver_override; - if (strlen(driver_override)) { - hv_dev->driver_override = driver_override; - } else { - kfree(driver_override); - hv_dev->driver_override = NULL; - } - device_unlock(dev); + int ret; - kfree(old); + ret = driver_set_override(dev, &hv_dev->driver_override, buf, count); + if (ret) + return ret; return count; } diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index fe2e0179ed51..12e2336b23b7 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1257,7 +1257,11 @@ struct hv_device { u16 device_id; struct device device; - char *driver_override; /* Driver name to force a match */ + /* + * Driver name to force a match. Do not set directly, because core + * frees it. Use driver_set_override() to set or clear it. + */ + const char *driver_override; struct vmbus_channel *channel; struct kset *channels_kset; -- cgit v1.2.3 From 23d99baf9d729ca30b2fb6798a7b403a37bfb800 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 19 Apr 2022 13:34:28 +0200 Subject: PCI: Use driver_set_override() instead of open-coding Use a helper to set driver_override to the reduce amount of duplicated code. Make the driver_override field const char, because it is not modified by the core and it matches other subsystems. Reviewed-by: Andy Shevchenko Acked-by: Bjorn Helgaas Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20220419113435.246203-6-krzysztof.kozlowski@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/pci/pci-sysfs.c | 28 ++++------------------------ include/linux/pci.h | 6 +++++- 2 files changed, 9 insertions(+), 25 deletions(-) diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index c263ffc5884a..fc804e08e3cb 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -567,31 +567,11 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct pci_dev *pdev = to_pci_dev(dev); - char *driver_override, *old, *cp; - - /* We need to keep extra room for a newline */ - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(dev); - old = pdev->driver_override; - if (strlen(driver_override)) { - pdev->driver_override = driver_override; - } else { - kfree(driver_override); - pdev->driver_override = NULL; - } - device_unlock(dev); + int ret; - kfree(old); + ret = driver_set_override(dev, &pdev->driver_override, buf, count); + if (ret) + return ret; return count; } diff --git a/include/linux/pci.h b/include/linux/pci.h index 60adf42460ab..844d38f589cf 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -516,7 +516,11 @@ struct pci_dev { u16 acs_cap; /* ACS Capability offset */ phys_addr_t rom; /* Physical address if not from BAR */ size_t romlen; /* Length if not from BAR */ - char *driver_override; /* Driver name to force a match */ + /* + * Driver name to force a match. Do not set directly, because core + * frees it. Use driver_set_override() to set or clear it. + */ + const char *driver_override; unsigned long priv_flags; /* Private flags for the PCI driver */ -- cgit v1.2.3 From 1e8ee51212b4876f1a8ca4b78d2499e2fc442cf3 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 19 Apr 2022 13:34:29 +0200 Subject: s390/cio: Use driver_set_override() instead of open-coding Use a helper to set driver_override to the reduce amount of duplicated code. Make the driver_override field const char, because it is not modified by the core and it matches other subsystems. Acked-by: Vineeth Vijayan Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20220419113435.246203-7-krzysztof.kozlowski@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/s390/cio/cio.h | 6 +++++- drivers/s390/cio/css.c | 28 ++++------------------------ 2 files changed, 9 insertions(+), 25 deletions(-) diff --git a/drivers/s390/cio/cio.h b/drivers/s390/cio/cio.h index 1cb9daf9c645..fa8df50bb49e 100644 --- a/drivers/s390/cio/cio.h +++ b/drivers/s390/cio/cio.h @@ -103,7 +103,11 @@ struct subchannel { struct work_struct todo_work; struct schib_config config; u64 dma_mask; - char *driver_override; /* Driver name to force a match */ + /* + * Driver name to force a match. Do not set directly, because core + * frees it. Use driver_set_override() to set or clear it. + */ + const char *driver_override; } __attribute__ ((aligned(8))); DECLARE_PER_CPU_ALIGNED(struct irb, cio_irb); diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c index fa8293335077..913b6ddd040b 100644 --- a/drivers/s390/cio/css.c +++ b/drivers/s390/cio/css.c @@ -338,31 +338,11 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct subchannel *sch = to_subchannel(dev); - char *driver_override, *old, *cp; - - /* We need to keep extra room for a newline */ - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(dev); - old = sch->driver_override; - if (strlen(driver_override)) { - sch->driver_override = driver_override; - } else { - kfree(driver_override); - sch->driver_override = NULL; - } - device_unlock(dev); + int ret; - kfree(old); + ret = driver_set_override(dev, &sch->driver_override, buf, count); + if (ret) + return ret; return count; } -- cgit v1.2.3 From 19368f0f23e80929691dd5b1354832c0e0494419 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 19 Apr 2022 13:34:30 +0200 Subject: spi: Use helper for safer setting of driver_override Use a helper to set driver_override to the reduce amount of duplicated code. Reviewed-by: Mark Brown Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20220419113435.246203-8-krzysztof.kozlowski@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/spi/spi.c | 26 ++++---------------------- include/linux/spi/spi.h | 2 ++ 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 2e6d6bbeb784..1bbfb57d687a 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -71,29 +71,11 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct spi_device *spi = to_spi_device(dev); - const char *end = memchr(buf, '\n', count); - const size_t len = end ? end - buf : count; - const char *driver_override, *old; - - /* We need to keep extra room for a newline when displaying value */ - if (len >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, len, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; + int ret; - device_lock(dev); - old = spi->driver_override; - if (len) { - spi->driver_override = driver_override; - } else { - /* Empty string, disable driver override */ - spi->driver_override = NULL; - kfree(driver_override); - } - device_unlock(dev); - kfree(old); + ret = driver_set_override(dev, &spi->driver_override, buf, count); + if (ret) + return ret; return count; } diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 5f8c063ddff4..f0177f9b6e13 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -138,6 +138,8 @@ extern int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer); * for driver coldplugging, and in uevents used for hotplugging * @driver_override: If the name of a driver is written to this attribute, then * the device will bind to the named driver and only the named driver. + * Do not set directly, because core frees it; use driver_set_override() to + * set or clear it. * @cs_gpiod: gpio descriptor of the chipselect line (optional, NULL when * not using a GPIO line) * @word_delay: delay to be inserted between consecutive -- cgit v1.2.3 From 240bf4e665741241983580812a2be1b033f120ee Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 19 Apr 2022 13:34:31 +0200 Subject: vdpa: Use helper for safer setting of driver_override Use a helper to set driver_override to the reduce amount of duplicated code. Acked-by: Michael S. Tsirkin Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20220419113435.246203-9-krzysztof.kozlowski@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/vdpa/vdpa.c | 29 ++++------------------------- include/linux/vdpa.h | 4 +++- 2 files changed, 7 insertions(+), 26 deletions(-) diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index 2b75c00b1005..33d1ad60cba7 100644 --- a/drivers/vdpa/vdpa.c +++ b/drivers/vdpa/vdpa.c @@ -77,32 +77,11 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct vdpa_device *vdev = dev_to_vdpa(dev); - const char *driver_override, *old; - char *cp; + int ret; - /* We need to keep extra room for a newline */ - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(dev); - old = vdev->driver_override; - if (strlen(driver_override)) { - vdev->driver_override = driver_override; - } else { - kfree(driver_override); - vdev->driver_override = NULL; - } - device_unlock(dev); - - kfree(old); + ret = driver_set_override(dev, &vdev->driver_override, buf, count); + if (ret) + return ret; return count; } diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h index 8943a209202e..c0a5083632ab 100644 --- a/include/linux/vdpa.h +++ b/include/linux/vdpa.h @@ -64,7 +64,9 @@ struct vdpa_mgmt_dev; * struct vdpa_device - representation of a vDPA device * @dev: underlying device * @dma_dev: the actual device that is performing DMA - * @driver_override: driver name to force a match + * @driver_override: driver name to force a match; do not set directly, + * because core frees it; use driver_set_override() to + * set or clear it. * @config: the configuration ops for this device. * @cf_mutex: Protects get and set access to configuration layout. * @index: device index -- cgit v1.2.3 From fb4ac6f18be1a6ac0dfdb25adfcf26462c6e2ac0 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 19 Apr 2022 13:34:32 +0200 Subject: clk: imx: scu: Fix kfree() of static memory on setting driver_override The driver_override field from platform driver should not be initialized from static memory (string literal) because the core later kfree() it, for example when driver_override is set via sysfs. Use dedicated helper to set driver_override properly. Fixes: 77d8f3068c63 ("clk: imx: scu: add two cells binding support") Acked-by: Stephen Boyd Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20220419113435.246203-10-krzysztof.kozlowski@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/clk/imx/clk-scu.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c index 083da31dc3ea..4b2268b7d0d0 100644 --- a/drivers/clk/imx/clk-scu.c +++ b/drivers/clk/imx/clk-scu.c @@ -683,7 +683,12 @@ struct clk_hw *imx_clk_scu_alloc_dev(const char *name, return ERR_PTR(ret); } - pdev->driver_override = "imx-scu-clk"; + ret = driver_set_override(&pdev->dev, &pdev->driver_override, + "imx-scu-clk", strlen("imx-scu-clk")); + if (ret) { + platform_device_put(pdev); + return ERR_PTR(ret); + } ret = imx_clk_scu_attach_pd(&pdev->dev, rsrc_id); if (ret) -- cgit v1.2.3 From 0f4b20ef41696a625e43587563d538bb4dd60d9c Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 19 Apr 2022 13:34:33 +0200 Subject: slimbus: qcom-ngd: Fix kfree() of static memory on setting driver_override The driver_override field from platform driver should not be initialized from static memory (string literal) because the core later kfree() it, for example when driver_override is set via sysfs. Use dedicated helper to set driver_override properly. Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver") Reviewed-by: Srinivas Kandagatla Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20220419113435.246203-11-krzysztof.kozlowski@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/slimbus/qcom-ngd-ctrl.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c index 7040293c2ee8..e5d9fdb81eb0 100644 --- a/drivers/slimbus/qcom-ngd-ctrl.c +++ b/drivers/slimbus/qcom-ngd-ctrl.c @@ -1434,6 +1434,7 @@ static int of_qcom_slim_ngd_register(struct device *parent, const struct of_device_id *match; struct device_node *node; u32 id; + int ret; match = of_match_node(qcom_slim_ngd_dt_match, parent->of_node); data = match->data; @@ -1455,7 +1456,17 @@ static int of_qcom_slim_ngd_register(struct device *parent, } ngd->id = id; ngd->pdev->dev.parent = parent; - ngd->pdev->driver_override = QCOM_SLIM_NGD_DRV_NAME; + + ret = driver_set_override(&ngd->pdev->dev, + &ngd->pdev->driver_override, + QCOM_SLIM_NGD_DRV_NAME, + strlen(QCOM_SLIM_NGD_DRV_NAME)); + if (ret) { + platform_device_put(ngd->pdev); + kfree(ngd); + of_node_put(node); + return ret; + } ngd->pdev->dev.of_node = node; ctrl->ngd = ngd; -- cgit v1.2.3 From e5f89131a06142e91073b6959d91cea73861d40e Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 19 Apr 2022 13:34:34 +0200 Subject: rpmsg: Constify local variable in field store macro Memory pointed by variable 'old' in field store macro is not modified, so it can be made a pointer to const. Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20220419113435.246203-12-krzysztof.kozlowski@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/rpmsg/rpmsg_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c index 79368a957d89..95fc283f6af7 100644 --- a/drivers/rpmsg/rpmsg_core.c +++ b/drivers/rpmsg/rpmsg_core.c @@ -400,7 +400,8 @@ field##_store(struct device *dev, struct device_attribute *attr, \ const char *buf, size_t sz) \ { \ struct rpmsg_device *rpdev = to_rpmsg_device(dev); \ - char *new, *old; \ + const char *old; \ + char *new; \ \ new = kstrndup(buf, sz, GFP_KERNEL); \ if (!new) \ -- cgit v1.2.3 From 42cd402b8fd4672b692400fe5f9eecd55d2794ac Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 19 Apr 2022 13:34:35 +0200 Subject: rpmsg: Fix kfree() of static memory on setting driver_override The driver_override field from platform driver should not be initialized from static memory (string literal) because the core later kfree() it, for example when driver_override is set via sysfs. Use dedicated helper to set driver_override properly. Fixes: 950a7388f02b ("rpmsg: Turn name service into a stand alone driver") Fixes: c0cdc19f84a4 ("rpmsg: Driver for user space endpoint interface") Reviewed-by: Bjorn Andersson Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20220419113435.246203-13-krzysztof.kozlowski@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/rpmsg/rpmsg_internal.h | 13 +++++++++++-- drivers/rpmsg/rpmsg_ns.c | 14 ++++++++++++-- include/linux/rpmsg.h | 6 ++++-- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h index d4b23fd019a8..3e81642238d2 100644 --- a/drivers/rpmsg/rpmsg_internal.h +++ b/drivers/rpmsg/rpmsg_internal.h @@ -94,10 +94,19 @@ int rpmsg_release_channel(struct rpmsg_device *rpdev, */ static inline int rpmsg_ctrldev_register_device(struct rpmsg_device *rpdev) { + int ret; + strcpy(rpdev->id.name, "rpmsg_ctrl"); - rpdev->driver_override = "rpmsg_ctrl"; + ret = driver_set_override(&rpdev->dev, &rpdev->driver_override, + rpdev->id.name, strlen(rpdev->id.name)); + if (ret) + return ret; + + ret = rpmsg_register_device(rpdev); + if (ret) + kfree(rpdev->driver_override); - return rpmsg_register_device(rpdev); + return ret; } #endif diff --git a/drivers/rpmsg/rpmsg_ns.c b/drivers/rpmsg/rpmsg_ns.c index 762ff1ae279f..8eb8f328237e 100644 --- a/drivers/rpmsg/rpmsg_ns.c +++ b/drivers/rpmsg/rpmsg_ns.c @@ -20,12 +20,22 @@ */ int rpmsg_ns_register_device(struct rpmsg_device *rpdev) { + int ret; + strcpy(rpdev->id.name, "rpmsg_ns"); - rpdev->driver_override = "rpmsg_ns"; + ret = driver_set_override(&rpdev->dev, &rpdev->driver_override, + rpdev->id.name, strlen(rpdev->id.name)); + if (ret) + return ret; + rpdev->src = RPMSG_NS_ADDR; rpdev->dst = RPMSG_NS_ADDR; - return rpmsg_register_device(rpdev); + ret = rpmsg_register_device(rpdev); + if (ret) + kfree(rpdev->driver_override); + + return ret; } EXPORT_SYMBOL(rpmsg_ns_register_device); diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h index 02fa9116cd60..20c8cd1cde21 100644 --- a/include/linux/rpmsg.h +++ b/include/linux/rpmsg.h @@ -41,7 +41,9 @@ struct rpmsg_channel_info { * rpmsg_device - device that belong to the rpmsg bus * @dev: the device struct * @id: device id (used to match between rpmsg drivers and devices) - * @driver_override: driver name to force a match + * @driver_override: driver name to force a match; do not set directly, + * because core frees it; use driver_set_override() to + * set or clear it. * @src: local address * @dst: destination address * @ept: the rpmsg endpoint of this channel @@ -51,7 +53,7 @@ struct rpmsg_channel_info { struct rpmsg_device { struct device dev; struct rpmsg_device_id id; - char *driver_override; + const char *driver_override; u32 src; u32 dst; struct rpmsg_endpoint *ept; -- cgit v1.2.3 From 4ac4a90d7728b161f0ce0527feb19d60af961dfb Mon Sep 17 00:00:00 2001 From: Russ Weight Date: Thu, 21 Apr 2022 14:21:57 -0700 Subject: firmware_loader: Clear data and size in fw_free_paged_buf The fw_free_paged_buf() function resets the paged buffer information in the fw_priv data structure. Additionally, clear the data and size members of fw_priv in order to facilitate the reuse of fw_priv. This is being done in preparation for enabling userspace to initiate multiple firmware uploads using this sysfs interface. Reviewed-by: Luis Chamberlain Reviewed-by: Tianfei zhang Tested-by: Matthew Gerlach Signed-off-by: Russ Weight Link: https://lore.kernel.org/r/20220421212204.36052-2-russell.h.weight@intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 74830aeec7f6..3d9b2074912d 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -254,6 +254,8 @@ void fw_free_paged_buf(struct fw_priv *fw_priv) fw_priv->pages = NULL; fw_priv->page_array_size = 0; fw_priv->nr_pages = 0; + fw_priv->data = NULL; + fw_priv->size = 0; } int fw_grow_paged_buf(struct fw_priv *fw_priv, int pages_needed) -- cgit v1.2.3 From 736da0b657f615db7e29606eb8818871534a8943 Mon Sep 17 00:00:00 2001 From: Russ Weight Date: Thu, 21 Apr 2022 14:21:58 -0700 Subject: firmware_loader: Check fw_state_is_done in loading_store Rename fw_sysfs_done() and fw_sysfs_loading() to fw_state_is_done() and fw_state_is_loading() respectively, and place them along side companion functions in drivers/base/firmware_loader/firmware.h. Use the fw_state_is_done() function to exit early from firmware_loading_store() if the state is already "done". This is being done in preparation for supporting persistent sysfs nodes to allow userspace to upload firmware to a device, potentially reusing the sysfs loading and data files multiple times. Reviewed-by: Luis Chamberlain Reviewed-by: Tianfei zhang Tested-by: Matthew Gerlach Signed-off-by: Russ Weight Link: https://lore.kernel.org/r/20220421212204.36052-3-russell.h.weight@intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/fallback.c | 28 ++++++++-------------------- drivers/base/firmware_loader/firmware.h | 10 ++++++++++ 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c index 4afb0e9312c0..8063eb595719 100644 --- a/drivers/base/firmware_loader/fallback.c +++ b/drivers/base/firmware_loader/fallback.c @@ -58,16 +58,6 @@ static long firmware_loading_timeout(void) __firmware_loading_timeout() * HZ : MAX_JIFFY_OFFSET; } -static inline bool fw_sysfs_done(struct fw_priv *fw_priv) -{ - return __fw_state_check(fw_priv, FW_STATUS_DONE); -} - -static inline bool fw_sysfs_loading(struct fw_priv *fw_priv) -{ - return __fw_state_check(fw_priv, FW_STATUS_LOADING); -} - static inline int fw_sysfs_wait_timeout(struct fw_priv *fw_priv, long timeout) { return __fw_state_wait_common(fw_priv, timeout); @@ -91,7 +81,7 @@ static void __fw_load_abort(struct fw_priv *fw_priv) * There is a small window in which user can write to 'loading' * between loading done/aborted and disappearance of 'loading' */ - if (fw_state_is_aborted(fw_priv) || fw_sysfs_done(fw_priv)) + if (fw_state_is_aborted(fw_priv) || fw_state_is_done(fw_priv)) return; fw_state_aborted(fw_priv); @@ -220,7 +210,7 @@ static ssize_t firmware_loading_show(struct device *dev, mutex_lock(&fw_lock); if (fw_sysfs->fw_priv) - loading = fw_sysfs_loading(fw_sysfs->fw_priv); + loading = fw_state_is_loading(fw_sysfs->fw_priv); mutex_unlock(&fw_lock); return sysfs_emit(buf, "%d\n", loading); @@ -250,19 +240,17 @@ static ssize_t firmware_loading_store(struct device *dev, mutex_lock(&fw_lock); fw_priv = fw_sysfs->fw_priv; - if (fw_state_is_aborted(fw_priv)) + if (fw_state_is_aborted(fw_priv) || fw_state_is_done(fw_priv)) goto out; switch (loading) { case 1: /* discarding any previous partial load */ - if (!fw_sysfs_done(fw_priv)) { - fw_free_paged_buf(fw_priv); - fw_state_start(fw_priv); - } + fw_free_paged_buf(fw_priv); + fw_state_start(fw_priv); break; case 0: - if (fw_sysfs_loading(fw_priv)) { + if (fw_state_is_loading(fw_priv)) { int rc; /* @@ -350,7 +338,7 @@ static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj, mutex_lock(&fw_lock); fw_priv = fw_sysfs->fw_priv; - if (!fw_priv || fw_sysfs_done(fw_priv)) { + if (!fw_priv || fw_state_is_done(fw_priv)) { ret_count = -ENODEV; goto out; } @@ -410,7 +398,7 @@ static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj, mutex_lock(&fw_lock); fw_priv = fw_sysfs->fw_priv; - if (!fw_priv || fw_sysfs_done(fw_priv)) { + if (!fw_priv || fw_state_is_done(fw_priv)) { retval = -ENODEV; goto out; } diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h index 2889f446ad41..d5ff32a1ba2d 100644 --- a/drivers/base/firmware_loader/firmware.h +++ b/drivers/base/firmware_loader/firmware.h @@ -149,6 +149,16 @@ static inline void fw_state_done(struct fw_priv *fw_priv) __fw_state_set(fw_priv, FW_STATUS_DONE); } +static inline bool fw_state_is_done(struct fw_priv *fw_priv) +{ + return __fw_state_check(fw_priv, FW_STATUS_DONE); +} + +static inline bool fw_state_is_loading(struct fw_priv *fw_priv) +{ + return __fw_state_check(fw_priv, FW_STATUS_LOADING); +} + int assign_fw(struct firmware *fw, struct device *device); #ifdef CONFIG_FW_LOADER -- cgit v1.2.3 From 3677563eb8731e1ad5970e3e57f74e5f9d63502a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thi=C3=A9baud=20Weksteen?= Date: Fri, 22 Apr 2022 11:32:15 +1000 Subject: firmware_loader: use kernel credentials when reading firmware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Device drivers may decide to not load firmware when probed to avoid slowing down the boot process should the firmware filesystem not be available yet. In this case, the firmware loading request may be done when a device file associated with the driver is first accessed. The credentials of the userspace process accessing the device file may be used to validate access to the firmware files requested by the driver. Ensure that the kernel assumes the responsibility of reading the firmware. This was observed on Android for a graphic driver loading their firmware when the device file (e.g. /dev/mali0) was first opened by userspace (i.e. surfaceflinger). The security context of surfaceflinger was used to validate the access to the firmware file (e.g. /vendor/firmware/mali.bin). Because previous configurations were relying on the userspace fallback mechanism, the security context of the userspace daemon (i.e. ueventd) was consistently used to read firmware files. More devices are found to use the command line argument firmware_class.path which gives the kernel the opportunity to read the firmware directly, hence surfacing this misattribution. Signed-off-by: ThiĆ©baud Weksteen Reviewed-by: Luis Chamberlain Tested-by: John Stultz Link: https://lore.kernel.org/r/20220422013215.2301793-1-tweek@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/main.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 3d9b2074912d..6ac6c7753acc 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -802,6 +802,8 @@ _request_firmware(const struct firmware **firmware_p, const char *name, size_t offset, u32 opt_flags) { struct firmware *fw = NULL; + struct cred *kern_cred = NULL; + const struct cred *old_cred; bool nondirect = false; int ret; @@ -818,6 +820,18 @@ _request_firmware(const struct firmware **firmware_p, const char *name, if (ret <= 0) /* error or already assigned */ goto out; + /* + * We are about to try to access the firmware file. Because we may have been + * called by a driver when serving an unrelated request from userland, we use + * the kernel credentials to read the file. + */ + kern_cred = prepare_kernel_cred(NULL); + if (!kern_cred) { + ret = -ENOMEM; + goto out; + } + old_cred = override_creds(kern_cred); + ret = fw_get_filesystem_firmware(device, fw->priv, "", NULL); /* Only full reads can support decompression, platform, and sysfs. */ @@ -848,6 +862,8 @@ _request_firmware(const struct firmware **firmware_p, const char *name, } else ret = assign_fw(fw, device); + revert_creds(old_cred); + out: if (ret < 0) { fw_abort_batch_reqs(fw); -- cgit v1.2.3 From 4e224719f5d9b92abf1e0edfb2a83053208f3026 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Fri, 22 Apr 2022 10:13:48 +0200 Subject: drivers/base/memory: Fix an unlikely reference counting issue in __add_memory_block() __add_memory_block() calls both put_device() and device_unregister() when storing the memory block into the xarray. This is incorrect because xarray doesn't take an additional reference and device_unregister() already calls put_device(). Triggering the issue looks really unlikely and its only effect should be to log a spurious warning about a ref counted issue. Fixes: 4fb6eabf1037 ("drivers/base/memory.c: cache memory blocks in xarray to accelerate lookup") Signed-off-by: Christophe JAILLET Acked-by: Michal Hocko Reviewed-by: David Hildenbrand Link: https://lore.kernel.org/r/d44c63d78affe844f020dc02ad6af29abc448fc4.1650611702.git.christophe.jaillet@wanadoo.fr Signed-off-by: Greg Kroah-Hartman --- drivers/base/memory.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/base/memory.c b/drivers/base/memory.c index 7222ff9b5e05..084d67fd55cc 100644 --- a/drivers/base/memory.c +++ b/drivers/base/memory.c @@ -636,10 +636,9 @@ static int __add_memory_block(struct memory_block *memory) } ret = xa_err(xa_store(&memory_blocks, memory->dev.id, memory, GFP_KERNEL)); - if (ret) { - put_device(&memory->dev); + if (ret) device_unregister(&memory->dev); - } + return ret; } -- cgit v1.2.3 From 5b5bfecaa333fb6a0cce1bfc4852a622dacfed1d Mon Sep 17 00:00:00 2001 From: SeongJae Park Date: Tue, 19 Apr 2022 12:16:36 +0000 Subject: scripts/get_abi: Fix wrong script file name in the help message The help message of 'get_abi.pl' is mistakenly saying it's 'abi_book.pl'. This commit fixes the wrong name in the help message. Fixes: bbc249f2b859 ("scripts: add an script to parse the ABI files") Signed-off-by: SeongJae Park Link: https://lore.kernel.org/r/20220419121636.290407-1-sj@kernel.org Signed-off-by: Greg Kroah-Hartman --- scripts/get_abi.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/get_abi.pl b/scripts/get_abi.pl index 1389db76cff3..0ffd5531242a 100755 --- a/scripts/get_abi.pl +++ b/scripts/get_abi.pl @@ -981,11 +981,11 @@ __END__ =head1 NAME -abi_book.pl - parse the Linux ABI files and produce a ReST book. +get_abi.pl - parse the Linux ABI files and produce a ReST book. =head1 SYNOPSIS -B [--debug ] [--enable-lineno] [--man] [--help] +B [--debug ] [--enable-lineno] [--man] [--help] [--(no-)rst-source] [--dir=] [--show-hints] [--search-string ] [] -- cgit v1.2.3 From e0c11a8b985137aebf4bcd07cd957b80ac23924d Mon Sep 17 00:00:00 2001 From: Russ Weight Date: Thu, 21 Apr 2022 14:21:59 -0700 Subject: firmware_loader: Split sysfs support from fallback In preparation for sharing the "loading" and "data" sysfs nodes with the new firmware upload support, split out sysfs functionality from fallback.c and fallback.h into sysfs.c and sysfs.h. This includes the firmware class driver code that is associated with the sysfs files and the fw_fallback_config support for the timeout sysfs node. CONFIG_FW_LOADER_SYSFS is created and is selected by CONFIG_FW_LOADER_USER_HELPER in order to include sysfs.o in firmware_class-objs. This is mostly just a code reorganization. There are a few symbols that change in scope, and these can be identified by looking at the header file changes. A few white-space warnings from checkpatch are also addressed in this patch. Reviewed-by: Luis Chamberlain Reviewed-by: Tianfei zhang Tested-by: Matthew Gerlach Signed-off-by: Russ Weight Link: https://lore.kernel.org/r/20220421212204.36052-4-russell.h.weight@intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/Kconfig | 4 + drivers/base/firmware_loader/Makefile | 1 + drivers/base/firmware_loader/fallback.c | 418 -------------------------------- drivers/base/firmware_loader/fallback.h | 46 +--- drivers/base/firmware_loader/sysfs.c | 401 ++++++++++++++++++++++++++++++ drivers