Commit d636fc5d authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller
Browse files

net: sched: add rcu annotations around qdisc->qdisc_sleeping 



syzbot reported a race around qdisc->qdisc_sleeping [1]

It is time we add proper annotations to reads and writes to/from
qdisc->qdisc_sleeping.

[1]
BUG: KCSAN: data-race in dev_graft_qdisc / qdisc_lookup_rcu

read to 0xffff8881286fc618 of 8 bytes by task 6928 on cpu 1:
qdisc_lookup_rcu+0x192/0x2c0 net/sched/sch_api.c:331
__tcf_qdisc_find+0x74/0x3c0 net/sched/cls_api.c:1174
tc_get_tfilter+0x18f/0x990 net/sched/cls_api.c:2547
rtnetlink_rcv_msg+0x7af/0x8c0 net/core/rtnetlink.c:6386
netlink_rcv_skb+0x126/0x220 net/netlink/af_netlink.c:2546
rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:6413
netlink_unicast_kernel net/netlink/af_netlink.c:1339 [inline]
netlink_unicast+0x56f/0x640 net/netlink/af_netlink.c:1365
netlink_sendmsg+0x665/0x770 net/netlink/af_netlink.c:1913
sock_sendmsg_nosec net/socket.c:724 [inline]
sock_sendmsg net/socket.c:747 [inline]
____sys_sendmsg+0x375/0x4c0 net/socket.c:2503
___sys_sendmsg net/socket.c:2557 [inline]
__sys_sendmsg+0x1e3/0x270 net/socket.c:2586
__do_sys_sendmsg net/socket.c:2595 [inline]
__se_sys_sendmsg net/socket.c:2593 [inline]
__x64_sys_sendmsg+0x46/0x50 net/socket.c:2593
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd

write to 0xffff8881286fc618 of 8 bytes by task 6912 on cpu 0:
dev_graft_qdisc+0x4f/0x80 net/sched/sch_generic.c:1115
qdisc_graft+0x7d0/0xb60 net/sched/sch_api.c:1103
tc_modify_qdisc+0x712/0xf10 net/sched/sch_api.c:1693
rtnetlink_rcv_msg+0x807/0x8c0 net/core/rtnetlink.c:6395
netlink_rcv_skb+0x126/0x220 net/netlink/af_netlink.c:2546
rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:6413
netlink_unicast_kernel net/netlink/af_netlink.c:1339 [inline]
netlink_unicast+0x56f/0x640 net/netlink/af_netlink.c:1365
netlink_sendmsg+0x665/0x770 net/netlink/af_netlink.c:1913
sock_sendmsg_nosec net/socket.c:724 [inline]
sock_sendmsg net/socket.c:747 [inline]
____sys_sendmsg+0x375/0x4c0 net/socket.c:2503
___sys_sendmsg net/socket.c:2557 [inline]
__sys_sendmsg+0x1e3/0x270 net/socket.c:2586
__do_sys_sendmsg net/socket.c:2595 [inline]
__se_sys_sendmsg net/socket.c:2593 [inline]
__x64_sys_sendmsg+0x46/0x50 net/socket.c:2593
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd

Reported by Kernel Concurrency Sanitizer on:
CPU: 0 PID: 6912 Comm: syz-executor.5 Not tainted 6.4.0-rc3-syzkaller-00190-g0d85b27b0cc6 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/16/2023

Fixes: 3a7d0d07 ("net: sched: extend Qdisc with rcu")
Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Cc: Vlad Buslov <vladbu@nvidia.com>
Acked-by: default avatarJamal Hadi <Salim&lt;jhs@mojatatu.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent e3144ff5

include/linux/netdevice.h

