Skip to content

Commit 886b7a5

Browse files
iamkafaidavem330
authored andcommitted
ipv6: A few fixes on dereferencing rt->from
It is a followup after the fix in commit 9c69a13 ("route: Avoid crash from dereferencing NULL rt->from") rt6_do_redirect(): 1. NULL checking is needed on rt->from because a parallel fib6_info delete could happen that sets rt->from to NULL. (e.g. rt6_remove_exception() and fib6_drop_pcpu_from()). 2. fib6_info_hold() is not enough. Same reason as (1). Meaning, holding dst->__refcnt cannot ensure rt->from is not NULL or rt->from->fib6_ref is not 0. Instead of using fib6_info_hold_safe() which ip6_rt_cache_alloc() is already doing, this patch chooses to extend the rcu section to keep "from" dereference-able after checking for NULL. inet6_rtm_getroute(): 1. NULL checking is also needed on rt->from for a similar reason. Note that inet6_rtm_getroute() is using RTNL_FLAG_DOIT_UNLOCKED. Fixes: a68886a ("net/ipv6: Make from in rt6_info rcu protected") Signed-off-by: Martin KaFai Lau <[email protected]> Acked-by: Wei Wang <[email protected]> Reviewed-by: David Ahern <[email protected]> Reviewed-by: Eric Dumazet <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent f350574 commit 886b7a5

File tree

1 file changed

+18
-20
lines changed

1 file changed

+18
-20
lines changed

net/ipv6/route.c

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3392,11 +3392,8 @@ static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu
33923392

33933393
rcu_read_lock();
33943394
from = rcu_dereference(rt->from);
3395-
/* This fib6_info_hold() is safe here because we hold reference to rt
3396-
* and rt already holds reference to fib6_info.
3397-
*/
3398-
fib6_info_hold(from);
3399-
rcu_read_unlock();
3395+
if (!from)
3396+
goto out;
34003397

34013398
nrt = ip6_rt_cache_alloc(from, &msg->dest, NULL);
34023399
if (!nrt)
@@ -3408,10 +3405,7 @@ static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu
34083405

34093406
nrt->rt6i_gateway = *(struct in6_addr *)neigh->primary_key;
34103407

3411-
/* No need to remove rt from the exception table if rt is
3412-
* a cached route because rt6_insert_exception() will
3413-
* takes care of it
3414-
*/
3408+
/* rt6_insert_exception() will take care of duplicated exceptions */
34153409
if (rt6_insert_exception(nrt, from)) {
34163410
dst_release_immediate(&nrt->dst);
34173411
goto out;
@@ -3424,7 +3418,7 @@ static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu
34243418
call_netevent_notifiers(NETEVENT_REDIRECT, &netevent);
34253419

34263420
out:
3427-
fib6_info_release(from);
3421+
rcu_read_unlock();
34283422
neigh_release(neigh);
34293423
}
34303424

@@ -5023,16 +5017,20 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
50235017

50245018
rcu_read_lock();
50255019
from = rcu_dereference(rt->from);
5026-
5027-
if (fibmatch)
5028-
err = rt6_fill_node(net, skb, from, NULL, NULL, NULL, iif,
5029-
RTM_NEWROUTE, NETLINK_CB(in_skb).portid,
5030-
nlh->nlmsg_seq, 0);
5031-
else
5032-
err = rt6_fill_node(net, skb, from, dst, &fl6.daddr,
5033-
&fl6.saddr, iif, RTM_NEWROUTE,
5034-
NETLINK_CB(in_skb).portid, nlh->nlmsg_seq,
5035-
0);
5020+
if (from) {
5021+
if (fibmatch)
5022+
err = rt6_fill_node(net, skb, from, NULL, NULL, NULL,
5023+
iif, RTM_NEWROUTE,
5024+
NETLINK_CB(in_skb).portid,
5025+
nlh->nlmsg_seq, 0);
5026+
else
5027+
err = rt6_fill_node(net, skb, from, dst, &fl6.daddr,
5028+
&fl6.saddr, iif, RTM_NEWROUTE,
5029+
NETLINK_CB(in_skb).portid,
5030+
nlh->nlmsg_seq, 0);
5031+
} else {
5032+
err = -ENETUNREACH;
5033+
}
50365034
rcu_read_unlock();
50375035

50385036
if (err < 0) {

0 commit comments

Comments
 (0)