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

[Danilo Krummrich]

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 write 0
>> to the loading attribute and complete the request before it is queued as
>> pending.
>> 
>> In that interleaving firmware_loading_store() calls fw_state_done() while
>> 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.
>> 
>> 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 only
>> active fallback requests.
>> 
>> Fixes: 75d95e2e39b2 ("firmware_loader: fix use-after-free in firmware_fallback_sysfs")
>> Signed-off-by: Mukesh Ojha <mukesh.ojha@xxxxxxxxxxxxxxxx>
>
> 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(+)
>> 
>> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_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 = -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);
>>  
>> -- 
>> 2.53.0
>> 
>
> -- 
> -Mukesh Ojha