This is a note to let you know that I've just added the patch titled
sch_netem: fix issues in netem_change() vs get_dist_table()
to the 4.19-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
sch_netem-fix-issues-in-netem_change-vs-get_dist_table.patch
and it can be found in the queue-4.19 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From 11b73313c12403f617b47752db0ab3deef201af7 Mon Sep 17 00:00:00 2001
From: Eric Dumazet <edumazet@xxxxxxxxxx>
Date: Thu, 22 Jun 2023 18:15:03 +0000
Subject: sch_netem: fix issues in netem_change() vs get_dist_table()
From: Eric Dumazet <edumazet@xxxxxxxxxx>
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: syzbot <syzkaller@xxxxxxxxxxxxxxxx>
Signed-off-by: Eric Dumazet <edumazet@xxxxxxxxxx>
Cc: Stephen Hemminger <stephen@xxxxxxxxxxxxxxxxxx>
Acked-by: Jamal Hadi Salim <jhs@xxxxxxxxxxxx>
Reviewed-by: Simon Horman <simon.horman@xxxxxxxxxxxx>
Link: https://lore.kernel.org/r/20230622181503.2327695-1-edumazet@xxxxxxxxxx
Signed-off-by: Jakub Kicinski <kuba@xxxxxxxxxx>
Signed-off-by: Fedor Pchelkin <pchelkin@xxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
net/sched/sch_netem.c | 59 +++++++++++++++++++++-----------------------------
1 file changed, 25 insertions(+), 34 deletions(-)
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -748,12 +748,10 @@ static void dist_free(struct disttable *
* 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 *
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 *sc
{
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 *sc
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 *sc
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 *sc
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,
Patches currently in stable-queue which might be from edumazet@xxxxxxxxxx are
queue-4.19/dccp-fix-data-race-around-dp-dccps_mss_cache.patch
queue-4.19/net-packet-annotate-data-races-around-tp-status.patch
queue-4.19/sch_netem-fix-issues-in-netem_change-vs-get_dist_table.patch
