Messages in this thread | | | Date | Tue, 29 Nov 2022 11:06:57 -0800 | Subject | Re: [PATCH] drivers/firmware_loader: remove list entry before deallocation | From | Russ Weight <> |
| |
On 11/23/22 03:14, Michal Lach wrote: > 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")
Reviewed-by: Russ Weight <russell.h.weight@intel.com>
> 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); >
| |