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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 11 Oct 2022 12:50:52 +0200 From: Paolo Abeni <pabeni@...hat.com> To: Kuniyuki Iwashima <kuniyu@...zon.com>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>, David Ahern <dsahern@...nel.org>, Martin KaFai Lau <martin.lau@...nel.org> Cc: Craig Gallek <kraig@...gle.com>, Willem de Bruijn <willemb@...gle.com>, Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v1 net 1/3] udp: Update reuse->has_conns under reuseport_lock. 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@...zon.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
Powered by blists - more mailing lists