Re: [PATCH] firmware_loader: do not queue completed sysfs fallback requests

"Danilo Krummrich" <[email protected]> Mon, 03 Aug 2026 20:40:06 +0200
Newsgroups dev.linux.lists.driver-core,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On Mon Aug 3, 2026 at 8:12 PM CEST, Mukesh Ojha wrote:
> On Thu, Jul 16, 2026 at 01:46:01PM +0530, Mukesh Ojha wrote:
>> fw_load_sysfs_fallback() calls device_add() before adding the fw_priv to
>> pending_fw_head. device_add() publishes the fallback loading interface, =
so
>> a userspace helper which discovers the device by scanning sysfs can writ=
e 0
>> to the loading attribute and complete the request before it is queued as
>> pending.
>>=20
>> In that interleaving firmware_loading_store() calls fw_state_done() whil=
e
>> pending_list still points to itself, so it cannot remove an entry from
>> pending_fw_head. The subsequent unconditional list_add() then queues an
>> already-completed fw_priv. Once the request is released, pending_fw_head
>> can retain a pointer to freed memory and the next fallback request can
>> fault while validating the list.
>>=20
>> Only in-flight fallback requests need suspend or reboot abort handling. =
If
>> the request is already DONE after device_add(), return success from the
>> fallback path without sending another uevent, waiting again, or queueing=
 it
>> as pending. This preserves the invariant that pending_fw_head contains o=
nly
>> active fallback requests.
>>=20
>> Fixes: 75d95e2e39b2 ("firmware_loader: fix use-after-free in firmware_fa=
llback_sysfs")
>> Signed-off-by: Mukesh Ojha <[email protected]>
>
> Can we consider this fix for this mentioned issue ?

Sure, how did you come across this issue?

>> ---
>>  drivers/base/firmware_loader/fallback.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>=20
>> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firm=
ware_loader/fallback.c
>> index 3ef0b312ae71..ffe1b3784788 100644
>> --- a/drivers/base/firmware_loader/fallback.c
>> +++ b/drivers/base/firmware_loader/fallback.c
>> @@ -95,6 +95,15 @@ static int fw_load_sysfs_fallback(struct fw_sysfs *fw=
_sysfs, long timeout)
>>  		retval =3D -EINTR;
>>  		goto out;
>>  	}
>> +	/*
>> +	 * device_add() exposes the loading interface before pending_list is
>> +	 * linked into pending_fw_head, so fw_state_done() may run first.
>> +	 */
>> +	if (fw_state_is_done(fw_priv)) {
>> +		mutex_unlock(&fw_lock);
>> +		goto out;
>> +	}
>> +
>>  	list_add(&fw_priv->pending_list, &pending_fw_head);
>>  	mutex_unlock(&fw_lock);
>> =20
>> --=20
>> 2.53.0
>>=20
>
> --=20
> -Mukesh Ojha