Re: [PATCH] HSI: ssi_protocol: Fix use after free vulnerability in ssi_protocol Driver Due to Race Condition

[Kaixin Wang]

At 2024-09-14 15:20:07, "Sebastian Reichel" <sre@xxxxxxxxxx> wrote:
>Hi,
>
>On Wed, Sep 11, 2024 at 11:19:15PM GMT, Kaixin Wang wrote:
>> In the ssi_protocol_probe function, &ssi->work is bound with
>> ssip_xmit_work, In ssip_pn_setup, the ssip_pn_xmit function
>> within the ssip_pn_ops structure is capable of starting the
>> work.
>> 
>> If we remove the module which will call ssi_protocol_remove
>> to make a cleanup, it will free ssi through kfree(ssi),
>> while the work mentioned above will be used. The sequence
>> of operations that may lead to a UAF bug is as follows:
>> 
>> CPU0                                    CPU1
>> 
>>                         | ssip_xmit_work
>> ssi_protocol_remove     |
>> kfree(ssi);             |
>>                         | struct hsi_client *cl = ssi->cl;
>>                         | // use ssi
>> 
>> Fix it by ensuring that the work is canceled before proceeding
>> with the cleanup in ssi_protocol_remove
>> 
>> Signed-off-by: Kaixin Wang <kxwang23@xxxxxxxxxxxxxx>
>> ---
>
>This does not even compile :(
>

Sorry for my mistake.

>During module removal the network device is unregistered (and thus
>stopped), which calls ssip_reset(), which should stop any activity
>regarding traffic exchange. That's the right place to cancel the
>work.
>
>Greetings,
>
>-- Sebastian
>

Thanks. I will fix it in the next version.

Best regards,
Kaixin Wang

>>  drivers/hsi/clients/ssi_protocol.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/drivers/hsi/clients/ssi_protocol.c b/drivers/hsi/clients/ssi_protocol.c
>> index afe470f3661c..3506c70e3505 100644
>> --- a/drivers/hsi/clients/ssi_protocol.c
>> +++ b/drivers/hsi/clients/ssi_protocol.c
>> @@ -1155,6 +1155,7 @@ static int ssi_protocol_remove(struct device *dev)
>>  	unregister_netdev(ssi->netdev);
>>  	ssip_free_cmds(ssi);
>>  	hsi_client_set_drvdata(cl, NULL);
>> +	cancel_work_sync(&ssi->work)
>>  	kfree(ssi);
>>  
>>  	return 0;
>> -- 
>> 2.25.1
>> 
>>