| 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 linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20230912013332.2048422-1-jrife@google.com>
Date: Mon, 11 Sep 2023 20:33:31 -0500
From: Jordan Rife <jrife@...gle.com>
To: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, netdev@...r.kernel.org
Cc: dborkman@...nel.org, Jordan Rife <jrife@...gle.com>
Subject: [PATCH net] net: prevent address overwrite in connect() and sendmsg()
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@...gle.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
Powered by blists - more mailing lists