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