Re: [PATCH iwl-net 1/3] idpf: fix memory leaks and crashes while performing a soft reset

[Alexander Lobakin]

From: Simon Horman <horms@xxxxxxxxxx>
Date: Fri, 26 Jul 2024 17:09:54 +0100

> On Wed, Jul 24, 2024 at 03:40:22PM +0200, Alexander Lobakin wrote:
>> The second tagged commit introduced a UAF, as it removed restoring
>> q_vector->vport pointers after reinitializating the structures.
>> This is due to that all queue allocation functions are performed here
>> with the new temporary vport structure and those functions rewrite
>> the backpointers to the vport. Then, this new struct is freed and
>> the pointers start leading to nowhere.

[...]

>>  err_reset:
>> -	idpf_vport_queues_rel(new_vport);
>> +	idpf_send_add_queues_msg(vport, vport->num_txq, vport->num_complq,
>> +				 vport->num_rxq, vport->num_bufq);
>> +
>> +err_open:
>> +	if (current_state == __IDPF_VPORT_UP)
>> +		idpf_vport_open(vport);
> 
> Hi Alexander,
> 
> Can the system end up in an odd state if this call to idpf_vport_open(), or
> the one above, fails. Likewise if the above call to
> idpf_send_add_queues_msg() fails.

Adding the queues with the parameters that were before changing them
almost can't fail. But if any of these two fails, it really will be in
an odd state...

Perhaps we need to do a more powerful reset then? Can we somehow tell
the kernel that in fact our iface is down, so that the user could try
to enable it manually once again?
Anyway, feels like a separate series or patch to -next, what do you think?

> 
>> +
>>  free_vport:
>>  	kfree(new_vport);

Thanks,
Olek