Re: [PATCH net v2] strparser: make sure __strp_recv isn't running before tearing down the parser

[Sabrina Dubroca <sd@queasysnail.net>]

2026-10-02, 00:06:06 +0000, netdev-bot+sashiko@kernel.org wrote:
> [Severity: High]
> This is a pre-existing issue, but the new sync here depends on
> strp->stopped being set reliably. In KCM, that store looks like it can
> be lost.

I guess I'll add that just to make sashiko shut up.


> [Severity: Medium]
> Can this oops for a parser set up in general mode? strp_init() still
> accepts a NULL sk as long as cb->lock and cb->unlock are supplied, and
> stores it unchanged:

Yes, and the notes in the patch say that.

> All in-tree strp_init() callers pass a real socket: espintcp_init_sk(),
> kcm_attach(), sk_psock_init_strp() and ovpn_tcp_socket_attach(). None of
> them overrides cb.lock/cb.unlock, so nothing in-tree hits this today.
> 
> This is headed to stable, and the API and documentation still allow
> general mode. Would one of these be better?

No...


> [Severity: Low]
> This isn't a bug introduced by this patch, but the comment above
> strp_done() reads:
> 
> /* strp must already be stopped so that strp_recv will no longer be called.
>  * Note that strp_done is not called with the lower socket held.
>  */
> 
> With the new lock_sock() here, would "must not be called" be clearer
> than "is not called"?

Would anyone actually do that when the _3rd line_ in the function is
lock_sock()?


Something needs to be fixed for the last 2, and it's not the patch...

-- 
Sabrina