From: Jordan Rife <jrife@google.com>
To: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, netdev@vger.kernel.org
Cc: dborkman@kernel.org, Jordan Rife <jrife@google.com>
Subject: [PATCH net] net: prevent address overwrite in connect() and sendmsg()
Date: Mon, 11 Sep 2023 20:33:31 -0500 [thread overview]
Message-ID: <20230912013332.2048422-1-jrife@google.com> (raw)
commit 0bdf399342c5 ("net: Avoid address overwrite in kernel_connect")
ensured that kernel_connect() will not overwrite the address parameter
in cases where BPF connect hooks perform an address rewrite. However,
there remain other cases where BPF hooks can overwrite an address held
by a kernel client.
==Scenarios Tested==
* Code in the SMB and Ceph modules calls sock->ops->connect() directly,
allowing the address overwrite to occur. In the case of SMB, this can
lead to broken mounts.
* NFS v3 mounts with proto=udp call sock_sendmsg() for each RPC call,
passing a pointer to the mount address in msg->msg_name which is
later overwritten by a BPF sendmsg hook. This can lead to broken NFS
mounts.
In order to more comprehensively fix this class of problems, this patch
pushes the address copy deeper into the stack and introduces an address
copy to both udp_sendmsg() and udpv6_sendmsg() to insulate all callers
from address rewrites.
Signed-off-by: Jordan Rife <jrife@google.com>
---
net/ipv4/af_inet.c | 18 ++++++++++++++++++
net/ipv4/udp.c | 21 ++++++++++++++++-----
net/ipv6/udp.c | 23 +++++++++++++++++------
net/socket.c | 7 +------
4 files changed, 52 insertions(+), 17 deletions(-)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 3d2e30e204735..c37d484fbee34 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -568,6 +568,7 @@ int inet_dgram_connect(struct socket *sock, struct sockaddr *uaddr,
{
struct sock *sk = sock->sk;
const struct proto *prot;
+ struct sockaddr_storage addr;
int err;
if (addr_len < sizeof(uaddr->sa_family))
@@ -580,6 +581,14 @@ int inet_dgram_connect(struct socket *sock, struct sockaddr *uaddr,
return prot->disconnect(sk, flags);
if (BPF_CGROUP_PRE_CONNECT_ENABLED(sk)) {
+ if (uaddr && addr_len <= sizeof(addr)) {
+ /* pre_connect can rewrite uaddr, so make a copy to
+ * insulate the caller.
+ */
+ memcpy(&addr, uaddr, addr_len);
+ uaddr = (struct sockaddr *)&addr;
+ }
+
err = prot->pre_connect(sk, uaddr, addr_len);
if (err)
return err;
@@ -625,6 +634,7 @@ int __inet_stream_connect(struct socket *sock, struct sockaddr *uaddr,
int addr_len, int flags, int is_sendmsg)
{
struct sock *sk = sock->sk;
+ struct sockaddr_storage addr;
int err;
long timeo;
@@ -668,6 +678,14 @@ int __inet_stream_connect(struct socket *sock, struct sockaddr *uaddr,
goto out;
if (BPF_CGROUP_PRE_CONNECT_ENABLED(sk)) {
+ if (uaddr && addr_len <= sizeof(addr)) {
+ /* pre_connect can rewrite uaddr, so make a copy to
+ * insulate the caller.
+ */
+ memcpy(&addr, uaddr, addr_len);
+ uaddr = (struct sockaddr *)&addr;
+ }
+
err = sk->sk_prot->pre_connect(sk, uaddr, addr_len);
if (err)
goto out;
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index f39b9c8445808..5f5ee2752eeb7 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1142,18 +1142,29 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
}
if (cgroup_bpf_enabled(CGROUP_UDP4_SENDMSG) && !connected) {
+ struct sockaddr_in tmp_addr;
+ struct sockaddr_in *addr = usin;
+
+ /* BPF_CGROUP_RUN_PROG_UDP4_SENDMSG_LOCK can rewrite usin, so make a
+ * copy to insulate the caller.
+ */
+ if (usin && msg->msg_namelen <= sizeof(tmp_addr)) {
+ memcpy(&tmp_addr, usin, msg->msg_namelen);
+ addr = &tmp_addr;
+ }
+
err = BPF_CGROUP_RUN_PROG_UDP4_SENDMSG_LOCK(sk,
- (struct sockaddr *)usin, &ipc.addr);
+ (struct sockaddr *)addr, &ipc.addr);
if (err)
goto out_free;
- if (usin) {
- if (usin->sin_port == 0) {
+ if (addr) {
+ if (addr->sin_port == 0) {
/* BPF program set invalid port. Reject it. */
err = -EINVAL;
goto out_free;
}
- daddr = usin->sin_addr.s_addr;
- dport = usin->sin_port;
+ daddr = addr->sin_addr.s_addr;
+ dport = addr->sin_port;
}
}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 86b5d509a4688..cbc1917fad629 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1506,26 +1506,37 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
fl6->fl6_sport = inet->inet_sport;
if (cgroup_bpf_enabled(CGROUP_UDP6_SENDMSG) && !connected) {
+ struct sockaddr_in6 tmp_addr;
+ struct sockaddr_in6 *addr = sin6;
+
+ /* BPF_CGROUP_RUN_PROG_UDP6_SENDMSG_LOCK can rewrite sin6, so make a
+ * copy to insulate the caller.
+ */
+ if (sin6 && addr_len <= sizeof(tmp_addr)) {
+ memcpy(&tmp_addr, sin6, addr_len);
+ addr = &tmp_addr;
+ }
+
err = BPF_CGROUP_RUN_PROG_UDP6_SENDMSG_LOCK(sk,
- (struct sockaddr *)sin6,
+ (struct sockaddr *)addr,
&fl6->saddr);
if (err)
goto out_no_dst;
- if (sin6) {
- if (ipv6_addr_v4mapped(&sin6->sin6_addr)) {
+ if (addr) {
+ if (ipv6_addr_v4mapped(&addr->sin6_addr)) {
/* BPF program rewrote IPv6-only by IPv4-mapped
* IPv6. It's currently unsupported.
*/
err = -ENOTSUPP;
goto out_no_dst;
}
- if (sin6->sin6_port == 0) {
+ if (addr->sin6_port == 0) {
/* BPF program set invalid port. Reject it. */
err = -EINVAL;
goto out_no_dst;
}
- fl6->fl6_dport = sin6->sin6_port;
- fl6->daddr = sin6->sin6_addr;
+ fl6->fl6_dport = addr->sin6_port;
+ fl6->daddr = addr->sin6_addr;
}
}
diff --git a/net/socket.c b/net/socket.c
index c8b08b32f097e..39794d026fa11 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -3570,12 +3570,7 @@ EXPORT_SYMBOL(kernel_accept);
int kernel_connect(struct socket *sock, struct sockaddr *addr, int addrlen,
int flags)
{
- struct sockaddr_storage address;
-
- memcpy(&address, addr, addrlen);
-
- return READ_ONCE(sock->ops)->connect(sock, (struct sockaddr *)&address,
- addrlen, flags);
+ return READ_ONCE(sock->ops)->connect(sock, addr, addrlen, flags);
}
EXPORT_SYMBOL(kernel_connect);
--
2.42.0.283.g2d96d420d3-goog
next reply other threads:[~2023-09-12 1:33 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-12 1:33 Jordan Rife [this message]
2023-09-12 13:33 ` [PATCH net] net: prevent address overwrite in connect() and sendmsg() Willem de Bruijn
2023-09-12 14:22 ` Daniel Borkmann
2023-09-12 18:31 ` Jordan Rife
2023-09-12 19:35 ` Willem de Bruijn
2023-09-12 20:07 ` Jordan Rife
2023-09-12 20:48 ` Willem de Bruijn
2023-09-12 21:08 ` Jordan Rife
2023-09-13 14:02 ` Willem de Bruijn
2023-09-13 18:04 ` Jordan Rife
2023-09-13 19:10 ` Willem de Bruijn
2023-09-14 8:24 ` Paolo Abeni
2023-09-14 18:41 ` Jordan Rife
2023-09-15 0:16 ` Willem de Bruijn
2023-09-15 0:21 ` Jordan Rife
2023-09-13 7:19 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230912013332.2048422-1-jrife@google.com \
--to=jrife@google.com \
--cc=davem@davemloft.net \
--cc=dborkman@kernel.org \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).