netdev - Re: [PATCH net v2 1/2] net/sched: Restrict conditions for adding duplicating netems to qdisc tree

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <sJSFrPAuQr53uFRQlRKAsEZNkDg1umtbxAlgqqhnuvTobg1qwtVa-0sR-IVhFYXdgYlBIZnSD6AKlI6BAzgCrBwo7lZ5AYzX_xhhCTP3P7o=@willsroot.io>
Date: Tue, 24 Jun 2025 05:36:34 +0000
From: William Liu <will@...lsroot.io>
To: netdev@...r.kernel.org
Cc: jhs@...atatu.com, xiyou.wangcong@...il.com, victor@...atatu.com, pctammela@...atatu.com, pabeni@...hat.com, kuba@...nel.org, stephen@...workplumber.org, dcaratti@...hat.com, savy@...t3mfailure.io, jiri@...nulli.us, davem@...emloft.net, edumazet@...gle.com, horms@...nel.org, William Liu <will@...lsroot.io>
Subject: Re: [PATCH net v2 1/2] net/sched: Restrict conditions for adding duplicating netems to qdisc tree






On Tuesday, June 24th, 2025 at 4:24 AM, William Liu <will@...lsroot.io> wrote:

> 
> 
> netem_enqueue's duplication prevention logic breaks when a netem
> resides in a qdisc tree with other netems - this can lead to a
> soft lockup and OOM loop in netem_dequeue as seen in [1].
> Ensure that a duplicating netem cannot exist in a tree with other
> netems.
> 
> [1] https://lore.kernel.org/netdev/8DuRWwfqjoRDLDmBMlIfbrsZg9Gx50DHJc1ilxsEBNe2D6NMoigR_eIRIG0LOjMc3r10nUUZtArXx4oZBIdUfZQrwjcQhdinnMis_0G7VEk=@willsroot.io/
> 
> Fixes: 0afb51e72855 ("[PKT_SCHED]: netem: reinsert for duplication")
> Reported-by: William Liu will@...lsroot.io
> 
> Reported-by: Savino Dicanosa savy@...t3mfailure.io
> 
> Signed-off-by: William Liu will@...lsroot.io
> 
> Signed-off-by: Savino Dicanosa savy@...t3mfailure.io
> 
> ---
> net/sched/sch_netem.c | 45 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
> 
> diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
> index fdd79d3ccd8c..be38458ae5bc 100644
> --- a/net/sched/sch_netem.c
> +++ b/net/sched/sch_netem.c
> @@ -973,6 +973,46 @@ static int parse_attr(struct nlattr *tb[], int maxtype, struct nlattr *nla,
> return 0;
> }
> 
> +static const struct Qdisc_class_ops netem_class_ops;
> +
> +static inline bool has_duplication(struct Qdisc *sch)
> +{
> + struct netem_sched_data *q = qdisc_priv(sch);
> +
> + return q->duplicate;
> 
> +}
> +
> +static int check_netem_in_tree(struct Qdisc *sch, bool duplicates,
> + struct netlink_ext_ack *extack)
> +{
> + struct Qdisc *root, *q;
> + unsigned int i;
> +
> + root = qdisc_root_sleeping(sch);
> +
> + if (sch != root && root->ops->cl_ops == &netem_class_ops) {
> 
> + if (duplicates || has_duplication(root))
> + goto err;
> + }
> +
> + if (!qdisc_dev(root))
> + return 0;
> +
> + hash_for_each(qdisc_dev(root)->qdisc_hash, i, q, hash) {
> 
> + if (sch != q && q->ops->cl_ops == &netem_class_ops) {
> 
> + if (duplicates || has_duplication(q))
> + goto err;
> + }
> + }
> +
> + return 0;
> +
> +err:
> + NL_SET_ERR_MSG(extack,
> + "netem: cannot mix duplicating netems with other netems in tree");
> + return -EINVAL;
> +}
> +
> /* Parse netlink message to set options */
> static int netem_change(struct Qdisc *sch, struct nlattr *opt,
> struct netlink_ext_ack *extack)
> @@ -1031,6 +1071,11 @@ 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.
> --
> 2.43.0

v2 was sent out right after some additional feedback from Cong. Please review v3 instead, which I will be posting shortly.

pw-bot: changes-requested

Thanks,
William