The memory allocated in ipc_protocol_init() is not freed on the error
paths that follow in ipc_imem_init(). Fix that by calling the
corresponding release function ipc_protocol_deinit() in the error path.

Fixes: 3670970dd8c6 ("net: iosm: shared memory IPC interface")
Cc: stable@vger.kernel.org
Signed-off-by: Abdun Nihaal <nihaal@cse.iitm.ac.in>
---
Compile tested only. Issue found using static analysis.

 drivers/net/wwan/iosm/iosm_ipc_imem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem.c b/drivers/net/wwan/iosm/iosm_ipc_imem.c
index 1b7bc7d63a2e..f4edb277efd9 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_imem.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem.c
@@ -1422,6 +1422,7 @@ struct iosm_imem *ipc_imem_init(struct iosm_pcie *pcie, unsigned int device_id,
 	hrtimer_cancel(&ipc_imem->fast_update_timer);
 	hrtimer_cancel(&ipc_imem->tdupdate_timer);
 	hrtimer_cancel(&ipc_imem->startup_timer);
+	ipc_protocol_deinit(ipc_imem->ipc_protocol);
 protocol_init_fail:
 	cancel_work_sync(&ipc_imem->run_state_worker);
 	ipc_task_deinit(ipc_imem->ipc_task);
-- 
2.43.0

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: wwan: iosm: fix potential memory leaks in ipc_imem_init()

The memory allocated in ipc_protocol_init() is not freed on the error
paths that follow in ipc_imem_init(). Fix that by calling the
corresponding release function ipc_protocol_deinit() in the error path.

> diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem.c b/drivers/net/wwan/iosm/iosm_ipc_imem.c
> index 1b7bc7d63a2e8..f4edb277efd92 100644
> --- a/drivers/net/wwan/iosm/iosm_ipc_imem.c
> +++ b/drivers/net/wwan/iosm/iosm_ipc_imem.c
> @@ -1422,6 +1422,7 @@ struct iosm_imem *ipc_imem_init(struct iosm_pcie *pcie, unsigned int device_id,
>  	hrtimer_cancel(&ipc_imem->fast_update_timer);
>  	hrtimer_cancel(&ipc_imem->tdupdate_timer);
>  	hrtimer_cancel(&ipc_imem->startup_timer);
> +	ipc_protocol_deinit(ipc_imem->ipc_protocol);
>  protocol_init_fail:
>  	cancel_work_sync(&ipc_imem->run_state_worker);
>  	ipc_task_deinit(ipc_imem->ipc_task);

Could this introduce a use-after-free by destroying the protocol before
canceling the work and tasklet?

If an error occurs after the timers are started (for example, if devlink
registration fails), the hrtimer cancellation stops new timer events.
However, if the startup timer has already fired, it might have queued a
task into the ipc_task queue.

Calling ipc_protocol_deinit() here frees the ipc_protocol structure.
Since ipc_task_deinit() has not yet been called to flush the queue and
kill the tasklet, any pending tasklet may still execute.

If a queued task such as ipc_imem_tq_startup_timer_cb() runs, does it
access the freed ipc_protocol?

ipc_imem_tq_startup_timer_cb()
  ipc_imem_phase_update()
    ipc_imem_get_exec_stage_buffered()
      ipc_protocol_get_ap_exec_stage(ipc_imem->ipc_protocol)

Would it be safer to place the ipc_protocol_deinit() call after the
tasklet and worker are fully destroyed?
-- 
pw-bot: cr

On Sun, May 10, 2026 at 10:04:51AM -0700, Jakub Kicinski wrote:
> > diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem.c b/drivers/net/wwan/iosm/iosm_ipc_imem.c
> > index 1b7bc7d63a2e8..f4edb277efd92 100644
> > --- a/drivers/net/wwan/iosm/iosm_ipc_imem.c
> > +++ b/drivers/net/wwan/iosm/iosm_ipc_imem.c
> > @@ -1422,6 +1422,7 @@ struct iosm_imem *ipc_imem_init(struct iosm_pcie *pcie, unsigned int device_id,
> >  	hrtimer_cancel(&ipc_imem->fast_update_timer);
> >  	hrtimer_cancel(&ipc_imem->tdupdate_timer);
> >  	hrtimer_cancel(&ipc_imem->startup_timer);
> > +	ipc_protocol_deinit(ipc_imem->ipc_protocol);
> >  protocol_init_fail:
> >  	cancel_work_sync(&ipc_imem->run_state_worker);
> >  	ipc_task_deinit(ipc_imem->ipc_task);

> Calling ipc_protocol_deinit() here frees the ipc_protocol structure.
> Since ipc_task_deinit() has not yet been called to flush the queue and
> kill the tasklet, any pending tasklet may still execute.
> 
> Would it be safer to place the ipc_protocol_deinit() call after the
> tasklet and worker are fully destroyed?

Thanks for reviewing the patch. I agree that this change may introduce a
use after free since we are freeing the ipc_protocol while tasklets and
workers are running concurrently. I'll fix it and send a v2 patch.

The same UAF bug seems to exist in the ipc_imem_cleanup() function.
Will send a patch for that as well.

Regards,
Nihaal