drivers/hsi/clients/ssi_protocol.c | 1 + 1 file changed, 1 insertion(+)
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@m.fudan.edu.cn>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2:
- cancel the work in ssip_reset(), suggested by Sebastian
- add the Acked-by label from Andy
- Link to v1: https://lore.kernel.org/r/20240911151915.844957-1-kxwang23@m.fudan.edu.cn
---
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..6105ea9a6c6a 100644
--- a/drivers/hsi/clients/ssi_protocol.c
+++ b/drivers/hsi/clients/ssi_protocol.c
@@ -401,6 +401,7 @@ static void ssip_reset(struct hsi_client *cl)
del_timer(&ssi->rx_wd);
del_timer(&ssi->tx_wd);
del_timer(&ssi->keep_alive);
+ cancel_work_sync(&ssi->work);
ssi->main_state = 0;
ssi->send_state = 0;
ssi->recv_state = 0;
--
2.39.1.windows.1On Sun, Sep 15, 2024 at 01:21:43AM +0800, 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. We refer to the functions as func(). E.g., ssip_pn_setup(), ssip_pn_xmit(). > 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. Same here. ... > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > - add the Acked-by label from Andy Not that Ack was given to _this_ version of the change (it has been changed a lot), but I'm fine with keeping it. -- With Best Regards, Andy Shevchenko
At 2024-09-16 18:39:46, "Andy Shevchenko" <andriy.shevchenko@linux.intel.com> wrote: >On Sun, Sep 15, 2024 at 01:21:43AM +0800, 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. > >We refer to the functions as func(). E.g., ssip_pn_setup(), >ssip_pn_xmit(). > I will correct it. >> 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. > >Same here. > I will correct it. >... > >> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > >> - add the Acked-by label from Andy > >Not that Ack was given to _this_ version of the change (it has been changed >a lot), but I'm fine with keeping it. > Sorry, I missed this point. >-- >With Best Regards, >Andy Shevchenko > > > Best regards, Kaixin Wang
© 2016 - 2024 Red Hat, Inc.