| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20251124223749.503979-1-victor@mojatatu.com>
Date: Mon, 24 Nov 2025 19:37:49 -0300
From: Victor Nogueira <victor@...atatu.com>
To: davem@...emloft.net,
kuba@...nel.org,
edumazet@...gle.com,
pabeni@...hat.com,
jhs@...atatu.com,
jiri@...nulli.us,
xiyou.wangcong@...il.com
Cc: netdev@...r.kernel.org
Subject: [RFC PATCH net-next] net/sched: Introduce qdisc quirk_chk op
There is a pattern of bugs that end up creating UAFs or null ptr derefs.
The majority of these bugs follow the formula below:
a) create a nonsense hierarchy of qdiscs which has no practical value,
b) start sending packets
Optional c) netlink cmds to change hierarchy some more; It's more fun if
you can get packets stuck - the formula in this case includes non
work-conserving qdiscs somewhere in the hierarchy
Optional d dependent on c) send more packets
e) profit
Current init/change qdisc APIs are localised to validate only within the
constraint of a single qdisc. So catching #a or #c is a challenge. Our
policy, when said bugs are presented, is to "make it work" by modifying
generally used data structures and code, but these come at the expense of
adding special checks for corner cases which are nonsensical to begin with.
The goal of this patchset is to create an equivalent to PCI quirks, which
will catch nonsensical hierarchies in #a and #c and reject such a config.
With that in mind, we are proposing the addition of a new qdisc op
(quirk_chk). We introduce, as a first example, the quirk_chk op to netem.
Its purpose here is to validate whether the user is attempting to add 2
netem duplicates in the same qdisc tree which will be forbidden unless
the root qdisc is multiqueue.
Here is an example that should now work:
DEV="eth0"
NUM_QUEUES=4
DUPLICATE_PERCENT="5%"
tc qdisc del dev $DEV root > /dev/null 2>&1
tc qdisc add dev $DEV root handle 1: mq
for i in $(seq 1 $NUM_QUEUES); do
HANDLE_ID=$((i * 10))
PARENT_ID="1:$i"
tc qdisc add dev $DEV parent $PARENT_ID handle \
${HANDLE_ID}: netem duplicate $DUPLICATE_PERCENT
done
Suggested-by: Jamal Hadi Salim <jhs@...atatu.com>
Signed-off-by: Victor Nogueira <victor@...atatu.com>
---
include/net/sch_generic.h | 3 +++
net/sched/sch_api.c | 12 ++++++++++
net/sched/sch_netem.c | 47 +++++++++++++++++++++++++++++----------
3 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 738cd5b13c62..405a6af22d8e 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -316,6 +316,9 @@ struct Qdisc_ops {
u32 block_index);
void (*egress_block_set)(struct Qdisc *sch,
u32 block_index);
+ int (*quirk_chk)(struct Qdisc *sch,
+ struct nlattr *arg,
+ struct netlink_ext_ack *extack);
u32 (*ingress_block_get)(struct Qdisc *sch);
u32 (*egress_block_get)(struct Qdisc *sch);
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index f56b18c8aebf..18b385d11587 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1315,6 +1315,12 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
rcu_assign_pointer(sch->stab, stab);
}
+ if (ops->quirk_chk) {
+ err = ops->quirk_chk(sch, tca[TCA_OPTIONS], extack);
+ if (err != 0)
+ goto err_out3;
+ }
+
if (ops->init) {
err = ops->init(sch, tca[TCA_OPTIONS], extack);
if (err != 0)
@@ -1378,6 +1384,12 @@ static int qdisc_change(struct Qdisc *sch, struct nlattr **tca,
NL_SET_ERR_MSG(extack, "Change of blocks is not supported");
return -EOPNOTSUPP;
}
+ if (sch->ops->quirk_chk) {
+ err = sch->ops->quirk_chk(sch, tca[TCA_OPTIONS],
+ extack);
+ if (err)
+ return err;
+ }
err = sch->ops->change(sch, tca[TCA_OPTIONS], extack);
if (err)
return err;
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index eafc316ae319..feed43900e0f 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -975,13 +975,32 @@ static int parse_attr(struct nlattr *tb[], int maxtype, struct nlattr *nla,
static const struct Qdisc_class_ops netem_class_ops;
-static int check_netem_in_tree(struct Qdisc *sch, bool duplicates,
- struct netlink_ext_ack *extack)
+static struct Qdisc *get_qdisc_root(struct Qdisc *sch)
{
+ struct net_device *dev;
+
+ dev = sch->dev_queue->dev;
+ if (netif_is_multiqueue(dev))
+ return rtnl_dereference(dev->qdisc);
+
+ return qdisc_root_sleeping(sch);
+}
+
+static int netem_quirk_chk(struct Qdisc *sch, struct nlattr *opt,
+ struct netlink_ext_ack *extack)
+{
+ struct tc_netem_qopt *qopt;
struct Qdisc *root, *q;
+ bool root_is_mq;
+ bool duplicates;
unsigned int i;
- root = qdisc_root_sleeping(sch);
+ qopt = nla_data(opt);
+ duplicates = qopt->duplicate;
+
+ root = get_qdisc_root(sch);
+ if (!root)
+ return 0;
if (sch != root && root->ops->cl_ops == &netem_class_ops) {
if (duplicates ||
@@ -992,19 +1011,27 @@ static int check_netem_in_tree(struct Qdisc *sch, bool duplicates,
if (!qdisc_dev(root))
return 0;
+ root_is_mq = root->flags & TCQ_F_MQROOT;
+
hash_for_each(qdisc_dev(root)->qdisc_hash, i, q, hash) {
if (sch != q && q->ops->cl_ops == &netem_class_ops) {
if (duplicates ||
- ((struct netem_sched_data *)qdisc_priv(q))->duplicate)
- goto err;
+ ((struct netem_sched_data *)qdisc_priv(q))->duplicate) {
+ struct Qdisc *parent;
+
+ parent = qdisc_lookup(qdisc_dev(q),
+ TC_H_MAJ(q->parent));
+ if (!root_is_mq || parent != root)
+ goto err;
+ }
}
}
return 0;
err:
- NL_SET_ERR_MSG(extack,
- "netem: cannot mix duplicating netems with other netems in tree");
+ NL_SET_ERR_MSG_MOD(extack,
+ "cannot mix duplicating netems with other netems in tree unless root is multiqueue");
return -EINVAL;
}
@@ -1066,11 +1093,6 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt,
q->gap = qopt->gap;
q->counter = 0;
q->loss = qopt->loss;
-
- ret = check_netem_in_tree(sch, qopt->duplicate, extack);
- if (ret)
- goto unlock;
-
q->duplicate = qopt->duplicate;
/* for compatibility with earlier versions.
@@ -1352,6 +1374,7 @@ static struct Qdisc_ops netem_qdisc_ops __read_mostly = {
.destroy = netem_destroy,
.change = netem_change,
.dump = netem_dump,
+ .quirk_chk = netem_quirk_chk,
.owner = THIS_MODULE,
};
MODULE_ALIAS_NET_SCH("netem");
--
2.51.0
Powered by blists - more mailing lists