[PATCH] HSI: ssi_protocol: don't cancel_work_sync() under ssi->lock in ssip_reset

[PATCH] HSI: ssi_protocol: don't cancel_work_sync() under ssi->lock in ssip_reset

[Hyunjung Ko]

ssip_reset() calls cancel_work_sync(&ssi->work) inside the
spin_lock_bh(&ssi->lock) section. cancel_work_sync() calls might_sleep(),
so running it under the bh spinlock triggers "BUG: sleeping function called
from invalid context". In addition, the cancelled work ssip_xmit_work() ->
ssip_xmit() takes ssi->lock itself, so waiting for it while holding the
lock can deadlock.

Move cancel_work_sync() out of the locked section. It still runs in
ssip_reset() before teardown, so the use-after-free fix is preserved.

Fixes: e3f88665a780 ("HSI: ssi_protocol: Fix use after free vulnerability in ssi_protocol Driver Due to Race Condition")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 drivers/hsi/clients/ssi_protocol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hsi/clients/ssi_protocol.c b/drivers/hsi/clients/ssi_protocol.c
index 7c6ca8391a6b..96c20850fd66 100644
--- a/drivers/hsi/clients/ssi_protocol.c
+++ b/drivers/hsi/clients/ssi_protocol.c
@@ -397,11 +397,11 @@ static void ssip_reset(struct hsi_client *cl)
 	spin_unlock_bh(&ssi->lock);
 	if (test_and_clear_bit(SSIP_WAKETEST_FLAG, &ssi->flags))
 		ssi_waketest(cl, 0); /* FIXME: To be removed */
+	cancel_work_sync(&ssi->work);
 	spin_lock_bh(&ssi->lock);
 	timer_delete(&ssi->rx_wd);
 	timer_delete(&ssi->tx_wd);
 	timer_delete(&ssi->keep_alive);
-	cancel_work_sync(&ssi->work);
 	ssi->main_state = 0;
 	ssi->send_state = 0;
 	ssi->recv_state = 0;
-- 
2.43.0

Re: [PATCH] HSI: ssi_protocol: don't cancel_work_sync() under ssi->lock in ssip_reset

[Andy Shevchenko]

On Wed, Nov 11, 2026 at 06:04:47PM +0900, Hyunjung Ko wrote:
> ssip_reset() calls cancel_work_sync(&ssi->work) inside the
> spin_lock_bh(&ssi->lock) section. cancel_work_sync() calls might_sleep(),
> so running it under the bh spinlock triggers "BUG: sleeping function called
> from invalid context". In addition, the cancelled work ssip_xmit_work() ->
> ssip_xmit() takes ssi->lock itself, so waiting for it while holding the
> lock can deadlock.
> 
> Move cancel_work_sync() out of the locked section. It still runs in
> ssip_reset() before teardown, so the use-after-free fix is preserved.

But is it okay to run before timers are gone?

...

>  	spin_unlock_bh(&ssi->lock);
>  	if (test_and_clear_bit(SSIP_WAKETEST_FLAG, &ssi->flags))
>  		ssi_waketest(cl, 0); /* FIXME: To be removed */
> +	cancel_work_sync(&ssi->work);
>  	spin_lock_bh(&ssi->lock);
>  	timer_delete(&ssi->rx_wd);
>  	timer_delete(&ssi->tx_wd);
>  	timer_delete(&ssi->keep_alive);
> -	cancel_work_sync(&ssi->work);

-- 
With Best Regards,
Andy Shevchenko