From d9679d0013a66849f23057978f92e76b255c50aa Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Wed, 13 Oct 2021 06:55:44 -0400 Subject: virtio: wrap config->reset calls This will enable cleanups down the road. The idea is to disable cbs, then add "flush_queued_cbs" callback as a parameter, this way drivers can flush any work queued after callbacks have been disabled. Signed-off-by: Michael S. Tsirkin Link: https://lore.kernel.org/r/20211013105226.20225-1-mst@redhat.com Signed-off-by: Michael S. Tsirkin --- drivers/firmware/arm_scmi/virtio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c index 87039c5c03fd..eefcc4146749 100644 --- a/drivers/firmware/arm_scmi/virtio.c +++ b/drivers/firmware/arm_scmi/virtio.c @@ -452,7 +452,7 @@ static void scmi_vio_remove(struct virtio_device *vdev) * outstanding message on any vqueue to be ignored by complete_cb: now * we can just stop processing buffers and destroy the vqueues. */ - vdev->config->reset(vdev); + virtio_reset_device(vdev); vdev->config->del_vqs(vdev); /* Ensure scmi_vdev is visible as NULL */ smp_store_mb(scmi_vdev, NULL); -- cgit v1.2.3 From d3e305592d69e21e36b76d24ca3c01971a2d09be Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 1 Dec 2021 14:25:25 +0100 Subject: firmware: qemu_fw_cfg: fix NULL-pointer deref on duplicate entries Commit fe3c60684377 ("firmware: Fix a reference count leak.") "fixed" a kobject leak in the file registration helper by properly calling kobject_put() for the entry in case registration of the object fails (e.g. due to a name collision). This would however result in a NULL pointer dereference when the release function tries to remove the never added entry from the fw_cfg_entry_cache list. Fix this by moving the list-removal out of the release function. Note that the offending commit was one of the benign looking umn.edu fixes which was reviewed but not reverted. [1][2] [1] https://lore.kernel.org/r/202105051005.49BFABCE@keescook [2] https://lore.kernel.org/all/YIg7ZOZvS3a8LjSv@kroah.com Fixes: fe3c60684377 ("firmware: Fix a reference count leak.") Cc: stable@vger.kernel.org # 5.8 Cc: Qiushi Wu Cc: Kees Cook Cc: Greg Kroah-Hartman Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20211201132528.30025-2-johan@kernel.org Signed-off-by: Michael S. Tsirkin --- drivers/firmware/qemu_fw_cfg.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c index 172c751a4f6c..a9c64ebfc49a 100644 --- a/drivers/firmware/qemu_fw_cfg.c +++ b/drivers/firmware/qemu_fw_cfg.c @@ -388,9 +388,7 @@ static void fw_cfg_sysfs_cache_cleanup(void) struct fw_cfg_sysfs_entry *entry, *next; list_for_each_entry_safe(entry, next, &fw_cfg_entry_cache, list) { - /* will end up invoking fw_cfg_sysfs_cache_delist() - * via each object's release() method (i.e. destructor) - */ + fw_cfg_sysfs_cache_delist(entry); kobject_put(&entry->kobj); } } @@ -448,7 +446,6 @@ static void fw_cfg_sysfs_release_entry(struct kobject *kobj) { struct fw_cfg_sysfs_entry *entry = to_entry(kobj); - fw_cfg_sysfs_cache_delist(entry); kfree(entry); } -- cgit v1.2.3 From 6004e351da50565fb561be85d45151dc9c370023 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 1 Dec 2021 14:25:26 +0100 Subject: firmware: qemu_fw_cfg: fix kobject leak in probe error path An initialised kobject must be freed using kobject_put() to avoid leaking associated resources (e.g. the object name). Commit fe3c60684377 ("firmware: Fix a reference count leak.") "fixed" the leak in the first error path of the file registration helper but left the second one unchanged. This "fix" would however result in a NULL pointer dereference due to the release function also removing the never added entry from the fw_cfg_entry_cache list. This has now been addressed. Fix the remaining kobject leak by restoring the common error path and adding the missing kobject_put(). Fixes: 75f3e8e47f38 ("firmware: introduce sysfs driver for QEMU's fw_cfg device") Cc: stable@vger.kernel.org # 4.6 Cc: Gabriel Somlo Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20211201132528.30025-3-johan@kernel.org Signed-off-by: Michael S. Tsirkin --- drivers/firmware/qemu_fw_cfg.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c index a9c64ebfc49a..ccb7ed62452f 100644 --- a/drivers/firmware/qemu_fw_cfg.c +++ b/drivers/firmware/qemu_fw_cfg.c @@ -603,15 +603,13 @@ static int fw_cfg_register_file(const struct fw_cfg_file *f) /* register entry under "/sys/firmware/qemu_fw_cfg/by_key/" */ err = kobject_init_and_add(&entry->kobj, &fw_cfg_sysfs_entry_ktype, fw_cfg_sel_ko, "%d", entry->select); - if (err) { - kobject_put(&entry->kobj); - return err; - } + if (err) + goto err_put_entry; /* add raw binary content access */ err = sysfs_create_bin_file(&entry->kobj, &fw_cfg_sysfs_attr_raw); if (err) - goto err_add_raw; + goto err_del_entry; /* try adding "/sys/firmware/qemu_fw_cfg/by_name/" symlink */ fw_cfg_build_symlink(fw_cfg_fname_kset, &entry->kobj, entry->name); @@ -620,9 +618,10 @@ static int fw_cfg_register_file(const struct fw_cfg_file *f) fw_cfg_sysfs_cache_enlist(entry); return 0; -err_add_raw: +err_del_entry: kobject_del(&entry->kobj); - kfree(entry); +err_put_entry: + kobject_put(&entry->kobj); return err; } -- cgit v1.2.3 From 1b656e9aad7f4886ed466094d1dc5ee4dd900d20 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 1 Dec 2021 14:25:27 +0100 Subject: firmware: qemu_fw_cfg: fix sysfs information leak Make sure to always NUL-terminate file names retrieved from the firmware to avoid accessing data beyond the entry slab buffer and exposing it through sysfs in case the firmware data is corrupt. Fixes: 75f3e8e47f38 ("firmware: introduce sysfs driver for QEMU's fw_cfg device") Cc: stable@vger.kernel.org # 4.6 Cc: Gabriel Somlo Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20211201132528.30025-4-johan@kernel.org Signed-off-by: Michael S. Tsirkin --- drivers/firmware/qemu_fw_cfg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c index ccb7ed62452f..f08e056ed0ae 100644 --- a/drivers/firmware/qemu_fw_cfg.c +++ b/drivers/firmware/qemu_fw_cfg.c @@ -598,7 +598,7 @@ static int fw_cfg_register_file(const struct fw_cfg_file *f) /* set file entry information */ entry->size = be32_to_cpu(f->size); entry->select = be16_to_cpu(f->select); - memcpy(entry->name, f->name, FW_CFG_MAX_FILE_PATH); + strscpy(entry->name, f->name, FW_CFG_MAX_FILE_PATH); /* register entry under "/sys/firmware/qemu_fw_cfg/by_key/" */ err = kobject_init_and_add(&entry->kobj, &fw_cfg_sysfs_entry_ktype, -- cgit v1.2.3 From 9f8b4ae2ac7dc5ff6e5dfa723c1ef2bad80a8c68 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 1 Dec 2021 14:25:28 +0100 Subject: firmware: qemu_fw_cfg: remove sysfs entries explicitly Explicitly remove the file entries from sysfs before dropping the final reference for symmetry reasons and for consistency with the rest of the driver. Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20211201132528.30025-5-johan@kernel.org Signed-off-by: Michael S. Tsirkin --- drivers/firmware/qemu_fw_cfg.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/firmware') diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c index f08e056ed0ae..b436342115af 100644 --- a/drivers/firmware/qemu_fw_cfg.c +++ b/drivers/firmware/qemu_fw_cfg.c @@ -389,6 +389,7 @@ static void fw_cfg_sysfs_cache_cleanup(void) list_for_each_entry_safe(entry, next, &fw_cfg_entry_cache, list) { fw_cfg_sysfs_cache_delist(entry); + kobject_del(&entry->kobj); kobject_put(&entry->kobj); } } -- cgit v1.2.3