linux-kernel - Re: [PATCH] net: openvswitch: fix race on port output

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: <20230403115011.0d93298c@kernel.org>
Date:   Mon, 3 Apr 2023 11:50:11 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Felix Hüttner <felix.huettner@...l.schwarz>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "dev@...nvswitch.org" <dev@...nvswitch.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Luca Czesla <Luca.Czesla@...l.schwarz>
Subject: Re: [PATCH] net: openvswitch: fix race on port output

On Mon, 3 Apr 2023 11:18:46 +0000 Felix Hüttner wrote:
> On Sat, 1 Apr 2023 6:25:00 +0000 Jakub Kicinski wrote:
> > On Fri, 31 Mar 2023 06:25:13 +0000 Felix Hüttner wrote:  
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index 253584777101..6628323b7bea 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -3199,6 +3199,7 @@ static u16 skb_tx_hash(const struct net_device *dev,
> > >         }
> > >
> > >         if (skb_rx_queue_recorded(skb)) {
> > > +               BUG_ON(unlikely(qcount == 0));  
> >
> > DEBUG_NET_WARN_ON()
> >  
> 
> However if this condition triggers we will be permanently stuck in the loop below.
> From my understading this also means that future calls to `synchronize_net` will never finish (as the packet never finishes processing).
> So the user will quite probably need to restart his system.
> I find DEBUG_NET_WARN_ON_ONCE to offer too little visiblity as CONFIG_DEBUG_NET is not necessarily enabled per default.
> I as the user would see it as helpful to have this information available without additional config flags.
> I would propose to use WARN_ON_ONCE

skb_tx_hash() may get called a lot, we shouldn't slow it down on
production systems just to catch buggy drivers, IMO.