Commit 3b0d2819 authored by Paolo Abeni's avatar Paolo Abeni
Browse files

Merge branch 'net-sched-fix-race-conditions-in-mini_qdisc_pair_swap' 

Peilin Ye says:

====================
net/sched: Fix race conditions in mini_qdisc_pair_swap()

These 2 patches fix race conditions for ingress and clsact Qdiscs as
reported [1] by syzbot, split out from another [2] series (last 2 patches
of it).  Per-patch changelog omitted.

Patch 1 hasn't been touched since last version; I just included
everybody's tag.

Patch 2 bases on patch 6 v1 of [2], with comments and commit log slightly
changed.  We also need rtnl_dereference() to load ->qdisc_sleeping since
commit d636fc5d ("net: sched: add rcu annotations around
qdisc->qdisc_sleeping"), so I changed that; please take yet another look,
thanks!

Patch 2 has been tested with the new reproducer Pedro posted [3].

[1] https://syzkaller.appspot.com/bug?extid=b53a9c0d1ea4ad62da8b
[2] https://lore.kernel.org/r/cover.1684887977.git.peilin.ye@bytedance.com/
[3] https://lore.kernel.org/r/7879f218-c712-e9cc-57ba-665990f5f4c9@mojatatu.com/
====================

Link: https://lore.kernel.org/r/cover.1686355297.git.peilin.ye@bytedance.com


Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parents 41f2c7c3 84ad0af0

include/net/sch_generic.h

+8 −0
Original line number Diff line number Diff line
@@ -137,6 +137,13 @@ static inline void qdisc_refcount_inc(struct Qdisc *qdisc) 
	refcount_inc(&qdisc->refcnt);

}



static inline bool qdisc_refcount_dec_if_one(struct Qdisc *qdisc)

{

	if (qdisc->flags & TCQ_F_BUILTIN)

		return true;

	return refcount_dec_if_one(&qdisc->refcnt);

}



/* Intended to be used by unlocked users, when concurrent qdisc release is

 * possible.

 */
@@ -652,6 +659,7 @@ void dev_deactivate_many(struct list_head *head); 
struct Qdisc *dev_graft_qdisc(struct netdev_queue *dev_queue,

			      struct Qdisc *qdisc);

void qdisc_reset(struct Qdisc *qdisc);

void qdisc_destroy(struct Qdisc *qdisc);

void qdisc_put(struct Qdisc *qdisc);

void qdisc_put_unlocked(struct Qdisc *qdisc);

void qdisc_tree_reduce_backlog(struct Qdisc *qdisc, int n, int len);

net/sched/sch_api.c

+31 −13
Original line number Diff line number Diff line
@@ -1079,17 +1079,29 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent, 


	if (parent == NULL) {

		unsigned int i, num_q, ingress;

		struct netdev_queue *dev_queue;



		ingress = 0;

		num_q = dev->num_tx_queues;

		if ((q && q->flags & TCQ_F_INGRESS) ||

		    (new && new->flags & TCQ_F_INGRESS)) {

			num_q = 1;

			ingress = 1;

			if (!dev_ingress_queue(dev)) {

			dev_queue = dev_ingress_queue(dev);

			if (!dev_queue) {

				NL_SET_ERR_MSG(extack, "Device does not have an ingress queue");

				return -ENOENT;

			}



			q = rtnl_dereference(dev_queue->qdisc_sleeping);



			/* This is the counterpart of that qdisc_refcount_inc_nz() call in

			 * __tcf_qdisc_find() for filter requests.

			 */

			if (!qdisc_refcount_dec_if_one(q)) {

				NL_SET_ERR_MSG(extack,

					       "Current ingress or clsact Qdisc has ongoing filter requests");

				return -EBUSY;

			}

		}



		if (dev->flags & IFF_UP)
@@ -1100,19 +1112,27 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent, 
		if (new && new->ops->attach && !ingress)

			goto skip;



		if (!ingress) {

			for (i = 0; i < num_q; i++) {

			struct netdev_queue *dev_queue = dev_ingress_queue(dev);



			if (!ingress)

				dev_queue = netdev_get_tx_queue(dev, i);



				old = dev_graft_qdisc(dev_queue, new);



				if (new && i > 0)

					qdisc_refcount_inc(new);



			if (!ingress)

				qdisc_put(old);

			}

		} else {

			old = dev_graft_qdisc(dev_queue, NULL);



			/* {ingress,clsact}_destroy() @old before grafting @new to avoid

			 * unprotected concurrent accesses to net_device::miniq_{in,e}gress

			 * pointer(s) in mini_qdisc_pair_swap().

			 */

			qdisc_notify(net, skb, n, classid, old, new, extack);

			qdisc_destroy(old);



			dev_graft_qdisc(dev_queue, new);

		}



skip:

		if (!ingress) {
@@ -1125,8 +1145,6 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent, 


			if (new && new->ops->attach)

				new->ops->attach(new);

		} else {

			notify_and_destroy(net, skb, n, classid, old, new, extack);

		}



		if (dev->flags & IFF_UP)

net/sched/sch_generic.c

+11 −3
Original line number Diff line number Diff line
@@ -1046,7 +1046,7 @@ static void qdisc_free_cb(struct rcu_head *head) 
	qdisc_free(q);

}



static void qdisc_destroy(struct Qdisc *qdisc)

static void __qdisc_destroy(struct Qdisc *qdisc)

{

	const struct Qdisc_ops  *ops = qdisc->ops;
@@ -1070,6 +1070,14 @@ static void qdisc_destroy(struct Qdisc *qdisc) 
	call_rcu(&qdisc->rcu, qdisc_free_cb);

}



void qdisc_destroy(struct Qdisc *qdisc)

{

	if (qdisc->flags & TCQ_F_BUILTIN)

		return;



	__qdisc_destroy(qdisc);

}



void qdisc_put(struct Qdisc *qdisc)

{

	if (!qdisc)
@@ -1079,7 +1087,7 @@ void qdisc_put(struct Qdisc *qdisc) 
	    !refcount_dec_and_test(&qdisc->refcnt))

		return;



	qdisc_destroy(qdisc);

	__qdisc_destroy(qdisc);

}

EXPORT_SYMBOL(qdisc_put);
@@ -1094,7 +1102,7 @@ void qdisc_put_unlocked(struct Qdisc *qdisc) 
	    !refcount_dec_and_rtnl_lock(&qdisc->refcnt))

		return;



	qdisc_destroy(qdisc);

	__qdisc_destroy(qdisc);

	rtnl_unlock();

}

EXPORT_SYMBOL(qdisc_put_unlocked);