sch_netem: fix issues in netem_change() vs get_dist_table()
commit11b73313c1
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:2174a08db8
("sch_netem: acquire qdisc lock in netem_change()") Reported-by: syzbot <syzkaller@googlegroups.com> Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Reviewed-by: Simon Horman <simon.horman@corigine.com> Link: https://lore.kernel.org/r/20230622181503.2327695-1-edumazet@google.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
3ae919c317
commit
4346a66ad1
|
@ -773,12 +773,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;
|
||||
|
||||
|
@ -793,13 +791,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;
|
||||
}
|
||||
|
||||
|
@ -956,6 +948,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;
|
||||
|
@ -966,6 +960,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;
|
||||
|
@ -975,26 +981,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);
|
||||
|
@ -1044,17 +1041,11 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt,
|
|||
|
||||
unlock:
|
||||
sch_tree_unlock(sch);
|
||||
|
||||
table_free:
|
||||
dist_free(delay_dist);
|
||||
dist_free(slot_dist);
|
||||
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;
|
||||
}
|
||||
|
||||
static int netem_init(struct Qdisc *sch, struct nlattr *opt,
|
||||
|
|
Loading…
Reference in New Issue