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

From: Sam P

Date: Wed Apr 08 2026 - 13:23:25 EST


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