netdev - Re: [PATCH 4.14,5.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] [day] [month] [year] [list]

Message-ID: <2023080103-zigzagged-morse-2a0f@gregkh>
Date: Tue, 1 Aug 2023 10:22:11 +0200
From: Greg KH <greg@...ah.com>
To: Shaoying Xu <shaoyi@...zon.com>
Cc: stable@...r.kernel.org, netdev@...r.kernel.org,
	Lion <nnamrec@...il.com>, Eric Dumazet <edumazet@...gle.com>,
	Jamal Hadi Salim <jhs@...atatu.com>,
	Pedro Tammela <pctammela@...atatu.com>,
	Simon Horman <simon.horman@...igine.com>,
	Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH 4.14,5.4] net/sched: sch_qfq: account for stab overhead
 in qfq_enqueue

On Thu, Jul 27, 2023 at 08:41:49PM +0000, Shaoying Xu wrote:
> [ Upstream commit 3e337087c3b5805fe0b8a46ba622a962880b5d64 ]
> 
> 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);
>          }
> 
>     // ...
> 
>      }
> 
> Similarly to CVE-2023-31436, "lmax" is increased without any bounds
> checks according to the packet length "len". Usually this would not
> impose a problem because packet sizes are naturally limited.
> 
> This is however not the actual packet length, rather the
> "qdisc_pkt_len(skb)" which might apply size transformations according to
> "struct qdisc_size_table" as created by "qdisc_get_stab()" in
> net/sched/sch_api.c if the TCA_STAB option was set when modifying the qdisc.
> 
> A user may choose virtually any size using such a table.
> 
> As a result the same issue as in CVE-2023-31436 can occur, allowing heap
> out-of-bounds read / writes in the kmalloc-8192 cache.
> -------
> 
> We can create the issue with the following commands:
> 
> tc qdisc add dev $DEV root handle 1: stab mtu 2048 tsize 512 mpu 0 \
> overhead 999999999 linklayer ethernet qfq
> tc class add dev $DEV parent 1: classid 1:1 htb rate 6mbit burst 15k
> tc filter add dev $DEV parent 1: matchall classid 1:1
> ping -I $DEV 1.1.1.2
> 
> 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>
> Reviewed-by: Eric Dumazet <edumazet@...gle.com>
> Signed-off-by: Jamal Hadi Salim <jhs@...atatu.com>
> Signed-off-by: Pedro Tammela <pctammela@...atatu.com>
> Reviewed-by: Simon Horman <simon.horman@...igine.com>
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> [Backport patch for stable kernels 4.14 and 5.4. Since QFQ_MAX_LMAX is not 
> defined, replace it with 1UL << QFQ_MTU_SHIFT.]
> Cc: <stable@...r.kernel.org> # 4.14, 5.4
> Signed-off-by: Shaoying Xu <shaoyi@...zon.com>
> 
> ---
>  net/sched/sch_qfq.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Now queued up, thanks.

greg k-h