netdev - Re: [Patch net 1/4] pfifo_tail_enqueue: Drop new packet when sch->limit == 0

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: <Z51GaME2q0QVc+kB@pop-os.localdomain>
Date: Fri, 31 Jan 2025 13:53:44 -0800
From: Cong Wang <xiyou.wangcong@...il.com>
To: Jamal Hadi Salim <jhs@...atatu.com>
Cc: netdev@...r.kernel.org, jiri@...nulli.us, quanglex97@...il.com,
	mincho@...ori.io, Cong Wang <cong.wang@...edance.com>
Subject: Re: [Patch net 1/4] pfifo_tail_enqueue: Drop new packet when
 sch->limit == 0

On Thu, Jan 30, 2025 at 06:17:36AM -0500, Jamal Hadi Salim wrote:
> On Fri, Jan 24, 2025 at 1:07 AM Cong Wang <xiyou.wangcong@...il.com> wrote:
> >
> > From: Quang Le <quanglex97@...il.com>
> >
> > Expected behaviour:
> > In case we reach scheduler's limit, pfifo_tail_enqueue() will drop a
> > packet in scheduler's queue and decrease scheduler's qlen by one.
> > Then, pfifo_tail_enqueue() enqueue new packet and increase
> > scheduler's qlen by one. Finally, pfifo_tail_enqueue() return
> > `NET_XMIT_CN` status code.
> >
> > Weird behaviour:
> > In case we set `sch->limit == 0` and trigger pfifo_tail_enqueue() on a
> > scheduler that has no packet, the 'drop a packet' step will do nothing.
> > This means the scheduler's qlen still has value equal 0.
> > Then, we continue to enqueue new packet and increase scheduler's qlen by
> > one. In summary, we can leverage pfifo_tail_enqueue() to increase qlen by
> > one and return `NET_XMIT_CN` status code.
> >
> > The problem is:
> > Let's say we have two qdiscs: Qdisc_A and Qdisc_B.
> >  - Qdisc_A's type must have '->graft()' function to create parent/child relationship.
> >    Let's say Qdisc_A's type is `hfsc`. Enqueue packet to this qdisc will trigger `hfsc_enqueue`.
> >  - Qdisc_B's type is pfifo_head_drop. Enqueue packet to this qdisc will trigger `pfifo_tail_enqueue`.
> >  - Qdisc_B is configured to have `sch->limit == 0`.
> >  - Qdisc_A is configured to route the enqueued's packet to Qdisc_B.
> >
> > Enqueue packet through Qdisc_A will lead to:
> >  - hfsc_enqueue(Qdisc_A) -> pfifo_tail_enqueue(Qdisc_B)
> >  - Qdisc_B->q.qlen += 1
> >  - pfifo_tail_enqueue() return `NET_XMIT_CN`
> >  - hfsc_enqueue() check for `NET_XMIT_SUCCESS` and see `NET_XMIT_CN` => hfsc_enqueue() don't increase qlen of Qdisc_A.
> >
> > The whole process lead to a situation where Qdisc_A->q.qlen == 0 and Qdisc_B->q.qlen == 1.
> > Replace 'hfsc' with other type (for example: 'drr') still lead to the same problem.
> > This violate the design where parent's qlen should equal to the sum of its childrens'qlen.
> >
> > Bug impact: This issue can be used for user->kernel privilege escalation when it is reachable.
> >
> > Reported-by: Quang Le <quanglex97@...il.com>
> > Signed-off-by: Quang Le <quanglex97@...il.com>
> > Signed-off-by: Cong Wang <cong.wang@...edance.com>
> 
> Fixes: 57dbb2d83d100 ?

Ah, probably. I will add the Fixes tag in v3 after I resolve the
selftest issue.

Thanks!