netdev - Re: [PATCH net v2 0/5] net_sched: Adapt qdiscs for reentrant enqueue cases

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]

Message-ID: <0fc72ff2-c13a-4da4-941a-36b4c945a015@mojatatu.com>
Date: Thu, 17 Apr 2025 18:49:51 -0300
From: Victor Nogueira <victor@...atatu.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, jhs@...atatu.com, xiyou.wangcong@...il.com,
 jiri@...nulli.us, davem@...emloft.net, edumazet@...gle.com,
 pabeni@...hat.com, toke@...hat.com, gerrard.tai@...rlabs.sg,
 pctammela@...atatu.com, Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH net v2 0/5] net_sched: Adapt qdiscs for reentrant enqueue
 cases

On 4/17/25 13:07, Jakub Kicinski wrote:
> On Wed, 16 Apr 2025 07:24:22 -0300 Victor Nogueira wrote:
>> As described in Gerrard's report [1], there are cases where netem can
>> make the qdisc enqueue callback reentrant. Some qdiscs (drr, hfsc, ets,
>> qfq) break whenever the enqueue callback has reentrant behaviour.
>> This series addresses these issues by adding extra checks that cater for
>> these reentrant corner cases. This series has passed all relevant test
>> cases in the TDC suite.
> 
> Sorry for asking this question a bit late, but reentrant enqueue seems
> error prone. Is there a clear use case for netem as a child?

We discussed this internally as well before i sent this fix.
The 3 examples are buggy in picking the correct active class in the
corner case the poc produced. So these are bug fixes regardless - and
they happen to fix the issue.
We also wondered why it was not appropriate for netem to always be root
qdisc. Looking around the manpage states that you can have a qdisc
like tbf as a parent. This issue happens only when netem "duplication
feature" is used. Not sure what other use cases are out there that
expect netem to be a child.
+Cc Stephen who can give a more authoritative answer.

> If so should we also add some sort of "capability" to avoid new qdiscs
> falling into the same trap, without giving the problem any thought?
I think the bug needs to be fix regardless, but I think your
suggestion also makes sense and is more future proof.
I can send another patch later that adds a "capability",as you
suggested, (let's say ALLOWS_REENTRANT_ENQ or something similar)
and only allows netem to be a child of qdiscs that have this flag.