Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 595500b2 authored by Eric Dumazet's avatar Eric Dumazet Committed by Greg Kroah-Hartman
Browse files

sch_netem: fix issues in netem_change() vs get_dist_table() 



commit 11b73313c12403f617b47752db0ab3deef201af7 upstream.

In blamed commit, I missed that get_dist_table() was allocating
memory using GFP_KERNEL, and acquiring qdisc lock to perform
the swap of newly allocated table with current one.

In this patch, get_dist_table() is allocating memory and
copy user data before we acquire the qdisc lock.

Then we perform swap operations while being protected by the lock.

Note that after this patch netem_change() no longer can do partial changes.
If an error is returned, qdisc conf is left unchanged.

Fixes: 2174a08db80d ("sch_netem: acquire qdisc lock in netem_change()")
Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: default avatarSimon Horman <simon.horman@corigine.com>
Link: https://lore.kernel.org/r/20230622181503.2327695-1-edumazet@google.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
Signed-off-by: default avatarFedor Pchelkin <pchelkin@ispras.ru>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent ff9c28ef

net/sched/sch_netem.c

+25 −34
Original line number Diff line number Diff line
@@ -748,12 +748,10 @@ static void dist_free(struct disttable *d) 
 * signed 16 bit values.

 */



static int get_dist_table(struct Qdisc *sch, struct disttable **tbl,

			  const struct nlattr *attr)

static int get_dist_table(struct disttable **tbl, const struct nlattr *attr)

{

	size_t n = nla_len(attr)/sizeof(__s16);

	const __s16 *data = nla_data(attr);

	spinlock_t *root_lock;

	struct disttable *d;

	int i;
@@ -768,13 +766,7 @@ static int get_dist_table(struct Qdisc *sch, struct disttable **tbl, 
	for (i = 0; i < n; i++)

		d->table[i] = data[i];



	root_lock = qdisc_root_sleeping_lock(sch);



	spin_lock_bh(root_lock);

	swap(*tbl, d);

	spin_unlock_bh(root_lock);



	dist_free(d);

	*tbl = d;

	return 0;

}
@@ -930,6 +922,8 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt, 
{

	struct netem_sched_data *q = qdisc_priv(sch);

	struct nlattr *tb[TCA_NETEM_MAX + 1];

	struct disttable *delay_dist = NULL;

	struct disttable *slot_dist = NULL;

	struct tc_netem_qopt *qopt;

	struct clgstate old_clg;

	int old_loss_model = CLG_RANDOM;
@@ -943,6 +937,18 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt, 
	if (ret < 0)

		return ret;



	if (tb[TCA_NETEM_DELAY_DIST]) {

		ret = get_dist_table(&delay_dist, tb[TCA_NETEM_DELAY_DIST]);

		if (ret)

			goto table_free;

	}



	if (tb[TCA_NETEM_SLOT_DIST]) {

		ret = get_dist_table(&slot_dist, tb[TCA_NETEM_SLOT_DIST]);

		if (ret)

			goto table_free;

	}



	sch_tree_lock(sch);

	/* backup q->clg and q->loss_model */

	old_clg = q->clg;
@@ -952,26 +958,17 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt, 
		ret = get_loss_clg(q, tb[TCA_NETEM_LOSS]);

		if (ret) {

			q->loss_model = old_loss_model;

			q->clg = old_clg;

			goto unlock;

		}

	} else {

		q->loss_model = CLG_RANDOM;

	}



	if (tb[TCA_NETEM_DELAY_DIST]) {

		ret = get_dist_table(sch, &q->delay_dist,

				     tb[TCA_NETEM_DELAY_DIST]);

		if (ret)

			goto get_table_failure;

	}



	if (tb[TCA_NETEM_SLOT_DIST]) {

		ret = get_dist_table(sch, &q->slot_dist,

				     tb[TCA_NETEM_SLOT_DIST]);

		if (ret)

			goto get_table_failure;

	}



	if (delay_dist)

		swap(q->delay_dist, delay_dist);

	if (slot_dist)

		swap(q->slot_dist, slot_dist);

	sch->limit = qopt->limit;



	q->latency = PSCHED_TICKS2NS(qopt->latency);
@@ -1021,17 +1018,11 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt, 


unlock:

	sch_tree_unlock(sch);

	return ret;



get_table_failure:

	/* recover clg and loss_model, in case of

	 * q->clg and q->loss_model were modified

	 * in get_loss_clg()

	 */

	q->clg = old_clg;

	q->loss_model = old_loss_model;



	goto unlock;

table_free:

	dist_free(delay_dist);

	dist_free(slot_dist);

	return ret;

}



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