From patchwork Tue Nov 24 15:18:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 331399 Delivered-To: patch@linaro.org Received: by 2002:a17:907:2110:0:0:0:0 with SMTP id qn16csp4456900ejb; Tue, 24 Nov 2020 07:20:18 -0800 (PST) X-Google-Smtp-Source: ABdhPJwrdbGsNjBLijer4+pbppblMBDuJjgk3WdHnLxEl0/9M68OkxPfYT8J9NAKRHrNMN/0qMCp X-Received: by 2002:aa7:c704:: with SMTP id i4mr4334435edq.51.1606231218678; Tue, 24 Nov 2020 07:20:18 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1606231218; cv=none; d=google.com; s=arc-20160816; b=0TMa3KnBRMO3jZVLm/8lg57cUQmApmu/JbcL4y4wLZFkRlv9uthMoKokkx/p8UPRBA 0jpcKVFaPVk/lbzUjqntthYsQwdeuaVOK+TBWBcstKbSU4SlstpHRKF/J0p1W08D5CZ6 X0kyDSYn+R7BRnrJ3dC9QI3OQ20Xo3/Ecm2/+cOx5Bhibodoj2AHRsvyPuP4m1SpIgJs GVraeP/pUPYKC6PbQYJEND+WMQ0vm1tXX2nb2PkE9SQyQACOytrNt87NVmyHcRTWaY8q SJpOqAuPXGUK97etgSERVs48FEHpX3KDcEgLTUeYFuExfMOCX5wIn6Kyekm/ng+FMlin QPKg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=FOBR/4Bs5+MIWnSykhcyheFHGut2Sj/YHpvRAdKHNfo=; b=YCBgRld76sNmX9m0qaKMVBxFNnQokzbZx47k15pJm4pnC+85KySJjaUnuxw8j/4+tg 03fO2lqsQ7WGHU9QxkdgBepVCz1cfJQl6NymGLSPQ9N2WUJvaVtwf2gm7sgC9U580ihr eIOh3dV7pufRLuwrw2sUxcrU84kl8M+Vt/yo/P0gc9J6FrDDtrbtzn2D1FCup5wbcaKc j4QX56gWTBrwcrcDJ37rYcUxe3hn+HGKgqycyWpqmB1p0rATkm5mV297rYr0aeNyHosm 6TtJ+Oyaxu6xMIwE6vxT5B+/FYcZzHofCwMBMfLoN2mRbHwH9TnAIk+uuSsoPEWhywwO i24A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=ENsn+raA; spf=pass (google.com: domain of netdev-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=netdev-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id b26si8980765edy.172.2020.11.24.07.20.18; Tue, 24 Nov 2020 07:20:18 -0800 (PST) Received-SPF: pass (google.com: domain of netdev-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=ENsn+raA; spf=pass (google.com: domain of netdev-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=netdev-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2389452AbgKXPSt (ORCPT + 8 others); Tue, 24 Nov 2020 10:18:49 -0500 Received: from mail.kernel.org ([198.145.29.99]:36498 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387697AbgKXPSs (ORCPT ); Tue, 24 Nov 2020 10:18:48 -0500 Received: from threadripper.lan (HSI-KBW-46-223-126-90.hsi.kabel-badenwuerttemberg.de [46.223.126.90]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 5637B206D5; Tue, 24 Nov 2020 15:18:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1606231127; bh=6CGs0fMLNY3x7sxLYsTt6iKTR6KheXOvnC5HRdMWN1A=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ENsn+raAkpDtR9h9MRDuzfK2n/ZY07nbd8fYPDKMDamUzPC+CYXCigi5R9CpjriOd 03pwvPQmN+SQmUYFBWymhqCF0p/3bSBAEG08qdkLBPCztmTtmQ4cYA3qCZZVgJHDic ZrgJdEAOF9pRM1E3CuvkUlXgmMg8tvVndHkT2KDw= From: Arnd Bergmann To: netdev@vger.kernel.org Cc: Arnd Bergmann Subject: [PATCH v4 4/4] net: socket: rework compat_ifreq_ioctl() Date: Tue, 24 Nov 2020 16:18:28 +0100 Message-Id: <20201124151828.169152-5-arnd@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20201124151828.169152-1-arnd@kernel.org> References: <20201124151828.169152-1-arnd@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Arnd Bergmann compat_ifreq_ioctl() is one of the last users of copy_in_user() and compat_alloc_user_space(), as it attempts to convert the 'struct ifreq' arguments from 32-bit to 64-bit format as used by dev_ioctl() and a couple of socket family specific interpretations. The current implementation works correctly when calling dev_ioctl(), inet_ioctl(), ieee802154_sock_ioctl(), atalk_ioctl(), qrtr_ioctl() and packet_ioctl(). The ioctl handlers for x25, netrom, rose and x25 do not interpret the arguments and only block the corresponding commands, so they do not care. For af_inet6 and af_decnet however, the compat conversion is slightly incorrect, as it will copy more data than the native handler accesses, both of them use a structure that is shorter than ifreq. Replace the copy_in_user() conversion with a pair of accessor functions to read and write the ifreq data in place with the correct length where needed, while leaving the other ones to copy the (already compatible) structures directly. Signed-off-by: Arnd Bergmann --- include/linux/compat.h | 53 ++++++++++---------- include/linux/netdevice.h | 2 + net/appletalk/ddp.c | 4 +- net/ieee802154/socket.c | 4 +- net/ipv4/af_inet.c | 6 +-- net/qrtr/qrtr.c | 4 +- net/socket.c | 100 ++++++++++++++++++++++++-------------- 7 files changed, 101 insertions(+), 72 deletions(-) -- 2.27.0 diff --git a/include/linux/compat.h b/include/linux/compat.h index a97f80b704ab..db722ba9ec58 100644 --- a/include/linux/compat.h +++ b/include/linux/compat.h @@ -110,6 +110,33 @@ struct compat_ifconf { compat_uptr_t ifcbuf; }; +struct compat_if_settings { + unsigned int type; /* Type of physical device or protocol */ + unsigned int size; /* Size of the data allocated by the caller */ + compat_uptr_t ifs_ifsu; /* union of pointers */ +}; + +struct compat_ifreq { + union { + char ifrn_name[IFNAMSIZ]; /* if name, e.g. "en0" */ + } ifr_ifrn; + union { + struct sockaddr ifru_addr; + struct sockaddr ifru_dstaddr; + struct sockaddr ifru_broadaddr; + struct sockaddr ifru_netmask; + struct sockaddr ifru_hwaddr; + short ifru_flags; + compat_int_t ifru_ivalue; + compat_int_t ifru_mtu; + struct compat_ifmap ifru_map; + char ifru_slave[IFNAMSIZ]; /* Just fits the size */ + char ifru_newname[IFNAMSIZ]; + compat_uptr_t ifru_data; + struct compat_if_settings ifru_settings; + } ifr_ifru; +}; + #ifdef CONFIG_COMPAT #ifndef compat_user_stack_pointer @@ -328,32 +355,6 @@ typedef struct compat_sigevent { } _sigev_un; } compat_sigevent_t; -struct compat_ifreq { - union { - char ifrn_name[IFNAMSIZ]; /* if name, e.g. "en0" */ - } ifr_ifrn; - union { - struct sockaddr ifru_addr; - struct sockaddr ifru_dstaddr; - struct sockaddr ifru_broadaddr; - struct sockaddr ifru_netmask; - struct sockaddr ifru_hwaddr; - short ifru_flags; - compat_int_t ifru_ivalue; - compat_int_t ifru_mtu; - struct compat_ifmap ifru_map; - char ifru_slave[IFNAMSIZ]; /* Just fits the size */ - char ifru_newname[IFNAMSIZ]; - compat_caddr_t ifru_data; - struct compat_if_settings ifru_settings; - } ifr_ifru; -}; - -struct compat_ifconf { - compat_int_t ifc_len; /* size of buffer */ - compat_caddr_t ifcbuf; -}; - struct compat_robust_list { compat_uptr_t next; }; diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index e99450a60d8e..c7935c3e5e05 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -3860,6 +3860,8 @@ 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); +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, bool *need_copyout); int dev_ifconf(struct net *net, struct ifconf __user *ifc); diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c index ca1a0d07a087..d598e2e57633 100644 --- a/net/appletalk/ddp.c +++ b/net/appletalk/ddp.c @@ -666,7 +666,7 @@ static int atif_ioctl(int cmd, void __user *arg) struct rtentry rtdef; int add_route; - if (copy_from_user(&atreq, arg, sizeof(atreq))) + if (get_user_ifreq(&atreq, NULL, arg)) return -EFAULT; dev = __dev_get_by_name(&init_net, atreq.ifr_name); @@ -865,7 +865,7 @@ static int atif_ioctl(int cmd, void __user *arg) return 0; } - return copy_to_user(arg, &atreq, sizeof(atreq)) ? -EFAULT : 0; + return put_user_ifreq(&atreq, arg); } static int atrtr_ioctl_addrt(struct rtentry *rt) diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c index a45a0401adc5..f5077de3619e 100644 --- a/net/ieee802154/socket.c +++ b/net/ieee802154/socket.c @@ -129,7 +129,7 @@ static int ieee802154_dev_ioctl(struct sock *sk, struct ifreq __user *arg, int ret = -ENOIOCTLCMD; struct net_device *dev; - if (copy_from_user(&ifr, arg, sizeof(struct ifreq))) + if (get_user_ifreq(&ifr, NULL, arg)) return -EFAULT; ifr.ifr_name[IFNAMSIZ-1] = 0; @@ -143,7 +143,7 @@ static int ieee802154_dev_ioctl(struct sock *sk, struct ifreq __user *arg, if (dev->type == ARPHRD_IEEE802154 && dev->netdev_ops->ndo_do_ioctl) ret = dev->netdev_ops->ndo_do_ioctl(dev, &ifr, cmd); - if (!ret && copy_to_user(arg, &ifr, sizeof(struct ifreq))) + if (!ret && put_user_ifreq(&ifr, arg)) ret = -EFAULT; dev_put(dev); diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index b7260c8cef2e..fb66d3f17a17 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -949,10 +949,10 @@ int inet_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg) case SIOCGIFNETMASK: case SIOCGIFDSTADDR: case SIOCGIFPFLAGS: - if (copy_from_user(&ifr, p, sizeof(struct ifreq))) + if (get_user_ifreq(&ifr, NULL, p)) return -EFAULT; err = devinet_ioctl(net, cmd, &ifr); - if (!err && copy_to_user(p, &ifr, sizeof(struct ifreq))) + if (!err && put_user_ifreq(&ifr, p)) err = -EFAULT; break; @@ -962,7 +962,7 @@ int inet_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg) case SIOCSIFDSTADDR: case SIOCSIFPFLAGS: case SIOCSIFFLAGS: - if (copy_from_user(&ifr, p, sizeof(struct ifreq))) + if (get_user_ifreq(&ifr, NULL, p)) return -EFAULT; err = devinet_ioctl(net, cmd, &ifr); break; diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c index f4ab3ca6d73b..ea56026457f7 100644 --- a/net/qrtr/qrtr.c +++ b/net/qrtr/qrtr.c @@ -1157,14 +1157,14 @@ static int qrtr_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg) rc = put_user(len, (int __user *)argp); break; case SIOCGIFADDR: - if (copy_from_user(&ifr, argp, sizeof(ifr))) { + if (get_user_ifreq(&ifr, NULL, argp)) { rc = -EFAULT; break; } sq = (struct sockaddr_qrtr *)&ifr.ifr_addr; *sq = ipc->us; - if (copy_to_user(argp, &ifr, sizeof(ifr))) { + if (put_user_ifreq(&ifr, argp)) { rc = -EFAULT; break; } diff --git a/net/socket.c b/net/socket.c index 41158e459f0b..4515c5f42bb0 100644 --- a/net/socket.c +++ b/net/socket.c @@ -3078,6 +3078,54 @@ void socket_seq_show(struct seq_file *seq) } #endif /* CONFIG_PROC_FS */ +/* Handle the fact that while struct ifreq has the same *layout* on + * 32/64 for everything but ifreq::ifru_ifmap and ifreq::ifru_data, + * which are handled elsewhere, it still has different *size* due to + * ifreq::ifru_ifmap (which is 16 bytes on 32 bit, 24 bytes on 64-bit, + * resulting in struct ifreq being 32 and 40 bytes respectively). + * As a result, if the struct happens to be at the end of a page and + * the next page isn't readable/writable, we get a fault. To prevent + * that, copy back and forth to the full size. + */ +int get_user_ifreq(struct ifreq *ifr, void __user **ifrdata, void __user *arg) +{ + if (in_compat_syscall()) { + struct compat_ifreq *ifr32 = (struct compat_ifreq *)ifr; + + memset(ifr, 0, sizeof(*ifr)); + if (copy_from_user(ifr32, arg, sizeof(*ifr32))) + return -EFAULT; + + if (ifrdata) + *ifrdata = compat_ptr(ifr32->ifr_data); + + return 0; + } + + if (copy_from_user(ifr, arg, sizeof(*ifr))) + return -EFAULT; + + if (ifrdata) + *ifrdata = ifr->ifr_data; + + return 0; +} +EXPORT_SYMBOL(get_user_ifreq); + +int put_user_ifreq(struct ifreq *ifr, void __user *arg) +{ + size_t size = sizeof(*ifr); + + if (in_compat_syscall()) + size = sizeof(struct compat_ifreq); + + if (copy_to_user(arg, ifr, size)) + return -EFAULT; + + return 0; +} +EXPORT_SYMBOL(put_user_ifreq); + #ifdef CONFIG_COMPAT static int compat_siocwandev(struct net *net, struct compat_ifreq __user *uifr32) { @@ -3086,7 +3134,7 @@ static int compat_siocwandev(struct net *net, struct compat_ifreq __user *uifr32 void __user *saved; int err; - if (copy_from_user(&ifr, uifr32, sizeof(struct compat_ifreq))) + if (get_user_ifreq(&ifr, NULL, uifr32)) return -EFAULT; if (get_user(uptr32, &uifr32->ifr_settings.ifs_ifsu)) @@ -3098,7 +3146,7 @@ static int compat_siocwandev(struct net *net, struct compat_ifreq __user *uifr32 err = dev_ioctl(net, SIOCWANDEV, &ifr, NULL); if (!err) { ifr.ifr_settings.ifs_ifsu.raw_hdlc = saved; - if (copy_to_user(uifr32, &ifr, sizeof(struct compat_ifreq))) + if (put_user_ifreq(&ifr, uifr32)) err = -EFAULT; } return err; @@ -3124,47 +3172,25 @@ static int compat_ifreq_ioctl(struct net *net, struct socket *sock, unsigned int cmd, struct compat_ifreq __user *uifr32) { - struct ifreq __user *uifr; + struct ifreq ifr; + bool need_copyout; int err; - /* Handle the fact that while struct ifreq has the same *layout* on - * 32/64 for everything but ifreq::ifru_ifmap and ifreq::ifru_data, - * which are handled elsewhere, it still has different *size* due to - * ifreq::ifru_ifmap (which is 16 bytes on 32 bit, 24 bytes on 64-bit, - * resulting in struct ifreq being 32 and 40 bytes respectively). - * As a result, if the struct happens to be at the end of a page and - * the next page isn't readable/writable, we get a fault. To prevent - * that, copy back and forth to the full size. + err = sock->ops->ioctl(sock, cmd, arg); + + /* If this ioctl is unknown try to hand it down + * to the NIC driver. */ + if (err != -ENOIOCTLCMD) + return err; - uifr = compat_alloc_user_space(sizeof(*uifr)); - if (copy_in_user(uifr, uifr32, sizeof(*uifr32))) + if (get_user_ifreq(&ifr, NULL, uifr32)) return -EFAULT; + err = dev_ioctl(net, cmd, &ifr, &need_copyout); + if (!err && need_copyout) + if (put_user_ifreq(&ifr, uifr32)) + return -EFAULT; - err = sock_do_ioctl(net, sock, cmd, (unsigned long)uifr); - - if (!err) { - switch (cmd) { - case SIOCGIFFLAGS: - case SIOCGIFMETRIC: - case SIOCGIFMTU: - case SIOCGIFMEM: - case SIOCGIFHWADDR: - case SIOCGIFINDEX: - case SIOCGIFADDR: - case SIOCGIFBRDADDR: - case SIOCGIFDSTADDR: - case SIOCGIFNETMASK: - case SIOCGIFPFLAGS: - case SIOCGIFTXQLEN: - case SIOCGMIIPHY: - case SIOCGMIIREG: - case SIOCGIFNAME: - if (copy_in_user(uifr32, uifr, sizeof(*uifr32))) - err = -EFAULT; - break; - } - } return err; }