[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANn89iJqs_C267y=2RQdWvdvoLdpoqaB7EjgnxPw3cn77gpG3w@mail.gmail.com>
Date: Wed, 4 Dec 2024 08:48:05 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Octavian Purdila <tavip@...gle.com>
Cc: jhs@...atatu.com, xiyou.wangcong@...il.com, jiri@...nulli.us,
davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, horms@...nel.org,
shuah@...nel.org, netdev@...r.kernel.org, syzbot <syzkaller@...glegroups.com>
Subject: Re: [PATCH net-next 1/2] net_sched: sch_sfq: don't allow 1 packet limit
On Wed, Dec 4, 2024 at 4:06 AM Octavian Purdila <tavip@...gle.com> wrote:
>
> The current implementation does not work correctly with a limit of
> 1. iproute2 actually checks for this and this patch adds the check in
> kernel as well.
>
> This fixes the following syzkaller reported crash:
>
> The crash can be also be reproduced with the following (with a tc
> recompiled to allow for sfq limits of 1):
>
> tc qdisc add dev dummy0 handle 1: root tbf rate 1Kbit burst 100b lat 1s
> ../iproute2-6.9.0/tc/tc qdisc add dev dummy0 handle 2: parent 1:10 sfq limit 1
> ifconfig dummy0 up
> ping -I dummy0 -f -c2 -W0.1 8.8.8.8
> sleep 1
>
> Scenario that triggers the crash:
>
> * the first packet is sent and queued in TBF and SFQ; qdisc qlen is 1
>
> * TBF dequeues: it peeks from SFQ which moves the packet to the
> gso_skb list and keeps qdisc qlen set to 1. TBF is out of tokens so
> it schedules itself for later.
>
> * the second packet is sent and TBF tries to queues it to SFQ. qdisc
> qlen is now 2 and because the SFQ limit is 1 the packet is dropped
> by SFQ. At this point qlen is 1, and all of the SFQ slots are empty,
> however q->tail is not NULL.
>
> At this point, assuming no more packets are queued, when sch_dequeue
> runs again it will decrement the qlen for the current empty slot
> causing an underflow and the subsequent out of bounds access.
>
> Reported-by: syzbot <syzkaller@...glegroups.com>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Octavian Purdila <tavip@...gle.com>
Reviewed-by: Eric Dumazet <edumazet@...gle.com>
Powered by blists - more mailing lists