Skip to content

Commit

Permalink
firmware: fix theoretical UAF race with firmware cache and resume
Browse files Browse the repository at this point in the history
[ Upstream commit 3ecc8cb ]

This race was discovered when I carefully analyzed the code to locate
another firmware-related UAF issue. It can be triggered only when the
firmware load operation is executed during suspend. This possibility is
almost impossible because there are few firmware load and suspend actions
in the actual environment.

		CPU0			CPU1
__device_uncache_fw_images():		assign_fw():
					fw_cache_piggyback_on_request()
					<----- P0
	spin_lock(&fwc->name_lock);
	...
	list_del(&fce->list);
	spin_unlock(&fwc->name_lock);

	uncache_firmware(fce->name);
					<----- P1
					kref_get(&fw_priv->ref);

If CPU1 is interrupted at position P0, the new 'fce' has been added to the
list fwc->fw_names by the fw_cache_piggyback_on_request(). In this case,
CPU0 executes __device_uncache_fw_images() and will be able to see it when
it traverses list fwc->fw_names. Before CPU1 executes kref_get() at P1, if
CPU0 further executes uncache_firmware(), the count of fw_priv->ref may
decrease to 0, causing fw_priv to be released in advance.

Move kref_get() to the lock protection range of fwc->name_lock to fix it.

Fixes: ac39b3e ("firmware loader: let caching firmware piggyback on loading firmware")
Acked-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
Link: https://lore.kernel.org/r/20210719064531.3733-2-thunder.leizhen@huawei.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
Zhen Lei authored and gregkh committed Sep 15, 2021
1 parent c4aaad8 commit 4225d35
Showing 1 changed file with 8 additions and 12 deletions.
20 changes: 8 additions & 12 deletions drivers/base/firmware_loader/main.c
Expand Up @@ -164,7 +164,7 @@ static inline int fw_state_wait(struct fw_priv *fw_priv)
return __fw_state_wait_common(fw_priv, MAX_SCHEDULE_TIMEOUT);
}

static int fw_cache_piggyback_on_request(const char *name);
static void fw_cache_piggyback_on_request(struct fw_priv *fw_priv);

static struct fw_priv *__allocate_fw_priv(const char *fw_name,
struct firmware_cache *fwc,
Expand Down Expand Up @@ -705,10 +705,8 @@ int assign_fw(struct firmware *fw, struct device *device)
* on request firmware.
*/
if (!(fw_priv->opt_flags & FW_OPT_NOCACHE) &&
fw_priv->fwc->state == FW_LOADER_START_CACHE) {
if (fw_cache_piggyback_on_request(fw_priv->fw_name))
kref_get(&fw_priv->ref);
}
fw_priv->fwc->state == FW_LOADER_START_CACHE)
fw_cache_piggyback_on_request(fw_priv);

/* pass the pages buffer to driver at the last minute */
fw_set_page_data(fw_priv, fw);
Expand Down Expand Up @@ -1257,25 +1255,24 @@ static int __fw_entry_found(const char *name)
return 0;
}

static int fw_cache_piggyback_on_request(const char *name)
static void fw_cache_piggyback_on_request(struct fw_priv *fw_priv)
{
struct firmware_cache *fwc = &fw_cache;
const char *name = fw_priv->fw_name;
struct firmware_cache *fwc = fw_priv->fwc;
struct fw_cache_entry *fce;
int ret = 0;

spin_lock(&fwc->name_lock);
if (__fw_entry_found(name))
goto found;

fce = alloc_fw_cache_entry(name);
if (fce) {
ret = 1;
list_add(&fce->list, &fwc->fw_names);
kref_get(&fw_priv->ref);
pr_debug("%s: fw: %s\n", __func__, name);
}
found:
spin_unlock(&fwc->name_lock);
return ret;
}

static void free_fw_cache_entry(struct fw_cache_entry *fce)
Expand Down Expand Up @@ -1506,9 +1503,8 @@ static inline void unregister_fw_pm_ops(void)
unregister_pm_notifier(&fw_cache.pm_notify);
}
#else
static int fw_cache_piggyback_on_request(const char *name)
static void fw_cache_piggyback_on_request(struct fw_priv *fw_priv)
{
return 0;
}
static inline int register_fw_pm_ops(void)
{
Expand Down

0 comments on commit 4225d35

Please sign in to comment.