+1 −1
Original line number Diff line number Diff line
@@ -620,7 +620,7 @@ struct netdev_queue { 
	netdevice_tracker	dev_tracker;



	struct Qdisc __rcu	*qdisc;

	struct Qdisc		*qdisc_sleeping;

	struct Qdisc __rcu	*qdisc_sleeping;

#ifdef CONFIG_SYSFS

	struct kobject		kobj;

#endif

include/net/sch_generic.h

+4 −2
Original line number Diff line number Diff line
@@ -545,7 +545,7 @@ static inline struct Qdisc *qdisc_root_bh(const struct Qdisc *qdisc) 


static inline struct Qdisc *qdisc_root_sleeping(const struct Qdisc *qdisc)

{

	return qdisc->dev_queue->qdisc_sleeping;

	return rcu_dereference_rtnl(qdisc->dev_queue->qdisc_sleeping);

}



static inline spinlock_t *qdisc_root_sleeping_lock(const struct Qdisc *qdisc)
@@ -754,7 +754,9 @@ static inline bool qdisc_tx_changing(const struct net_device *dev) 


	for (i = 0; i < dev->num_tx_queues; i++) {

		struct netdev_queue *txq = netdev_get_tx_queue(dev, i);

		if (rcu_access_pointer(txq->qdisc) != txq->qdisc_sleeping)



		if (rcu_access_pointer(txq->qdisc) !=

		    rcu_access_pointer(txq->qdisc_sleeping))

			return true;

	}

	return false;

net/core/dev.c

+1 −1
Original line number Diff line number Diff line
@@ -10543,7 +10543,7 @@ struct netdev_queue *dev_ingress_queue_create(struct net_device *dev) 
		return NULL;

	netdev_init_one_queue(dev, queue, NULL);

	RCU_INIT_POINTER(queue->qdisc, &noop_qdisc);

	queue->qdisc_sleeping = &noop_qdisc;

	RCU_INIT_POINTER(queue->qdisc_sleeping, &noop_qdisc);

	rcu_assign_pointer(dev->ingress_queue, queue);

#endif

	return queue;

net/sched/sch_api.c

+16 −10
Original line number Diff line number Diff line
@@ -309,7 +309,7 @@ struct Qdisc *qdisc_lookup(struct net_device *dev, u32 handle) 


	if (dev_ingress_queue(dev))

		q = qdisc_match_from_root(

			dev_ingress_queue(dev)->qdisc_sleeping,

			rtnl_dereference(dev_ingress_queue(dev)->qdisc_sleeping),

			handle);

out:

	return q;
@@ -328,7 +328,8 @@ struct Qdisc *qdisc_lookup_rcu(struct net_device *dev, u32 handle) 


	nq = dev_ingress_queue_rcu(dev);

	if (nq)

		q = qdisc_match_from_root(nq->qdisc_sleeping, handle);

		q = qdisc_match_from_root(rcu_dereference(nq->qdisc_sleeping),

					  handle);

out:

	return q;

}
@@ -634,8 +635,13 @@ EXPORT_SYMBOL(qdisc_watchdog_init); 
void qdisc_watchdog_schedule_range_ns(struct qdisc_watchdog *wd, u64 expires,

				      u64 delta_ns)

{

	if (test_bit(__QDISC_STATE_DEACTIVATED,

		     &qdisc_root_sleeping(wd->qdisc)->state))

	bool deactivated;



	rcu_read_lock();

	deactivated = test_bit(__QDISC_STATE_DEACTIVATED,

			       &qdisc_root_sleeping(wd->qdisc)->state);

	rcu_read_unlock();

	if (deactivated)

		return;



	if (hrtimer_is_queued(&wd->timer)) {
@@ -1478,7 +1484,7 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n, 
				}

				q = qdisc_leaf(p, clid);

			} else if (dev_ingress_queue(dev)) {

				q = dev_ingress_queue(dev)->qdisc_sleeping;

				q = rtnl_dereference(dev_ingress_queue(dev)->qdisc_sleeping);

			}

		} else {

			q = rtnl_dereference(dev->qdisc);
@@ -1564,7 +1570,7 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n, 
				}

				q = qdisc_leaf(p, clid);

			} else if (dev_ingress_queue_create(dev)) {

				q = dev_ingress_queue(dev)->qdisc_sleeping;

				q = rtnl_dereference(dev_ingress_queue(dev)->qdisc_sleeping);

			}

		} else {

			q = rtnl_dereference(dev->qdisc);
@@ -1805,8 +1811,8 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb) 


		dev_queue = dev_ingress_queue(dev);

		if (dev_queue &&

		    tc_dump_qdisc_root(dev_queue->qdisc_sleeping, skb, cb,

				       &q_idx, s_q_idx, false,

		    tc_dump_qdisc_root(rtnl_dereference(dev_queue->qdisc_sleeping),

				       skb, cb, &q_idx, s_q_idx, false,

				       tca[TCA_DUMP_INVISIBLE]) < 0)

			goto done;
@@ -2249,8 +2255,8 @@ static int tc_dump_tclass(struct sk_buff *skb, struct netlink_callback *cb) 


	dev_queue = dev_ingress_queue(dev);

	if (dev_queue &&

	    tc_dump_tclass_root(dev_queue->qdisc_sleeping, skb, tcm, cb,

				&t, s_t, false) < 0)

	    tc_dump_tclass_root(rtnl_dereference(dev_queue->qdisc_sleeping),

				skb, tcm, cb, &t, s_t, false) < 0)

		goto done;



done:

net/sched/sch_fq_pie.c

+2 −0
Original line number Diff line number Diff line
@@ -379,6 +379,7 @@ static void fq_pie_timer(struct timer_list *t) 
	spinlock_t *root_lock; /* to lock qdisc for probability calculations */

	u32 idx;



	rcu_read_lock();

	root_lock = qdisc_lock(qdisc_root_sleeping(sch));

	spin_lock(root_lock);
@@ -391,6 +392,7 @@ static void fq_pie_timer(struct timer_list *t) 
		mod_timer(&q->adapt_timer, jiffies + q->p_params.tupdate);



	spin_unlock(root_lock);

	rcu_read_unlock();

}



static int fq_pie_init(struct Qdisc *sch, struct nlattr *opt,