Skip to content

Commit 5093bbf

Browse files
jk-ozlabskuba-moo
authored andcommitted
mctp: perform route lookups under a RCU read-side lock
Our current route lookups (mctp_route_lookup and mctp_route_lookup_null) traverse the net's route list without the RCU read lock held. This means the route lookup is subject to preemption, resulting in an potential grace period expiry, and so an eventual kfree() while we still have the route pointer. Add the proper read-side critical section locks around the route lookups, preventing premption and a possible parallel kfree. The remaining net->mctp.routes accesses are already under a rcu_read_lock, or protected by the RTNL for updates. Based on an analysis from Sili Luo <[email protected]>, where introducing a delay in the route lookup could cause a UAF on simultaneous sendmsg() and route deletion. Reported-by: Sili Luo <[email protected]> Fixes: 889b7da ("mctp: Add initial routing framework") Cc: [email protected] Signed-off-by: Jeremy Kerr <[email protected]> Reviewed-by: Eric Dumazet <[email protected]> Link: https://lore.kernel.org/r/29c4b0e67dc1bf3571df3982de87df90cae9b631.1696837310.git.jk@codeconstruct.com.au Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 8527ca7 commit 5093bbf

File tree

1 file changed

+16
-6
lines changed

1 file changed

+16
-6
lines changed

net/mctp/route.c

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,8 @@ struct mctp_route *mctp_route_lookup(struct net *net, unsigned int dnet,
737737
{
738738
struct mctp_route *tmp, *rt = NULL;
739739

740+
rcu_read_lock();
741+
740742
list_for_each_entry_rcu(tmp, &net->mctp.routes, list) {
741743
/* TODO: add metrics */
742744
if (mctp_rt_match_eid(tmp, dnet, daddr)) {
@@ -747,21 +749,29 @@ struct mctp_route *mctp_route_lookup(struct net *net, unsigned int dnet,
747749
}
748750
}
749751

752+
rcu_read_unlock();
753+
750754
return rt;
751755
}
752756

753757
static struct mctp_route *mctp_route_lookup_null(struct net *net,
754758
struct net_device *dev)
755759
{
756-
struct mctp_route *rt;
760+
struct mctp_route *tmp, *rt = NULL;
757761

758-
list_for_each_entry_rcu(rt, &net->mctp.routes, list) {
759-
if (rt->dev->dev == dev && rt->type == RTN_LOCAL &&
760-
refcount_inc_not_zero(&rt->refs))
761-
return rt;
762+
rcu_read_lock();
763+
764+
list_for_each_entry_rcu(tmp, &net->mctp.routes, list) {
765+
if (tmp->dev->dev == dev && tmp->type == RTN_LOCAL &&
766+
refcount_inc_not_zero(&tmp->refs)) {
767+
rt = tmp;
768+
break;
769+
}
762770
}
763771

764-
return NULL;
772+
rcu_read_unlock();
773+
774+
return rt;
765775
}
766776

767777
static int mctp_do_fragment_route(struct mctp_route *rt, struct sk_buff *skb,

0 commit comments

Comments
 (0)