commit 86a7e0b69bd5b812e48a20c66c2161744f3caa16 upstream.
This commit was already queued for backport to 5.4+ but did not cleanly
apply to 4.19 due to some merge conflicts. This patch fixes merge
conflicts for backport to 4.19.
Callers of sock_sendmsg(), and similarly kernel_sendmsg(), in kernel
space may observe their value of msg_name change in cases where BPF
sendmsg hooks rewrite the send address. This has been confirmed to break
NFS mounts running in UDP mode and has the potential to break other
systems.
This patch:
1) Creates a new function called __sock_sendmsg() with same logic as the
old sock_sendmsg() function.
2) Replaces calls to sock_sendmsg() made by __sys_sendto() and
__sys_sendmsg() with __sock_sendmsg() to avoid an unnecessary copy,
as these system calls are already protected.
3) Modifies sock_sendmsg() so that it makes a copy of msg_name if
present before passing it down the stack to insulate callers from
changes to the send address.
Link: https://lore.kernel.org/netdev/20230912013332.2048422-1-jrife@google.com/
Fixes: 1cedee13d25a ("bpf: Hooks for sys_sendmsg")
Cc: <stable(a)vger.kernel.org> # 4.19.x
Reviewed-by: Willem de Bruijn <willemb(a)google.com>
Signed-off-by: Jordan Rife <jrife(a)google.com>
Reviewed-by: Simon Horman <horms(a)kernel.org>
Signed-off-by: David S. Miller <davem(a)davemloft.net>
---
net/socket.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index db9d908198f21..8c1a4e26c01fb 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -653,13 +653,30 @@ static inline int sock_sendmsg_nosec(struct socket *sock, struct msghdr *msg)
return ret;
}
-int sock_sendmsg(struct socket *sock, struct msghdr *msg)
+static int __sock_sendmsg(struct socket *sock, struct msghdr *msg)
{
int err = security_socket_sendmsg(sock, msg,
msg_data_left(msg));
return err ?: sock_sendmsg_nosec(sock, msg);
}
+
+int sock_sendmsg(struct socket *sock, struct msghdr *msg)
+{
+ struct sockaddr_storage *save_addr = (struct sockaddr_storage *)msg->msg_name;
+ struct sockaddr_storage address;
+ int ret;
+
+ if (msg->msg_name) {
+ memcpy(&address, msg->msg_name, msg->msg_namelen);
+ msg->msg_name = &address;
+ }
+
+ ret = __sock_sendmsg(sock, msg);
+ msg->msg_name = save_addr;
+
+ return ret;
+}
EXPORT_SYMBOL(sock_sendmsg);
/**
@@ -963,7 +980,7 @@ static ssize_t sock_write_iter(struct kiocb *iocb, struct iov_iter *from)
if (sock->type == SOCK_SEQPACKET)
msg.msg_flags |= MSG_EOR;
- res = sock_sendmsg(sock, &msg);
+ res = __sock_sendmsg(sock, &msg);
*from = msg.msg_iter;
return res;
}
@@ -1896,7 +1913,7 @@ int __sys_sendto(int fd, void __user *buff, size_t len, unsigned int flags,
if (sock->file->f_flags & O_NONBLOCK)
flags |= MSG_DONTWAIT;
msg.msg_flags = flags;
- err = sock_sendmsg(sock, &msg);
+ err = __sock_sendmsg(sock, &msg);
out_put:
fput_light(sock->file, fput_needed);
@@ -2224,7 +2241,7 @@ static int ___sys_sendmsg(struct socket *sock, struct user_msghdr __user *msg,
err = sock_sendmsg_nosec(sock, msg_sys);
goto out_freectl;
}
- err = sock_sendmsg(sock, msg_sys);
+ err = __sock_sendmsg(sock, msg_sys);
/*
* If this is sendmmsg() and sending to current destination address was
* successful, remember it.
--
2.42.0.609.gbb76f46606-goog
From: Arnd Bergmann <arnd(a)arndb.de>
bacmp() is a wrapper around memcpy(), which contain compile-time
checks for buffer overflow. Since the hci_conn_request_evt() also calls
bt_dev_dbg() with an implicit NULL pointer check, the compiler is now
aware of a case where 'hdev' is NULL and treats this as meaning that
zero bytes are available:
In file included from net/bluetooth/hci_event.c:32:
In function 'bacmp',
inlined from 'hci_conn_request_evt' at net/bluetooth/hci_event.c:3276:7:
include/net/bluetooth/bluetooth.h:364:16: error: 'memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread]
364 | return memcmp(ba1, ba2, sizeof(bdaddr_t));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Add another NULL pointer check before the bacmp() to ensure the compiler
understands the code flow enough to not warn about it. Since the patch
that introduced the warning is marked for stable backports, this one
should also go that way to avoid introducing build regressions.
Fixes: d70e44fef8621 ("Bluetooth: Reject connection with the device which has same BD_ADDR")
Cc: Kees Cook <keescook(a)chromium.org>
Cc: "Lee, Chun-Yi" <jlee(a)suse.com>
Cc: Luiz Augusto von Dentz <luiz.von.dentz(a)intel.com>
Cc: Marcel Holtmann <marcel(a)holtmann.org>
Cc: stable(a)vger.kernel.org
Signed-off-by: Arnd Bergmann <arnd(a)arndb.de>
---
v2: rewrite completely
---
net/bluetooth/hci_event.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 6f4409b4c3648..9b34c9f8ee02c 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3273,7 +3273,7 @@ static void hci_conn_request_evt(struct hci_dev *hdev, void *data,
/* Reject incoming connection from device with same BD ADDR against
* CVE-2020-26555
*/
- if (!bacmp(&hdev->bdaddr, &ev->bdaddr)) {
+ if (hdev && !bacmp(&hdev->bdaddr, &ev->bdaddr)) {
bt_dev_dbg(hdev, "Reject connection with same BD_ADDR %pMR\n",
&ev->bdaddr);
hci_reject_conn(hdev, &ev->bdaddr);
--
2.39.2