From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 46743C433F5 for ; Tue, 11 Oct 2022 10:51:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229831AbiJKKvB (ORCPT ); Tue, 11 Oct 2022 06:51:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56150 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229600AbiJKKu6 (ORCPT ); Tue, 11 Oct 2022 06:50:58 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5E02A32D8A for ; Tue, 11 Oct 2022 03:50:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1665485456; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=gA1xTwBH5RqPycM5mLkFtITn/VHkui9LBIiES9K73V0=; b=eAmBl+R6/SoI6jl7D4E7idIZnoFyBJ09AOMEufL6PR0nmDj75FRqvbBvwBv9FZSKjw3NX4 0fju5Ylfmos1UEF0lHHZkHPlqlm/6q8pAcEt7ZIolnwhynUIg/EnsBQvYaYtqLRKxsATJV zhhaq6U3/2+t+GmUISMquC201dP3vvw= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-227-WZMSWxtLOZi_Tw_8T_YM6g-1; Tue, 11 Oct 2022 06:50:55 -0400 X-MC-Unique: WZMSWxtLOZi_Tw_8T_YM6g-1 Received: by mail-wm1-f70.google.com with SMTP id o18-20020a05600c339200b003bf24961658so8267597wmp.6 for ; Tue, 11 Oct 2022 03:50:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:user-agent:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=gA1xTwBH5RqPycM5mLkFtITn/VHkui9LBIiES9K73V0=; b=wQrZl0wqndHeLp4w4pQxaHuKSgazdOYIw+/84bHRMvRnvE6zzWb3hc90rmJSBm5eyQ kEwEXbBlRorUjIhmEopyYNREHg1sC8PQshucYgD4BZb0U8LKdadxTYEsRWbkL79V20Ie WSLpYfy0Xt8fKGLRJfwq71o/eLvSVBETNfLJcmPCqgAzWDwi0jePwTzWoy/u5qGiVJrm 2o4rF0cFyEoZLavQtY63DzZPXSWGh9svVjd1FQUjSJNg8HKBvWdHcLTaLHsEQ3u1cIR7 QF+ex4sIvOyPt5Abxa3r6XM3MOqbZybmtPisgr1R06ZlpXRaV27R2gQKXixsJjJq3Ozb n9BA== X-Gm-Message-State: ACrzQf3mOeqF//YMeeRF81YSCKNLZYPwEQfeCl4dwnXI5eu4cROcVYQZ SJvQfO/dq7gEyKnXtxGozn/ARSvhBXpaNHcRlL6FM577JaZkfqDWBnuVPGI1TD/zPA3gYfIJNtS bXUjz8UJTfdh6PFq4yGPcgiOZ X-Received: by 2002:a05:600c:4e8b:b0:3b4:c8ce:be87 with SMTP id f11-20020a05600c4e8b00b003b4c8cebe87mr24324316wmq.157.1665485454149; Tue, 11 Oct 2022 03:50:54 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5A2aCSLEQ7n2luiRil7L1Xyh3BGEAwhpc9iGVeInfgi5+eo2/99Yn9JDz3Jz41LLe0v+8AMQ== X-Received: by 2002:a05:600c:4e8b:b0:3b4:c8ce:be87 with SMTP id f11-20020a05600c4e8b00b003b4c8cebe87mr24324305wmq.157.1665485453907; Tue, 11 Oct 2022 03:50:53 -0700 (PDT) Received: from gerbillo.redhat.com (146-241-103-235.dyn.eolo.it. [146.241.103.235]) by smtp.gmail.com with ESMTPSA id r3-20020a5d4983000000b002301c026acasm5949767wrq.85.2022.10.11.03.50.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 Oct 2022 03:50:53 -0700 (PDT) Message-ID: <468f01fc1dde6cf44fab51653eeb626fc8521db2.camel@redhat.com> Subject: Re: [PATCH v1 net 1/3] udp: Update reuse->has_conns under reuseport_lock. From: Paolo Abeni To: Kuniyuki Iwashima , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Hideaki YOSHIFUJI , David Ahern , Martin KaFai Lau Cc: Craig Gallek , Willem de Bruijn , Kuniyuki Iwashima , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Date: Tue, 11 Oct 2022 12:50:52 +0200 In-Reply-To: <20221010174351.11024-2-kuniyu@amazon.com> References: <20221010174351.11024-1-kuniyu@amazon.com> <20221010174351.11024-2-kuniyu@amazon.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.42.4 (3.42.4-2.fc35) MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2022-10-10 at 10:43 -0700, Kuniyuki Iwashima wrote: > When we call connect() for a UDP socket in a reuseport group, we have > to update sk->sk_reuseport_cb->has_conns to 1. Otherwise, the kernel > could select a unconnected socket wrongly for packets sent to the > connected socket. > > However, the current way to set has_conns is illegal and possible to > trigger that problem. reuseport_has_conns() changes has_conns under > rcu_read_lock(), which upgrades the RCU reader to the updater. Then, > it must do the update under the updater's lock, reuseport_lock, but > it doesn't for now. > > For this reason, there is a race below where we fail to set has_conns > resulting in the wrong socket selection. To avoid the race, let's split > the reader and updater with proper locking. > > cpu1 cpu2 > +----+ +----+ > > __ip[46]_datagram_connect() reuseport_grow() > . . > > - reuseport_has_conns(sk, true) |- more_reuse = __reuseport_alloc(more_socks_size) > > . | > > |- rcu_read_lock() > > |- reuse = rcu_dereference(sk->sk_reuseport_cb) > > | > > | | /* reuse->has_conns == 0 here */ > > | |- more_reuse->has_conns = reuse->has_conns > > |- reuse->has_conns = 1 | /* more_reuse->has_conns SHOULD BE 1 HERE */ > > | | > > | |- rcu_assign_pointer(reuse->socks[i]->sk_reuseport_cb, > > | | more_reuse) > > `- rcu_read_unlock() `- kfree_rcu(reuse, rcu) > > > > - sk->sk_state = TCP_ESTABLISHED > > Fixes: acdcecc61285 ("udp: correct reuseport selection with connected sockets") > Signed-off-by: Kuniyuki Iwashima > --- > include/net/sock_reuseport.h | 23 +++++++++++++++++------ > net/ipv4/datagram.c | 2 +- > net/ipv4/udp.c | 2 +- > net/ipv6/datagram.c | 2 +- > net/ipv6/udp.c | 2 +- > 5 files changed, 21 insertions(+), 10 deletions(-) > > diff --git a/include/net/sock_reuseport.h b/include/net/sock_reuseport.h > index 473b0b0fa4ab..fe9779e6d90f 100644 > --- a/include/net/sock_reuseport.h > +++ b/include/net/sock_reuseport.h > @@ -43,21 +43,32 @@ struct sock *reuseport_migrate_sock(struct sock *sk, > extern int reuseport_attach_prog(struct sock *sk, struct bpf_prog *prog); > extern int reuseport_detach_prog(struct sock *sk); > > -static inline bool reuseport_has_conns(struct sock *sk, bool set) > +static inline bool reuseport_has_conns(struct sock *sk) > { > struct sock_reuseport *reuse; > bool ret = false; > > rcu_read_lock(); > reuse = rcu_dereference(sk->sk_reuseport_cb); > - if (reuse) { > - if (set) > - reuse->has_conns = 1; > - ret = reuse->has_conns; > - } > + if (reuse && reuse->has_conns) > + ret = true; > rcu_read_unlock(); > > return ret; > } > > +static inline void reuseport_has_conns_set(struct sock *sk) > +{ > + struct sock_reuseport *reuse; > + > + if (!rcu_access_pointer(sk->sk_reuseport_cb)) > + return; > + > + spin_lock(&reuseport_lock); > + reuse = rcu_dereference_protected(sk->sk_reuseport_cb, > + lockdep_is_held(&reuseport_lock)); > + reuse->has_conns = 1; > + spin_unlock(&reuseport_lock); > +} Since the above is not super critical, it's probably better move it into sock_reuseport.c file and export it (to fix the build issue) Cheers, Paolo