Blob Blame History Raw
From 7bf99c75124102fede4884355ec0d9e48c3956a7 Mon Sep 17 00:00:00 2001
From: Peter Collingbourne <pcc@google.com>
Date: Thu, 26 Aug 2021 12:46:01 -0700
Subject: [PATCH] net: don't unconditionally copy_from_user a struct ifreq for socket ioctls
Git-commit: d0efb16294d145d157432feda83877ae9d7cdf37
Patch-mainline: v5.15-rc1
References: stable-5.14.1

commit d0efb16294d145d157432feda83877ae9d7cdf37 upstream.

A common implementation of isatty(3) involves calling a ioctl passing
a dummy struct argument and checking whether the syscall failed --
bionic and glibc use TCGETS (passing a struct termios), and musl uses
TIOCGWINSZ (passing a struct winsize). If the FD is a socket, we will
copy sizeof(struct ifreq) bytes of data from the argument and return
-EFAULT if that fails. The result is that the isatty implementations
may return a non-POSIX-compliant value in errno in the case where part
of the dummy struct argument is inaccessible, as both struct termios
and struct winsize are smaller than struct ifreq (at least on arm64).

Although there is usually enough stack space following the argument
on the stack that this did not present a practical problem up to now,
with MTE stack instrumentation it's more likely for the copy to fail,
as the memory following the struct may have a different tag.

Fix the problem by adding an early check for whether the ioctl is a
valid socket ioctl, and return -ENOTTY if it isn't.

Fixes: 44c02a2c3dc5 ("dev_ioctl(): move copyin/copyout to callers")
Link: https://linux-review.googlesource.com/id/I869da6cf6daabc3e4b7b82ac979683ba05e27d4d
Signed-off-by: Peter Collingbourne <pcc@google.com>
Cc: <stable@vger.kernel.org> # 4.19
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Takashi Iwai <tiwai@suse.de>

---
 include/linux/netdevice.h |    4 ++++
 net/socket.c              |    5 +++++
 2 files changed, 9 insertions(+)

--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4013,6 +4013,10 @@ int netdev_rx_handler_register(struct ne
 void netdev_rx_handler_unregister(struct net_device *dev);
 
 bool dev_valid_name(const char *name);
+static inline bool is_socket_ioctl_cmd(unsigned int cmd)
+{
+	return _IOC_TYPE(cmd) == SOCK_IOC_TYPE;
+}
 int get_user_ifreq(struct ifreq *ifr, void __user **ifrdata, void __user *arg);
 int put_user_ifreq(struct ifreq *ifr, void __user *arg);
 int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr,
--- a/net/socket.c
+++ b/net/socket.c
@@ -1103,6 +1103,9 @@ static long sock_do_ioctl(struct net *ne
 	if (err != -ENOIOCTLCMD)
 		return err;
 
+	if (!is_socket_ioctl_cmd(cmd))
+		return -ENOTTY;
+
 	if (get_user_ifreq(&ifr, &data, argp))
 		return -EFAULT;
 	err = dev_ioctl(net, cmd, &ifr, data, &need_copyout);
@@ -3204,6 +3207,8 @@ static int compat_ifr_data_ioctl(struct
 	struct ifreq ifreq;
 	void __user *data;
 
+	if (!is_socket_ioctl_cmd(cmd))
+		return -ENOTTY;
 	if (get_user_ifreq(&ifreq, &data, u_ifreq32))
 		return -EFAULT;
 	ifreq.ifr_data = data;