netdev - Re: [PATCH net] net: sched: gred: prevent races when adding offloads to stats

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 19 Jan 2023 13:16:28 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Paolo Abeni <pabeni@...hat.com>, davem@...emloft.net,
        netdev@...r.kernel.org, edumazet@...gle.com,
        Linux Kernel Functional Testing <lkft@...aro.org>,
        jhs@...atatu.com, jiri@...nulli.us
Subject: Re: [PATCH net] net: sched: gred: prevent races when adding
 offloads to stats

On Thu, 19 Jan 2023 12:19:53 -0800 Cong Wang wrote:
> > > I'm unsure I read you comment correctly. Please note that the
> > > referenced message includes several splats. The first one - arguably
> > > the most relevant - points to the lack of locking in the gred control
> > > path.  
> > 
> > Yup, I'm not really sure if we're fixing the right splat for the bug.
> > But I am fairly confident we should be holding a lock while writing
> > bstats from the dump path, enqueue/dequeue may run concurrently.  
> 
> Explain htb_dump_class_stats()? :) I see two _bstats_update() calls but
> I don't see any tree lock there.

I don't know the HTB offload, if the datapath writes to the same
cl->bstats then it's buggy in the same way. But maybe it doesn't - 
the HTB offload is local (as in offloads normal host egress) while
the RED offloads are for switches so the data flows both thru the
"representor" netdevs and therefore the SW instance and the offload 
on the HW forwarding path.