[PATCH] drivers/firmware_loader: remove list entry before deallocation

[Michal Lach]

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@xxxxxxxxxxx>
---
 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