On Wed, Aug 25, 2021 at 11:39 PM Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Aug 25, 2021 at 06:27:22PM -0700, Peter Collingbourne wrote:
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/I869da6cf6daabc3e4b7b82ac979683ba05... Signed-off-by: Peter Collingbourne pcc@google.com Cc: stable@vger.kernel.org # 4.19
include/linux/netdevice.h | 1 + net/core/dev_ioctl.c | 64 ++++++++++++++++++++++++++++++++------- net/socket.c | 6 +++- 3 files changed, 59 insertions(+), 12 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index eaf5bb008aa9..481b90ef0d32 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -4012,6 +4012,7 @@ int netdev_rx_handler_register(struct net_device *dev, void netdev_rx_handler_unregister(struct net_device *dev);
bool dev_valid_name(const char *name); +bool is_dev_ioctl_cmd(unsigned int cmd);
"is_socket_ioctl_cmd()" might be a better global name here.
SGTM, done in v2.
Peter