From d67907154808745b0fae5874edc7b0f78d33991c Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 2 Oct 2024 12:01:21 +0200 Subject: firmware: qcom: scm: suppress download mode error Stop spamming the logs with errors about missing mechanism for setting the so called download (or dump) mode for users that have not requested that feature to be enabled in the first place. This avoids the follow error being logged on boot as well as on shutdown when the feature it not available and download mode has not been enabled on the kernel command line: qcom_scm firmware:scm: No available mechanism for setting download mode Fixes: 79cb2cb8d89b ("firmware: qcom: scm: Disable SDI and write no dump to dump mode") Fixes: 781d32d1c970 ("firmware: qcom_scm: Clear download bit during reboot") Cc: Mukesh Ojha Cc: stable@vger.kernel.org # 6.4 Signed-off-by: Johan Hovold Reviewed-by: Mukesh Ojha Link: https://lore.kernel.org/r/20241002100122.18809-2-johan+linaro@kernel.org Signed-off-by: Bjorn Andersson --- drivers/firmware/qcom/qcom_scm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index 10986cb11ec0..e2ac595902ed 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -545,7 +545,7 @@ static void qcom_scm_set_download_mode(u32 dload_mode) } else if (__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_SET_DLOAD_MODE)) { ret = __qcom_scm_set_dload_mode(__scm->dev, !!dload_mode); - } else { + } else if (dload_mode) { dev_err(__scm->dev, "No available mechanism for setting download mode\n"); } -- cgit v1.2.3 From ca61d6836e6f4442a77762e1074d2706a2a6e578 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Mon, 30 Sep 2024 10:33:28 +0200 Subject: firmware: qcom: scm: fix a NULL-pointer dereference Some SCM calls can be invoked with __scm being NULL (the driver may not have been and will not be probed as there's no SCM entry in device-tree). Make sure we don't dereference a NULL pointer. Fixes: 449d0d84bcd8 ("firmware: qcom: scm: smc: switch to using the SCM allocator") Reported-by: Rudraksha Gupta Closes: https://lore.kernel.org/lkml/692cfe9a-8c05-4ce4-813e-82b3f310019a@gmail.com/ Reviewed-by: Konrad Dybcio Tested-by: Rudraksha Gupta Reviewed-by: Dmitry Baryshkov Reviewed-by: Stephan Gerhold Signed-off-by: Bartosz Golaszewski Reviewed-by: Kuldeep Singh Link: https://lore.kernel.org/r/20240930083328.17904-1-brgl@bgdev.pl Signed-off-by: Bjorn Andersson --- drivers/firmware/qcom/qcom_scm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index e2ac595902ed..f47fdf32d2da 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -216,7 +216,7 @@ static DEFINE_SPINLOCK(scm_query_lock); struct qcom_tzmem_pool *qcom_scm_get_tzmem_pool(void) { - return __scm->mempool; + return __scm ? __scm->mempool : NULL; } static enum qcom_scm_convention __get_convention(void) -- cgit v1.2.3 From 83beece5aff75879bdfc6df8ba84ea88fd93050e Mon Sep 17 00:00:00 2001 From: Conor Dooley Date: Wed, 16 Oct 2024 17:35:06 +0100 Subject: firmware: microchip: auto-update: fix poll_complete() to not report spurious timeout errors fw_upload's poll_complete() is really intended for use with asynchronous write() implementations - or at least those where the write() loop may terminate without the kernel yet being aware of whether or not the firmware upload has succeeded. For auto-update, write() is only ever called once and will only return when uploading has completed, be that by passing or failing. The core fw_upload code only calls poll_complete() after the final call to write() has returned. However, the poll_complete() implementation in the auto-update driver was written to expect poll_complete() to be called from another context, and it waits for a completion signalled from write(). Since poll_complete() is actually called from the same context, after the write() loop has terminated, wait_for_completion() never sees the completion get signalled and always times out, causing programming to always report a failing. Since write() is full synchronous, and its return value will indicate whether or not programming passed or failed, poll_complete() serves no purpose and can be cut down to simply return FW_UPLOAD_ERR_NONE. Cc: stable@vger.kernel.org Fixes: ec5b0f1193ad4 ("firmware: microchip: add PolarFire SoC Auto Update support") Reported-by: Jamie Gibbons Tested-by: Jamie Gibbons Signed-off-by: Conor Dooley --- drivers/firmware/microchip/mpfs-auto-update.c | 42 +++++---------------------- 1 file changed, 7 insertions(+), 35 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/microchip/mpfs-auto-update.c b/drivers/firmware/microchip/mpfs-auto-update.c index 9ca5ee58edbd..0f7ec8848202 100644 --- a/drivers/firmware/microchip/mpfs-auto-update.c +++ b/drivers/firmware/microchip/mpfs-auto-update.c @@ -76,14 +76,11 @@ #define AUTO_UPDATE_INFO_SIZE SZ_1M #define AUTO_UPDATE_BITSTREAM_BASE (AUTO_UPDATE_DIRECTORY_SIZE + AUTO_UPDATE_INFO_SIZE) -#define AUTO_UPDATE_TIMEOUT_MS 60000 - struct mpfs_auto_update_priv { struct mpfs_sys_controller *sys_controller; struct device *dev; struct mtd_info *flash; struct fw_upload *fw_uploader; - struct completion programming_complete; size_t size_per_bitstream; bool cancel_request; }; @@ -156,19 +153,6 @@ static void mpfs_auto_update_cancel(struct fw_upload *fw_uploader) static enum fw_upload_err mpfs_auto_update_poll_complete(struct fw_upload *fw_uploader) { - struct mpfs_auto_update_priv *priv = fw_uploader->dd_handle; - int ret; - - /* - * There is no meaningful way to get the status of the programming while - * it is in progress, so attempting anything other than waiting for it - * to complete would be misplaced. - */ - ret = wait_for_completion_timeout(&priv->programming_complete, - msecs_to_jiffies(AUTO_UPDATE_TIMEOUT_MS)); - if (!ret) - return FW_UPLOAD_ERR_TIMEOUT; - return FW_UPLOAD_ERR_NONE; } @@ -349,33 +333,23 @@ static enum fw_upload_err mpfs_auto_update_write(struct fw_upload *fw_uploader, u32 offset, u32 size, u32 *written) { struct mpfs_auto_update_priv *priv = fw_uploader->dd_handle; - enum fw_upload_err err = FW_UPLOAD_ERR_NONE; int ret; - reinit_completion(&priv->programming_complete); - ret = mpfs_auto_update_write_bitstream(fw_uploader, data, offset, size, written); - if (ret) { - err = FW_UPLOAD_ERR_RW_ERROR; - goto out; - } + if (ret) + return FW_UPLOAD_ERR_RW_ERROR; - if (priv->cancel_request) { - err = FW_UPLOAD_ERR_CANCELED; - goto out; - } + if (priv->cancel_request) + return FW_UPLOAD_ERR_CANCELED; if (mpfs_auto_update_is_bitstream_info(data, size)) - goto out; + return FW_UPLOAD_ERR_NONE; ret = mpfs_auto_update_verify_image(fw_uploader); if (ret) - err = FW_UPLOAD_ERR_FW_INVALID; + return FW_UPLOAD_ERR_FW_INVALID; -out: - complete(&priv->programming_complete); - - return err; + return FW_UPLOAD_ERR_NONE; } static const struct fw_upload_ops mpfs_auto_update_ops = { @@ -461,8 +435,6 @@ static int mpfs_auto_update_probe(struct platform_device *pdev) return dev_err_probe(dev, ret, "The current bitstream does not support auto-update\n"); - init_completion(&priv->programming_complete); - fw_uploader = firmware_upload_register(THIS_MODULE, dev, "mpfs-auto-update", &mpfs_auto_update_ops, priv); if (IS_ERR(fw_uploader)) -- cgit v1.2.3 From 295416091e44806760ccf753aeafdafc0ae268f3 Mon Sep 17 00:00:00 2001 From: Xinqi Zhang Date: Wed, 16 Oct 2024 14:13:38 +0800 Subject: firmware: arm_scmi: Fix slab-use-after-free in scmi_bus_notifier() The scmi_dev->name is released prematurely in __scmi_device_destroy(), which causes slab-use-after-free when accessing scmi_dev->name in scmi_bus_notifier(). So move the release of scmi_dev->name to scmi_device_release() to avoid slab-use-after-free. | BUG: KASAN: slab-use-after-free in strncmp+0xe4/0xec | Read of size 1 at addr ffffff80a482bcc0 by task swapper/0/1 | | CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.6.38-debug #1 | Hardware name: Qualcomm Technologies, Inc. SA8775P Ride (DT) | Call trace: | dump_backtrace+0x94/0x114 | show_stack+0x18/0x24 | dump_stack_lvl+0x48/0x60 | print_report+0xf4/0x5b0 | kasan_report+0xa4/0xec | __asan_report_load1_noabort+0x20/0x2c | strncmp+0xe4/0xec | scmi_bus_notifier+0x5c/0x54c | notifier_call_chain+0xb4/0x31c | blocking_notifier_call_chain+0x68/0x9c | bus_notify+0x54/0x78 | device_del+0x1bc/0x840 | device_unregister+0x20/0xb4 | __scmi_device_destroy+0xac/0x280 | scmi_device_destroy+0x94/0xd0 | scmi_chan_setup+0x524/0x750 | scmi_probe+0x7fc/0x1508 | platform_probe+0xc4/0x19c | really_probe+0x32c/0x99c | __driver_probe_device+0x15c/0x3c4 | driver_probe_device+0x5c/0x170 | __driver_attach+0x1c8/0x440 | bus_for_each_dev+0xf4/0x178 | driver_attach+0x3c/0x58 | bus_add_driver+0x234/0x4d4 | driver_register+0xf4/0x3c0 | __platform_driver_register+0x60/0x88 | scmi_driver_init+0xb0/0x104 | do_one_initcall+0xb4/0x664 | kernel_init_freeable+0x3c8/0x894 | kernel_init+0x24/0x1e8 | ret_from_fork+0x10/0x20 | | Allocated by task 1: | kasan_save_stack+0x2c/0x54 | kasan_set_track+0x2c/0x40 | kasan_save_alloc_info+0x24/0x34 | __kasan_kmalloc+0xa0/0xb8 | __kmalloc_node_track_caller+0x6c/0x104 | kstrdup+0x48/0x84 | kstrdup_const+0x34/0x40 | __scmi_device_create.part.0+0x8c/0x408 | scmi_device_create+0x104/0x370 | scmi_chan_setup+0x2a0/0x750 | scmi_probe+0x7fc/0x1508 | platform_probe+0xc4/0x19c | really_probe+0x32c/0x99c | __driver_probe_device+0x15c/0x3c4 | driver_probe_device+0x5c/0x170 | __driver_attach+0x1c8/0x440 | bus_for_each_dev+0xf4/0x178 | driver_attach+0x3c/0x58 | bus_add_driver+0x234/0x4d4 | driver_register+0xf4/0x3c0 | __platform_driver_register+0x60/0x88 | scmi_driver_init+0xb0/0x104 | do_one_initcall+0xb4/0x664 | kernel_init_freeable+0x3c8/0x894 | kernel_init+0x24/0x1e8 | ret_from_fork+0x10/0x20 | | Freed by task 1: | kasan_save_stack+0x2c/0x54 | kasan_set_track+0x2c/0x40 | kasan_save_free_info+0x38/0x5c | __kasan_slab_free+0xe8/0x164 | __kmem_cache_free+0x11c/0x230 | kfree+0x70/0x130 | kfree_const+0x20/0x40 | __scmi_device_destroy+0x70/0x280 | scmi_device_destroy+0x94/0xd0 | scmi_chan_setup+0x524/0x750 | scmi_probe+0x7fc/0x1508 | platform_probe+0xc4/0x19c | really_probe+0x32c/0x99c | __driver_probe_device+0x15c/0x3c4 | driver_probe_device+0x5c/0x170 | __driver_attach+0x1c8/0x440 | bus_for_each_dev+0xf4/0x178 | driver_attach+0x3c/0x58 | bus_add_driver+0x234/0x4d4 | driver_register+0xf4/0x3c0 | __platform_driver_register+0x60/0x88 | scmi_driver_init+0xb0/0x104 | do_one_initcall+0xb4/0x664 | kernel_init_freeable+0x3c8/0x894 | kernel_init+0x24/0x1e8 | ret_from_fork+0x10/0x20 Fixes: ee7a9c9f67c5 ("firmware: arm_scmi: Add support for multiple device per protocol") Signed-off-by: Xinqi Zhang Reviewed-by: Cristian Marussi Reviewed-by: Bjorn Andersson Message-Id: <20241016-fix-arm-scmi-slab-use-after-free-v2-1-1783685ef90d@quicinc.com> Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/bus.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c index 96b2e5f9a8ef..157172a5f2b5 100644 --- a/drivers/firmware/arm_scmi/bus.c +++ b/drivers/firmware/arm_scmi/bus.c @@ -325,7 +325,10 @@ EXPORT_SYMBOL_GPL(scmi_driver_unregister); static void scmi_device_release(struct device *dev) { - kfree(to_scmi_dev(dev)); + struct scmi_device *scmi_dev = to_scmi_dev(dev); + + kfree_const(scmi_dev->name); + kfree(scmi_dev); } static void __scmi_device_destroy(struct scmi_device *scmi_dev) @@ -338,7 +341,6 @@ static void __scmi_device_destroy(struct scmi_device *scmi_dev) if (scmi_dev->protocol_id == SCMI_PROTOCOL_SYSTEM) atomic_set(&scmi_syspower_registered, 0); - kfree_const(scmi_dev->name); ida_free(&scmi_bus_id, scmi_dev->id); device_unregister(&scmi_dev->dev); } @@ -410,7 +412,6 @@ __scmi_device_create(struct device_node *np, struct device *parent, return scmi_dev; put_dev: - kfree_const(scmi_dev->name); put_device(&scmi_dev->dev); ida_free(&scmi_bus_id, id); return NULL; -- cgit v1.2.3 From a0a18e91eb3a6ef75a6de69dc00f206b913e3848 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 21 Oct 2024 18:15:44 +0100 Subject: firmware: arm_scmi: Reject clear channel request on A2P The clear channel transport operation is supposed to be called exclusively on the P2A channel from the agent, since it relinquishes the ownership of the channel to the platform, after this latter has initiated some sort of P2A communication. Make sure that, if it is ever called on a A2P, is logged and ignored. Signed-off-by: Cristian Marussi Reviewed-by: Florian Fainelli Message-Id: <20241021171544.2579551-1-cristian.marussi@arm.com> Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/common.h | 2 ++ drivers/firmware/arm_scmi/driver.c | 6 ++++++ 2 files changed, 8 insertions(+) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 6d9227db473f..c52ed6c64006 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -163,6 +163,7 @@ void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id); * used to initialize this channel * @dev: Reference to device in the SCMI hierarchy corresponding to this * channel + * @is_p2a: A flag to identify a channel as P2A (RX) * @rx_timeout_ms: The configured RX timeout in milliseconds. * @handle: Pointer to SCMI entity handle * @no_completion_irq: Flag to indicate that this channel has no completion @@ -174,6 +175,7 @@ void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id); struct scmi_chan_info { int id; struct device *dev; + bool is_p2a; unsigned int rx_timeout_ms; struct scmi_handle *handle; bool no_completion_irq; diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index a477b5ade38d..5bd4cc68a3e3 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -1048,6 +1048,11 @@ static inline void scmi_xfer_command_release(struct scmi_info *info, static inline void scmi_clear_channel(struct scmi_info *info, struct scmi_chan_info *cinfo) { + if (!cinfo->is_p2a) { + dev_warn(cinfo->dev, "Invalid clear on A2P channel !\n"); + return; + } + if (info->desc->ops->clear_channel) info->desc->ops->clear_channel(cinfo); } @@ -2638,6 +2643,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node, if (!cinfo) return -ENOMEM; + cinfo->is_p2a = !tx; cinfo->rx_timeout_ms = info->desc->max_rx_timeout_ms; /* Create a unique name for this transport device */ -- cgit v1.2.3 From f489f6c6eb26482010470d77bad3901a3de1b166 Mon Sep 17 00:00:00 2001 From: Qingqing Zhou Date: Wed, 23 Oct 2024 00:51:48 +0530 Subject: firmware: qcom: scm: Return -EOPNOTSUPP for unsupported SHM bridge enabling When enabling SHM bridge, QTEE returns 0 and sets error 4 in result to qcom_scm for unsupported platforms. Currently, tzmem interprets this as an unknown error rather than recognizing it as an unsupported platform. Error log: [ 0.177224] qcom_scm firmware:scm: error (____ptrval____): Failed to enable the TrustZone memory allocator [ 0.177244] qcom_scm firmware:scm: probe with driver qcom_scm failed with error 4 To address this, modify the function call qcom_scm_shm_bridge_enable() to remap result to indicate an unsupported error. This way, tzmem will correctly identify it as an unsupported platform case instead of reporting it as an error. Fixes: 178e19c0df1b ("firmware: qcom: scm: add support for SHM bridge operations") Signed-off-by: Qingqing Zhou Co-developed-by: Kuldeep Singh Signed-off-by: Kuldeep Singh Reviewed-by: Bartosz Golaszewski Reviewed-by: Mukesh Ojha Link: https://lore.kernel.org/r/20241022192148.1626633-1-quic_kuldsing@quicinc.com Signed-off-by: Bjorn Andersson --- drivers/firmware/qcom/qcom_scm.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index f47fdf32d2da..2e4260ba5f79 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -112,6 +112,7 @@ enum qcom_scm_qseecom_tz_cmd_info { }; #define QSEECOM_MAX_APP_NAME_SIZE 64 +#define SHMBRIDGE_RESULT_NOTSUPP 4 /* Each bit configures cold/warm boot address for one of the 4 CPUs */ static const u8 qcom_scm_cpu_cold_bits[QCOM_SCM_BOOT_MAX_CPUS] = { @@ -1361,6 +1362,8 @@ EXPORT_SYMBOL_GPL(qcom_scm_lmh_dcvsh_available); int qcom_scm_shm_bridge_enable(void) { + int ret; + struct qcom_scm_desc desc = { .svc = QCOM_SCM_SVC_MP, .cmd = QCOM_SCM_MP_SHM_BRIDGE_ENABLE, @@ -1373,7 +1376,15 @@ int qcom_scm_shm_bridge_enable(void) QCOM_SCM_MP_SHM_BRIDGE_ENABLE)) return -EOPNOTSUPP; - return qcom_scm_call(__scm->dev, &desc, &res) ?: res.result[0]; + ret = qcom_scm_call(__scm->dev, &desc, &res); + + if (ret) + return ret; + + if (res.result[0] == SHMBRIDGE_RESULT_NOTSUPP) + return -EOPNOTSUPP; + + return res.result[0]; } EXPORT_SYMBOL_GPL(qcom_scm_shm_bridge_enable); -- cgit v1.2.3 From 54962707f8b8b53812d3d7fca279a68c6e18faae Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 28 Oct 2024 12:01:51 +0000 Subject: firmware: arm_scmi: Use vendor string in max-rx-timeout-ms The original optional property was missing a vendor string prefix; this has been rectified. Fix the naming of such optional property in code too. Cc: Peng Fan Fixes: 1780e411ef94 ("firmware: arm_scmi: Use max-rx-timeout-ms from devicetree") Signed-off-by: Cristian Marussi Message-Id: <20241028120151.1301177-8-cristian.marussi@arm.com> Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 5bd4cc68a3e3..f8934d049d68 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -3048,10 +3048,10 @@ static const struct scmi_desc *scmi_transport_setup(struct device *dev) dev_info(dev, "Using %s\n", dev_driver_string(trans->supplier)); - ret = of_property_read_u32(dev->of_node, "max-rx-timeout-ms", + ret = of_property_read_u32(dev->of_node, "arm,max-rx-timeout-ms", &trans->desc->max_rx_timeout_ms); if (ret && ret != -EINVAL) - dev_err(dev, "Malformed max-rx-timeout-ms DT property.\n"); + dev_err(dev, "Malformed arm,max-rx-timeout-ms DT property.\n"); dev_info(dev, "SCMI max-rx-timeout: %dms\n", trans->desc->max_rx_timeout_ms); -- cgit v1.2.3