Re: [PATCH net] can: raw: fix ro->uniq use-after-free in raw_rcv()

[Sam P <sam@bynar.io>]

On 08/04/2026 17:28, Oliver Hartkopp wrote:
> Hello Sam,
> 
> many thanks for your investigation and for the provided fix.
> Excellent work!
> 
> Btw. you also suggested a different solution with synchronize_rcu():
> 
> diff --git a/net/can/raw.c b/net/can/raw.c
> index eee244ffc31e..5bb9a84f2471 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -431,6 +431,13 @@ static int raw_release(struct socket *sock)
>       if (ro->count > 1)
>           kfree(ro->filter);
> 
> +    /*
> +     * Wait for any in-flight raw_rcv() calls to finish before freeing
> +     * ro->uniq.  can_rx_unregister() scheduled deletion via call_rcu(),
> +     * but RCU readers (raw_rcv in softirq) may still be active.
> +     */
> +    synchronize_rcu();
> +
>       ro->ifindex = 0;
>       ro->bound = 0;
>       ro->dev = NULL;
> 
> 
> Can you tell why you preferred the destructor solution now?

Thank you :) I preferred the destructor solution as it seemed to match the socket lifetime model better and I wasn't sure if the blocking sync in the raw_release() was too heavy-handed for this specific issue, given raw_release() already holds rtnl_lock() and lock_sock(sk). That said, I'm happy to defer to your experience if the sync fix is better suited, I have tested both of them.

> And if I see it correctly the UAF problem might also show up with the
> kfree(ro->filter) statement we can see at the beginning of the above patch.
> 
> So either free_percpu(ro->uniq) and kfree(ro->filter) should be handled after the finalized synchronize_rcu() process, right?

ro->filter isn't accessed in the racey raw_rcv() path as far as I can tell, and I don't *think* there are other racey paths but it wouldn't hurt to handle it just in-case. I think this would be simple with the synchronize_rcu() patch, as you mentioned, but I'm not sure with the destructor.

Kind Regards,
Sam