Messages in this thread Patch in this message | | | From | Michal Lach <> | Subject | [PATCH] drivers/firmware_loader: remove list entry before deallocation | Date | Wed, 23 Nov 2022 12:14:56 +0100 |
| |
If CONFIG_FW_LOADER_USER_HELPER is enabled, it is possible to interrupt the loading process after adding pending_list to pending_fw_list. Subsequently, if user calls release_firmware() which deallocates the fw_priv structure which pending_list is a member of, the entry in the list is not deleted. This causes a use-after-free on further attempts to add an entry to the list or on list traversal.
While not problematic in most drivers since this function is mainly used in probe or init routines, some drivers expose firmware loading functionality via user-accessible functions like write() etc. In this case during the sysfs loading process, we can send SIGKILL to the thread which is then in kernel, leave the entry in the list and then free the structure.
Example kernel panics with CONFIG_DEBUG_LIST turned on:
kernel BUG at lib/list_debug.c:25! /* ... */ Call trace: __list_add_valid+0x7c/0x98 fw_load_sysfs_fallback+0xd4/0x334 fw_load_from_user_helper+0x148/0x1f8 firmware_fallback_sysfs+0xe0/0x17c _request_firmware+0x1a0/0x470 request_firmware+0x50/0x78 /* ... */
or
kernel BUG at lib/list_debug.c:56! /* ... */ Call trace: __list_del_entry_valid+0xa0/0xa4 fw_load_abort+0x38/0x64 fw_load_sysfs_fallback+0x354/0x468 fw_load_from_user_helper+0x17c/0x1c0 firmware_fallback_sysfs+0xc0/0x11c _request_firmware+0xe0/0x4a4 request_firmware+0x20/0x2c /* ... */
Fixes: fe304143b0c3 ("firmware: Avoid deadlock of usermodehelper lock at shutdown") Signed-off-by: Michal Lach <michal.lach@samsung.com> --- drivers/base/firmware_loader/main.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 7c3590fd97c2..381997c84e4f 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -209,6 +209,10 @@ static void __free_fw_priv(struct kref *ref) __func__, fw_priv->fw_name, fw_priv, fw_priv->data, (unsigned int)fw_priv->size); +#ifdef CONFIG_FW_LOADER_USER_HELPER + list_del(&fw_priv->pending_list); +#endif + list_del(&fw_priv->list); spin_unlock(&fwc->lock); -- 2.25.1
| |