netdev - Re: [PATCH net v2 3/4] net/sched: sch_qfq: account for stab overhead in qfq

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] [thread-next>] [day] [month] [year] [list]

Date: Sat, 8 Jul 2023 10:09:58 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Pedro Tammela <pctammela@...atatu.com>
Cc: netdev@...r.kernel.org, jhs@...atatu.com, xiyou.wangcong@...il.com, 
	jiri@...nulli.us, davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, 
	shuah@...nel.org, shaozhengchao@...wei.com, victor@...atatu.com, 
	simon.horman@...igine.com, paolo.valente@...more.it, Lion <nnamrec@...il.com>
Subject: Re: [PATCH net v2 3/4] net/sched: sch_qfq: account for stab overhead
 in qfq_enqueue

On Sat, Jul 8, 2023 at 12:01 AM Pedro Tammela <pctammela@...atatu.com> wrote:
>
> Lion says:
> -------
> In the QFQ scheduler a similar issue to CVE-2023-31436
> persists.
>
> Consider the following code in net/sched/sch_qfq.c:
>
> static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
>                 struct sk_buff **to_free)
> {
>      unsigned int len = qdisc_pkt_len(skb), gso_segs;
>
>     // ...
>
>      if (unlikely(cl->agg->lmax < len)) {
>          pr_debug("qfq: increasing maxpkt from %u to %u for class %u",
>               cl->agg->lmax, len, cl->common.classid);
>          err = qfq_change_agg(sch, cl, cl->agg->class_weight, len);
>          if (err) {
>              cl->qstats.drops++;
>              return qdisc_drop(skb, sch, to_free);
>          }
>
>     // ...
>
>      }
>

> This is caused by incorrectly assuming that qdisc_pkt_len() returns a
> length within the QFQ_MIN_LMAX < len < QFQ_MAX_LMAX.
>
> Fixes: 462dbc9101ac ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
> Reported-by: Lion <nnamrec@...il.com>
> Signed-off-by: Jamal Hadi Salim <jhs@...atatu.com>
> Signed-off-by: Pedro Tammela <pctammela@...atatu.com>
> ---
>  net/sched/sch_qfq.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
> index 63a5b277c117..befaf74b33ca 100644
> --- a/net/sched/sch_qfq.c
> +++ b/net/sched/sch_qfq.c
> @@ -381,8 +381,13 @@ static int qfq_change_agg(struct Qdisc *sch, struct qfq_class *cl, u32 weight,
>                            u32 lmax)
>  {
>         struct qfq_sched *q = qdisc_priv(sch);
> -       struct qfq_aggregate *new_agg = qfq_find_agg(q, lmax, weight);
> +       struct qfq_aggregate *new_agg;
>
> +       /* 'lmax' can range from [QFQ_MIN_LMAX, pktlen + stab overhead] */
> +       if (lmax > QFQ_MAX_LMAX)
> +               return -EINVAL;
> +
> +       new_agg = qfq_find_agg(q, lmax, weight);

Reviewed-by: Eric Dumazet <edumazet@...gle.com>