Message ID | 20230912031630.678178-2-khalid.elmously@canonical.com |
---|---|
State | New |
Headers | show |
Series | net: Avoid address overwrite in kernel_connect | expand |
Sent v2 On 2023-09-11 23:16:30 , Khalid Elmously wrote: > From: Jordan Rife <jrife@google.com> > > BugLink: https://bugs.launchpad.net/bugs/2035163 > > BPF programs that run on connect can rewrite the connect address. For > the connect system call this isn't a problem, because a copy of the address > is made when it is moved into kernel space. However, kernel_connect > simply passes through the address it is given, so the caller may observe > its address value unexpectedly change. > > A practical example where this is problematic is where NFS is combined > with a system such as Cilium which implements BPF-based load balancing. > A common pattern in software-defined storage systems is to have an NFS > mount that connects to a persistent virtual IP which in turn maps to an > ephemeral server IP. This is usually done to achieve high availability: > if your server goes down you can quickly spin up a replacement and remap > the virtual IP to that endpoint. With BPF-based load balancing, mounts > will forget the virtual IP address when the address rewrite occurs > because a pointer to the only copy of that address is passed down the > stack. Server failover then breaks, because clients have forgotten the > virtual IP address. Reconnects fail and mounts remain broken. This patch > was tested by setting up a scenario like this and ensuring that NFS > reconnects worked after applying the patch. > > Signed-off-by: Jordan Rife <jrife@google.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > (backported from commit 0bdf399342c5acbd817c9098b6c7ed21f1974312) > [ kmously: adjusted for lack of READ_ONCE() ] > Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com> > --- > net/socket.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/net/socket.c b/net/socket.c > index 5c49074ef7f2ae..7344dcc7cb1ccb 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -3453,7 +3453,12 @@ EXPORT_SYMBOL(kernel_accept); > int kernel_connect(struct socket *sock, struct sockaddr *addr, int addrlen, > int flags) > { > - return sock->ops->connect(sock, addr, addrlen, flags); > + struct sockaddr_storage address; > + > + memcpy(&address, addr, addrlen); > + > + return sock->ops->connect(sock, (struct sockaddr *)&address, > + addrlen, flags); > } > EXPORT_SYMBOL(kernel_connect); > > -- > 2.34.1 >
diff --git a/net/socket.c b/net/socket.c index 5c49074ef7f2ae..7344dcc7cb1ccb 100644 --- a/net/socket.c +++ b/net/socket.c @@ -3453,7 +3453,12 @@ EXPORT_SYMBOL(kernel_accept); int kernel_connect(struct socket *sock, struct sockaddr *addr, int addrlen, int flags) { - return sock->ops->connect(sock, addr, addrlen, flags); + struct sockaddr_storage address; + + memcpy(&address, addr, addrlen); + + return sock->ops->connect(sock, (struct sockaddr *)&address, + addrlen, flags); } EXPORT_SYMBOL(kernel_connect);