lkml.org 
[lkml]   [2022]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v1 net 1/3] udp: Update reuse->has_conns under reuseport_lock.
From
Date
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 <kuniyu@amazon.com>
> ---
> 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

\
 
 \ /
  Last update: 2022-10-11 12:51    [W:0.076 / U:0.496 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site