[PATCH] net: Avoid address overwrite in kernel_connect

[PATCH] net: Avoid address overwrite in kernel_connect

[Jordan Rife]

commit 0bdf399 upstream.

This fix applies to all stable kernel versions 4.19+.

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>
---
 net/socket.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/socket.c b/net/socket.c
index ce70c01eb2f3e..db9d908198f21 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -3468,7 +3468,11 @@ 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.42.0.283.g2d96d420d3-goog

Re: [PATCH] net: Avoid address overwrite in kernel_connect

[Greg KH]

On Tue, Sep 05, 2023 at 06:58:46PM -0500, Jordan Rife wrote:
> commit 0bdf399 upstream.

Nit, next time use more sha1 characters, the kernel documentation shows
how many we usually rely on.

> This fix applies to all stable kernel versions 4.19+.

Why not also 4.14?

Anyway, now queued up, thanks.

greg k-h

Re: [PATCH] net: Avoid address overwrite in kernel_connect

[Jordan Rife]

> Nit, next time use more sha1 characters, the kernel documentation shows
how many we usually rely on.

Ack. I'll keep this in mind next time.

> Why not also 4.14?
The BPF hooks that lead to this problem were introduced after 4.14 in
this upstream commit (d74bad4e74ee373787a9ae24197c17b7cdc428d5). 4.19
is the earliest supported kernel version in which this bug appears.

Thanks,
Jordan

On Thu, Sep 7, 2023 at 4:27 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Sep 05, 2023 at 06:58:46PM -0500, Jordan Rife wrote:
> > commit 0bdf399 upstream.
>
> Nit, next time use more sha1 characters, the kernel documentation shows
> how many we usually rely on.
>
> > This fix applies to all stable kernel versions 4.19+.
>
> Why not also 4.14?
>
> Anyway, now queued up, thanks.
>
> greg k-h