Re: [PATCH net v3 4/7] net/sched: netem: restructure dequeue to avoid re-entrancy with child qdisc

[Simon Horman]

On Mon, Apr 06, 2026 at 10:12:38AM -0700, Stephen Hemminger wrote:
> On Mon, 6 Apr 2026 08:41:33 -0700
> Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
> 
> > On Sat, 4 Apr 2026 10:49:46 +0100 Simon Horman wrote:
> > > On Thu, Apr 02, 2026 at 01:19:32PM -0700, Stephen Hemminger wrote:  
> > > > netem_dequeue() enqueues packets into its child qdisc while being
> > > > called from the parent's dequeue path. This causes two problems:
> > > > 
> > > > - HFSC tracks class active/inactive state on qlen transitions.
> > > >   A child enqueue during dequeue causes double-insertion into
> > > >   the eltree (CVE-2025-37890, CVE-2025-38001).
> > > > 
> > > > - Non-work-conserving children like TBF may refuse to dequeue
> > > >   packets just enqueued, causing netem to return NULL despite
> > > >   having backlog. Parents like DRR then incorrectly deactivate
> > > >   the class.
> > > > 
> > > > Split the dequeue into helpers:
> > > > 
> > > >   netem_pull_tfifo()    - remove head packet from tfifo
> > > >   netem_slot_account()  - update slot pacing counters
> > > >   netem_dequeue_child() - batch-transfer ready packets to the
> > > >                           child, then dequeue from the child
> > > >   netem_dequeue_direct()- dequeue from tfifo when no child
> > > > 
> > > > When a child qdisc is present, all time-ready packets are moved
> > > > into the child before calling its dequeue. This separates the
> > > > enqueue and dequeue phases so the parent sees consistent qlen
> > > > transitions.
> > > > 
> > > > Fixes: 50612537e9ab ("netem: fix classful handling")
> > > > Signed-off-by: Stephen Hemminger <stephen@xxxxxxxxxxxxxxxxxx>
> > > > ---
> > > >  net/sched/sch_netem.c | 201 +++++++++++++++++++++++++++---------------
> > > >  1 file changed, 128 insertions(+), 73 deletions(-)    
> > > 
> > > Hi Stephen,
> > > 
> > > As a fix this is a large and complex patch.
> > > Could it be split up somehow to aid review?  
> > 
> > +1, FWIW it's perfectly fine to have refactoring patch in a net series
> > (without a Fixes tag) if it makes the fix a lot easier to review.
> 
> I split it into refactoring followed by fix for next version
> 
> The fix alone just gets really confusing to look at;
> I got more confused the pre-existing spaghetti code here..

Thanks Stephen,

Just to clarify, in case others come back to this thread for some reason,
that the aim here is to aid review. So whatever works in that direction is
appreciated.