Re: [PATCH mptcp-next v3 5/5] Squash to "mptcp: pm: init and release mptcp_pm_ops"

[email protected]
Newsgroups dev.linux.lists.mptcp,dev.linux.lists.sashiko-reviews
Message-ID <[email protected]>
August 20, 2026 at 7:06 PM, "Matthieu Baerts" <[email protected] mailto:[email protected]?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:


> 
> On 20/08/2026 12:16, [email protected] wrote:
> 
> > 
> > August 20, 2026 at 5:07 PM, "Matthieu Baerts" <[email protected] mailto:[email protected]?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:
> >  
> > 
> > > 
> > > Can you first check if this can happen? I thought that
> > >  mptcp_destroy_common would first close all subflows, remove the token,
> > >  then release everything linked to the PM → so the PM will no longer be
> > >  called at that point for this msk, right? (I didn't check)
> > > 
> >  
> >  Hi Matt,
> >  
> >  Thans for looking at this and kindly remind.
> >  Yes, I just checked the order you described is right, but I thought it
> >  could happen in such order:
> >  
> >  CPU A (softirq) CPU B(disconnect)
> >  ->subflow_token_join_request 
> >  ->mptcp_token_get_sock()
> >  ->mptcp_destroy_common()
> >  ->pm_ops_release
> >  ->mptcp_pm_get_local_id():
> >  msk->pm.ops->get_local_id() <- NULL deref
> > 
> Ah yes, indeed, subflow_token_join_request doesn't hold the msk lock.
> 
> > 
> > WDYT? And, I think the window seems too narrow, we may not see it in practice,
> >  maybe a NULL check is enough?
> > 
> Would it be safe to have a NULL check? Could you have something like this?
> 
> CPU A CPU B
> -> if (ops)
>  -> mptcp_destroy_common()
>  -> pm_ops_release()
> -> msk->pm.ops->get_local_id()
> 
> The BPF module might have been unloaded.
Hi Matt

Yes, sorry for the silly mistake.

I’ll check if RCU can help us address this problem, and update the patch.

Thanks
Gang

> 
> (Again, I didn't check in details, only looking at your reply.)
> 
> Cheers,
> Matt
> -- 
> Sponsored by the NGI0 Core fund.
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.