Commit afd8c2c9 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'ipv6-f6i-fib6_siblings-and-rt-fib6_nsiblings-fixes'

Eric Dumazet says:

====================
ipv6: f6i->fib6_siblings and rt->fib6_nsiblings fixes

Series based on an internal syzbot report with a repro.

After fixing (in the first patch) the original minor issue,
I found that syzbot repro was able to trigger a second
more serious bug in rt6_nlmsg_size().

Code review then led to the two final patches.

I have not released the syzbot bug, because other issues
still need investigations.
====================

Link: https://patch.msgid.link/20250725140725.3626540-1-edumazet@google.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents f388f807 31d7d67b
Loading
Loading
Loading
Loading
+15 −9
Original line number Diff line number Diff line
@@ -445,15 +445,17 @@ struct fib6_dump_arg {
static int fib6_rt_dump(struct fib6_info *rt, struct fib6_dump_arg *arg)
{
	enum fib_event_type fib_event = FIB_EVENT_ENTRY_REPLACE;
	unsigned int nsiblings;
	int err;

	if (!rt || rt == arg->net->ipv6.fib6_null_entry)
		return 0;

	if (rt->fib6_nsiblings)
	nsiblings = READ_ONCE(rt->fib6_nsiblings);
	if (nsiblings)
		err = call_fib6_multipath_entry_notifier(arg->nb, fib_event,
							 rt,
							 rt->fib6_nsiblings,
							 nsiblings,
							 arg->extack);
	else
		err = call_fib6_entry_notifier(arg->nb, fib_event, rt,
@@ -1138,7 +1140,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,

			if (rt6_duplicate_nexthop(iter, rt)) {
				if (rt->fib6_nsiblings)
					rt->fib6_nsiblings = 0;
					WRITE_ONCE(rt->fib6_nsiblings, 0);
				if (!(iter->fib6_flags & RTF_EXPIRES))
					return -EEXIST;
				if (!(rt->fib6_flags & RTF_EXPIRES)) {
@@ -1167,7 +1169,8 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
			 */
			if (rt_can_ecmp &&
			    rt6_qualify_for_ecmp(iter))
				rt->fib6_nsiblings++;
				WRITE_ONCE(rt->fib6_nsiblings,
					   rt->fib6_nsiblings + 1);
		}

		if (iter->fib6_metric > rt->fib6_metric)
@@ -1217,7 +1220,8 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
		fib6_nsiblings = 0;
		list_for_each_entry_safe(sibling, temp_sibling,
					 &rt->fib6_siblings, fib6_siblings) {
			sibling->fib6_nsiblings++;
			WRITE_ONCE(sibling->fib6_nsiblings,
				   sibling->fib6_nsiblings + 1);
			BUG_ON(sibling->fib6_nsiblings != rt->fib6_nsiblings);
			fib6_nsiblings++;
		}
@@ -1264,8 +1268,9 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
				list_for_each_entry_safe(sibling, next_sibling,
							 &rt->fib6_siblings,
							 fib6_siblings)
					sibling->fib6_nsiblings--;
				rt->fib6_nsiblings = 0;
					WRITE_ONCE(sibling->fib6_nsiblings,
						   sibling->fib6_nsiblings - 1);
				WRITE_ONCE(rt->fib6_nsiblings, 0);
				list_del_rcu(&rt->fib6_siblings);
				rcu_read_lock();
				rt6_multipath_rebalance(next_sibling);
@@ -2014,8 +2019,9 @@ static void fib6_del_route(struct fib6_table *table, struct fib6_node *fn,
			notify_del = true;
		list_for_each_entry_safe(sibling, next_sibling,
					 &rt->fib6_siblings, fib6_siblings)
			sibling->fib6_nsiblings--;
		rt->fib6_nsiblings = 0;
			WRITE_ONCE(sibling->fib6_nsiblings,
				   sibling->fib6_nsiblings - 1);
		WRITE_ONCE(rt->fib6_nsiblings, 0);
		list_del_rcu(&rt->fib6_siblings);
		rt6_multipath_rebalance(next_sibling);
	}
+42 −29
Original line number Diff line number Diff line
@@ -5346,7 +5346,8 @@ static void ip6_route_mpath_notify(struct fib6_info *rt,
	 */
	rcu_read_lock();

	if ((nlflags & NLM_F_APPEND) && rt_last && rt_last->fib6_nsiblings) {
	if ((nlflags & NLM_F_APPEND) && rt_last &&
	    READ_ONCE(rt_last->fib6_nsiblings)) {
		rt = list_first_or_null_rcu(&rt_last->fib6_siblings,
					    struct fib6_info,
					    fib6_siblings);
@@ -5670,32 +5671,34 @@ static int rt6_nh_nlmsg_size(struct fib6_nh *nh, void *arg)

static size_t rt6_nlmsg_size(struct fib6_info *f6i)
{
	struct fib6_info *sibling;
	struct fib6_nh *nh;
	int nexthop_len;

	if (f6i->nh) {
		nexthop_len = nla_total_size(4); /* RTA_NH_ID */
		nexthop_for_each_fib6_nh(f6i->nh, rt6_nh_nlmsg_size,
					 &nexthop_len);
	} else {
		struct fib6_nh *nh = f6i->fib6_nh;
		struct fib6_info *sibling;
		goto common;
	}

	rcu_read_lock();
retry:
	nh = f6i->fib6_nh;
	nexthop_len = 0;
		if (f6i->fib6_nsiblings) {
	if (READ_ONCE(f6i->fib6_nsiblings)) {
		rt6_nh_nlmsg_size(nh, &nexthop_len);

			rcu_read_lock();

		list_for_each_entry_rcu(sibling, &f6i->fib6_siblings,
					fib6_siblings) {
			rt6_nh_nlmsg_size(sibling->fib6_nh, &nexthop_len);
			if (!READ_ONCE(f6i->fib6_nsiblings))
				goto retry;
		}

			rcu_read_unlock();
	}
	rcu_read_unlock();
	nexthop_len += lwtunnel_get_encap_size(nh->fib_nh_lws);
	}

common:
	return NLMSG_ALIGN(sizeof(struct rtmsg))
	       + nla_total_size(16) /* RTA_SRC */
	       + nla_total_size(16) /* RTA_DST */
@@ -5854,7 +5857,7 @@ static int rt6_fill_node(struct net *net, struct sk_buff *skb,
		if (dst->lwtstate &&
		    lwtunnel_fill_encap(skb, dst->lwtstate, RTA_ENCAP, RTA_ENCAP_TYPE) < 0)
			goto nla_put_failure;
	} else if (rt->fib6_nsiblings) {
	} else if (READ_ONCE(rt->fib6_nsiblings)) {
		struct fib6_info *sibling;
		struct nlattr *mp;

@@ -5956,16 +5959,21 @@ static bool fib6_info_uses_dev(const struct fib6_info *f6i,
	if (f6i->fib6_nh->fib_nh_dev == dev)
		return true;

	if (f6i->fib6_nsiblings) {
		struct fib6_info *sibling, *next_sibling;
	if (READ_ONCE(f6i->fib6_nsiblings)) {
		const struct fib6_info *sibling;

		list_for_each_entry_safe(sibling, next_sibling,
					 &f6i->fib6_siblings, fib6_siblings) {
			if (sibling->fib6_nh->fib_nh_dev == dev)
		rcu_read_lock();
		list_for_each_entry_rcu(sibling, &f6i->fib6_siblings,
					fib6_siblings) {
			if (sibling->fib6_nh->fib_nh_dev == dev) {
				rcu_read_unlock();
				return true;
			}
			if (!READ_ONCE(f6i->fib6_nsiblings))
				break;
		}
		rcu_read_unlock();
	}

	return false;
}

@@ -6321,8 +6329,9 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
void inet6_rt_notify(int event, struct fib6_info *rt, struct nl_info *info,
		     unsigned int nlm_flags)
{
	struct sk_buff *skb;
	struct net *net = info->nl_net;
	struct sk_buff *skb;
	size_t sz;
	u32 seq;
	int err;

@@ -6330,17 +6339,21 @@ void inet6_rt_notify(int event, struct fib6_info *rt, struct nl_info *info,
	seq = info->nlh ? info->nlh->nlmsg_seq : 0;

	rcu_read_lock();

	skb = nlmsg_new(rt6_nlmsg_size(rt), GFP_ATOMIC);
	sz = rt6_nlmsg_size(rt);
retry:
	skb = nlmsg_new(sz, GFP_ATOMIC);
	if (!skb)
		goto errout;

	err = rt6_fill_node(net, skb, rt, NULL, NULL, NULL, 0,
			    event, info->portid, seq, nlm_flags);
	if (err < 0) {
		/* -EMSGSIZE implies BUG in rt6_nlmsg_size() */
		WARN_ON(err == -EMSGSIZE);
		kfree_skb(skb);
		/* -EMSGSIZE implies needed space grew under us. */
		if (err == -EMSGSIZE) {
			sz = max(rt6_nlmsg_size(rt), sz << 1);
			goto retry;
		}
		goto errout;
	}