netdev - Re: [PATCH net v2] 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: <CANn89iLpqh3vZ2qEYhhL12qDgVH1rgSJWqHf42cfX0qQfCvQ_w@mail.gmail.com>
Date:   Tue, 4 Apr 2023 10:03:21 +0200
From:   Eric Dumazet <edumazet@...gle.com>
To:     Felix Huettner <felix.huettner@...l.schwarz>
Cc:     netdev@...r.kernel.org, dev@...nvswitch.org,
        linux-kernel@...r.kernel.org, pabeni@...hat.com, kuba@...nel.org,
        pshelar@....org, davem@...emloft.net, luca.czesla@...l.schwarz
Subject: Re: [PATCH net v2] net: openvswitch: fix race on port output

On Tue, Apr 4, 2023 at 9:33 AM Felix Huettner
<felix.huettner@...l.schwarz> wrote:
>
> assume the following setup on a single machine:
> 1. An openvswitch instance with one bridge and default flows
> 2. two network namespaces "server" and "client"
> 3. two ovs interfaces "server" and "client" on the bridge
> 4. for each ovs interface a veth pair with a matching name and 32 rx and
>    tx queues
> 5. move the ends of the veth pairs to the respective network namespaces
> 6. assign ip addresses to each of the veth ends in the namespaces (needs
>    to be the same subnet)
> 7. start some http server on the server network namespace
> 8. test if a client in the client namespace can reach the http server
>
> when following the actions below the host has a chance of getting a cpu
> stuck in a infinite loop:
> 1. send a large amount of parallel requests to the http server (around
>    3000 curls should work)
> 2. in parallel delete the network namespace (do not delete interfaces or
>    stop the server, just kill the namespace)
>

> Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action")
> Co-developed-by: Luca Czesla <luca.czesla@...l.schwarz>
> Signed-off-by: Luca Czesla <luca.czesla@...l.schwarz>
> Signed-off-by: Felix Huettner <felix.huettner@...l.schwarz>
> ---
> v2:
>   - replace BUG_ON with DEBUG_NET_WARN_ON_ONCE
>   - use netif_carrier_ok() instead of checking for NETREG_REGISTERED
> v1: https://lore.kernel.org/netdev/ZCaXfZTwS9MVk8yZ@kernel-bug-kernel-bug/
>
>  net/core/dev.c            | 1 +
>  net/openvswitch/actions.c | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 253584777101..37b26017f458 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)) {
> +               DEBUG_NET_WARN_ON_ONCE(unlikely(qcount == 0));

No need for unlikely(), it is already done in DEBUG_NET_WARN_ON_ONCE()

Thanks.