From patchwork Fri Nov 6 17:32: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: 320811 Delivered-To: patch@linaro.org Received: by 2002:a92:7b12:0:0:0:0:0 with SMTP id w18csp1488332ilc; Fri, 6 Nov 2020 09:32:54 -0800 (PST) X-Google-Smtp-Source: ABdhPJyiOXvAWI6/mmIAqSnHm7JkKacq0cKNu6T2ZLrcgFvuDdVyQAIu6BnpEwRC5jyTK8/mp9yq X-Received: by 2002:a17:906:85c1:: with SMTP id i1mr3169788ejy.157.1604683973829; Fri, 06 Nov 2020 09:32:53 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1604683973; cv=none; d=google.com; s=arc-20160816; b=C3Thgb8TZVFjiuVpbZoLXW3OoIF/pFalxED6n4ub6B9sUVagyeEuXQJD2pinweeE53 nZWMZj3E0lSA7G5L4352aShaE0Xv5HDt1z5ExJdI/k7y+V2jEv330lDCf4NuRcUoiBPn hslnpJbO4IWjQ4V7RnGW/2DVQP0xpSCTvRIZFQwSK5kQ1QdHL2iuQHZaifuiumBrkcXA NAcBKt5M6BfHFucRbXgDnYfraZX/MOysXSnf2GXeJqOal7CKgmp078ktai0rXp0/vrVQ QJuwsaJaXFZyF7eXXummn0XXXl4mgGJRPRB368r9CkLehKE9uCDgfSs3M+eM1uYIcQB0 l4vA== 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=BIleVjcJV595fhW6gpbg7PjlVOwSMZ44JOZjXMQ9D8U=; b=HEZe8U+oYYghQ1BTLmoSmLVyZC5LSLIvwnghoAmTjzlmnPyKy21Ra9mKYqso/hTO5W VgBhRE5DhzDVzVUakQ3FeUnnVDJTOyYRa0k3vM0xtghTyhu0SSpawHc1SE4pv78igcxz Mto3YDwxPlzV6VNJxfMjwsPlJBQcwIUsA63sRKDDf5/IkhUkDKY+4A//w+Eb1yXflsfh jp6aHCavPPx+EYJBjYbB4+3yl6f6OMa5h48cq+pXRJuAYI2KdYS9kFwHR2PPWzbLt0k5 BuiJYgBrHAz+kCMsFeJ35Vk4xZa0yhFycWM5L+3t4nSLzQCJ9Vu7vmY74YzD/NFRKV9Z 8HlQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=NRrvJvGV; 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 q19si1312747ejr.425.2020.11.06.09.32.53; Fri, 06 Nov 2020 09:32:53 -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=NRrvJvGV; 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 S1727650AbgKFRct (ORCPT + 8 others); Fri, 6 Nov 2020 12:32:49 -0500 Received: from mail.kernel.org ([198.145.29.99]:45012 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726880AbgKFRcs (ORCPT ); Fri, 6 Nov 2020 12:32:48 -0500 Received: from localhost.localdomain (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 966EB22203; Fri, 6 Nov 2020 17:32:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604683966; bh=4V1TMe5eCMQ3LCFhr/zC1TssRG3frQ13q52tyMWVSKY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=NRrvJvGV0Rr6B7pRErws7p1v0D5W3h+qRkz/kIxvnadISwXc3mcRlbdRYO4Uq7e1h d3K4DtHxwZJ5BWNmmuLLEXah0flmIOskiVFRk8n6kwui9guAke5bOgWvwDrmByKlVu mzBLcmmgidfvdGVHw89T8XfUgs9BmaKyM/CLwPIU= From: Arnd Bergmann To: netdev@vger.kernel.org Cc: Arnd Bergmann , "David S. Miller" , Jakub Kicinski , Christoph Hellwig , Jiri Pirko , Taehee Yoo , Eric Dumazet , Alexei Starovoitov , Andrew Lunn , linux-kernel@vger.kernel.org, Christoph Hellwig Subject: [PATCH net-next v3 1/4] ethtool: improve compat ioctl handling Date: Fri, 6 Nov 2020 18:32:28 +0100 Message-Id: <20201106173231.3031349-2-arnd@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20201106173231.3031349-1-arnd@kernel.org> References: <20201106173231.3031349-1-arnd@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Arnd Bergmann The ethtool compat ioctl handling is hidden away in net/socket.c, which introduces a couple of minor oddities: - The implementation may end up diverging, as seen in the RXNFC extension in commit 84a1d9c48200 ("net: ethtool: extend RXNFC API to support RSS spreading of filter matches") that does not work in compat mode. - Most architectures do not need the compat handling at all because u64 and compat_u64 have the same alignment. - On x86, the conversion is done for both x32 and i386 user space, but it's actually wrong to do it for x32 and cannot work there. - On 32-bit Arm, it never worked for compat oabi user space, since that needs to do the same conversion but does not. - It would be nice to get rid of both compat_alloc_user_space() and copy_in_user() throughout the kernel. None of these actually seems to be a serious problem that real users are likely to encounter, but fixing all of them actually leads to code that is both shorter and more readable. Signed-off-by: Arnd Bergmann Reviewed-by: Christoph Hellwig --- Changes in v2: - remove extraneous 'inline' keyword (davem) - split helper functions into smaller units (hch) - remove arm oabi check with missing dependency (0day bot) --- include/linux/ethtool.h | 4 -- net/ethtool/ioctl.c | 143 +++++++++++++++++++++++++++++++++++----- net/socket.c | 125 +---------------------------------- 3 files changed, 128 insertions(+), 144 deletions(-) -- 2.27.0 diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index 6408b446051f..b98291d391f3 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -17,8 +17,6 @@ #include #include -#ifdef CONFIG_COMPAT - struct compat_ethtool_rx_flow_spec { u32 flow_type; union ethtool_flow_union h_u; @@ -38,8 +36,6 @@ struct compat_ethtool_rxnfc { u32 rule_locs[]; }; -#endif /* CONFIG_COMPAT */ - #include /** diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index ec2cd7aab5ad..f571d390e774 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -7,6 +7,7 @@ * the information ethtool needs. */ +#include #include #include #include @@ -807,6 +808,127 @@ static noinline_for_stack int ethtool_get_sset_info(struct net_device *dev, return ret; } +static bool ethtool_translate_compat(void) +{ +#ifdef CONFIG_X86_64 + /* On x86, translation is needed for i386 but not x32 */ + return in_ia32_syscall(); +#else + BUILD_BUG_ON(sizeof(struct compat_ethtool_rxnfc) != + sizeof(struct ethtool_rxnfc)); +#endif + return 0; +} + +static int ethtool_rxnfc_copy_from_compat(struct ethtool_rxnfc *rxnfc, + const struct compat_ethtool_rxnfc __user *useraddr, + size_t size) +{ + struct compat_ethtool_rxnfc crxnfc = {}; + + /* We expect there to be holes between fs.m_ext and + * fs.ring_cookie and at the end of fs, but nowhere else. + */ + BUILD_BUG_ON(offsetof(struct compat_ethtool_rxnfc, fs.m_ext) + + sizeof(useraddr->fs.m_ext) != + offsetof(struct ethtool_rxnfc, fs.m_ext) + + sizeof(rxnfc->fs.m_ext)); + BUILD_BUG_ON(offsetof(struct compat_ethtool_rxnfc, fs.location) - + offsetof(struct compat_ethtool_rxnfc, fs.ring_cookie) != + offsetof(struct ethtool_rxnfc, fs.location) - + offsetof(struct ethtool_rxnfc, fs.ring_cookie)); + + if (copy_from_user(&crxnfc, useraddr, min(size, sizeof(crxnfc)))) + return -EFAULT; + + *rxnfc = (struct ethtool_rxnfc) { + .cmd = crxnfc.cmd, + .flow_type = crxnfc.flow_type, + .data = crxnfc.data, + .fs = { + .flow_type = crxnfc.fs.flow_type, + .h_u = crxnfc.fs.h_u, + .h_ext = crxnfc.fs.h_ext, + .m_u = crxnfc.fs.m_u, + .m_ext = crxnfc.fs.m_ext, + .ring_cookie = crxnfc.fs.ring_cookie, + .location = crxnfc.fs.location, + }, + .rule_cnt = crxnfc.rule_cnt, + }; + + return 0; +} + +static int ethtool_rxnfc_copy_from_user(struct ethtool_rxnfc *rxnfc, + const void __user *useraddr, size_t size) +{ + if (ethtool_translate_compat()) + return ethtool_rxnfc_copy_from_compat(rxnfc, useraddr, size); + + if (copy_from_user(&rxnfc, useraddr, size)) + return -EFAULT; + + return 0; +} + +static int ethtool_rxnfc_copy_to_compat(void __user *useraddr, + const struct ethtool_rxnfc *rxnfc, + size_t size, const u32 *rule_buf) +{ + + struct compat_ethtool_rxnfc crxnfc; + + memset(&crxnfc, 0, sizeof(crxnfc)); + crxnfc = (struct compat_ethtool_rxnfc) { + .cmd = rxnfc->cmd, + .flow_type = rxnfc->flow_type, + .data = rxnfc->data, + .fs = { + .flow_type = rxnfc->fs.flow_type, + .h_u = rxnfc->fs.h_u, + .h_ext = rxnfc->fs.h_ext, + .m_u = rxnfc->fs.m_u, + .m_ext = rxnfc->fs.m_ext, + .ring_cookie = rxnfc->fs.ring_cookie, + .location = rxnfc->fs.location, + }, + .rule_cnt = rxnfc->rule_cnt, + }; + + if (copy_to_user(useraddr, &crxnfc, min(size, sizeof(crxnfc)))) + return -EFAULT; + + return 0; +} + +static int ethtool_rxnfc_copy_to_user(void __user *useraddr, + const struct ethtool_rxnfc *rxnfc, + size_t size, const u32 *rule_buf) +{ + int ret; + + if (ethtool_translate_compat()) { + ret = ethtool_rxnfc_copy_to_compat(useraddr, rxnfc, size, + rule_buf); + useraddr += offsetof(struct compat_ethtool_rxnfc, rule_locs); + } else { + ret = copy_to_user(useraddr, &rxnfc, size); + useraddr += offsetof(struct ethtool_rxnfc, rule_locs); + } + + if (ret) + return -EFAULT; + + if (rule_buf) { + if (copy_to_user(useraddr, rule_buf, + rxnfc->rule_cnt * sizeof(u32))) + return -EFAULT; + } + + return 0; +} + static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev, u32 cmd, void __user *useraddr) { @@ -825,7 +947,7 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev, info_size = (offsetof(struct ethtool_rxnfc, data) + sizeof(info.data)); - if (copy_from_user(&info, useraddr, info_size)) + if (ethtool_rxnfc_copy_from_user(&info, useraddr, info_size)) return -EFAULT; rc = dev->ethtool_ops->set_rxnfc(dev, &info); @@ -833,7 +955,7 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev, return rc; if (cmd == ETHTOOL_SRXCLSRLINS && - copy_to_user(useraddr, &info, info_size)) + ethtool_rxnfc_copy_to_user(useraddr, &info, info_size, NULL)) return -EFAULT; return 0; @@ -859,7 +981,7 @@ static noinline_for_stack int ethtool_get_rxnfc(struct net_device *dev, info_size = (offsetof(struct ethtool_rxnfc, data) + sizeof(info.data)); - if (copy_from_user(&info, useraddr, info_size)) + if (ethtool_rxnfc_copy_from_user(&info, useraddr, info_size)) return -EFAULT; /* If FLOW_RSS was requested then user-space must be using the @@ -867,7 +989,7 @@ static noinline_for_stack int ethtool_get_rxnfc(struct net_device *dev, */ if (cmd == ETHTOOL_GRXFH && info.flow_type & FLOW_RSS) { info_size = sizeof(info); - if (copy_from_user(&info, useraddr, info_size)) + if (ethtool_rxnfc_copy_from_user(&info, useraddr, info_size)) return -EFAULT; /* Since malicious users may modify the original data, * we need to check whether FLOW_RSS is still requested. @@ -893,18 +1015,7 @@ static noinline_for_stack int ethtool_get_rxnfc(struct net_device *dev, if (ret < 0) goto err_out; - ret = -EFAULT; - if (copy_to_user(useraddr, &info, info_size)) - goto err_out; - - if (rule_buf) { - useraddr += offsetof(struct ethtool_rxnfc, rule_locs); - if (copy_to_user(useraddr, rule_buf, - info.rule_cnt * sizeof(u32))) - goto err_out; - } - ret = 0; - + ret = ethtool_rxnfc_copy_to_user(useraddr, &info, info_size, rule_buf); err_out: kfree(rule_buf); diff --git a/net/socket.c b/net/socket.c index 6e6cccc2104f..1670bc86a53c 100644 --- a/net/socket.c +++ b/net/socket.c @@ -3123,128 +3123,6 @@ static int compat_dev_ifconf(struct net *net, struct compat_ifconf __user *uifc3 return 0; } -static int ethtool_ioctl(struct net *net, struct compat_ifreq __user *ifr32) -{ - struct compat_ethtool_rxnfc __user *compat_rxnfc; - bool convert_in = false, convert_out = false; - size_t buf_size = 0; - struct ethtool_rxnfc __user *rxnfc = NULL; - struct ifreq ifr; - u32 rule_cnt = 0, actual_rule_cnt; - u32 ethcmd; - u32 data; - int ret; - - if (get_user(data, &ifr32->ifr_ifru.ifru_data)) - return -EFAULT; - - compat_rxnfc = compat_ptr(data); - - if (get_user(ethcmd, &compat_rxnfc->cmd)) - return -EFAULT; - - /* Most ethtool structures are defined without padding. - * Unfortunately struct ethtool_rxnfc is an exception. - */ - switch (ethcmd) { - default: - break; - case ETHTOOL_GRXCLSRLALL: - /* Buffer size is variable */ - if (get_user(rule_cnt, &compat_rxnfc->rule_cnt)) - return -EFAULT; - if (rule_cnt > KMALLOC_MAX_SIZE / sizeof(u32)) - return -ENOMEM; - buf_size += rule_cnt * sizeof(u32); - fallthrough; - case ETHTOOL_GRXRINGS: - case ETHTOOL_GRXCLSRLCNT: - case ETHTOOL_GRXCLSRULE: - case ETHTOOL_SRXCLSRLINS: - convert_out = true; - fallthrough; - case ETHTOOL_SRXCLSRLDEL: - buf_size += sizeof(struct ethtool_rxnfc); - convert_in = true; - rxnfc = compat_alloc_user_space(buf_size); - break; - } - - if (copy_from_user(&ifr.ifr_name, &ifr32->ifr_name, IFNAMSIZ)) - return -EFAULT; - - ifr.ifr_data = convert_in ? rxnfc : (void __user *)compat_rxnfc; - - if (convert_in) { - /* We expect there to be holes between fs.m_ext and - * fs.ring_cookie and at the end of fs, but nowhere else. - */ - BUILD_BUG_ON(offsetof(struct compat_ethtool_rxnfc, fs.m_ext) + - sizeof(compat_rxnfc->fs.m_ext) != - offsetof(struct ethtool_rxnfc, fs.m_ext) + - sizeof(rxnfc->fs.m_ext)); - BUILD_BUG_ON( - offsetof(struct compat_ethtool_rxnfc, fs.location) - - offsetof(struct compat_ethtool_rxnfc, fs.ring_cookie) != - offsetof(struct ethtool_rxnfc, fs.location) - - offsetof(struct ethtool_rxnfc, fs.ring_cookie)); - - if (copy_in_user(rxnfc, compat_rxnfc, - (void __user *)(&rxnfc->fs.m_ext + 1) - - (void __user *)rxnfc) || - copy_in_user(&rxnfc->fs.ring_cookie, - &compat_rxnfc->fs.ring_cookie, - (void __user *)(&rxnfc->fs.location + 1) - - (void __user *)&rxnfc->fs.ring_cookie)) - return -EFAULT; - if (ethcmd == ETHTOOL_GRXCLSRLALL) { - if (put_user(rule_cnt, &rxnfc->rule_cnt)) - return -EFAULT; - } else if (copy_in_user(&rxnfc->rule_cnt, - &compat_rxnfc->rule_cnt, - sizeof(rxnfc->rule_cnt))) - return -EFAULT; - } - - ret = dev_ioctl(net, SIOCETHTOOL, &ifr, NULL); - if (ret) - return ret; - - if (convert_out) { - if (copy_in_user(compat_rxnfc, rxnfc, - (const void __user *)(&rxnfc->fs.m_ext + 1) - - (const void __user *)rxnfc) || - copy_in_user(&compat_rxnfc->fs.ring_cookie, - &rxnfc->fs.ring_cookie, - (const void __user *)(&rxnfc->fs.location + 1) - - (const void __user *)&rxnfc->fs.ring_cookie) || - copy_in_user(&compat_rxnfc->rule_cnt, &rxnfc->rule_cnt, - sizeof(rxnfc->rule_cnt))) - return -EFAULT; - - if (ethcmd == ETHTOOL_GRXCLSRLALL) { - /* As an optimisation, we only copy the actual - * number of rules that the underlying - * function returned. Since Mallory might - * change the rule count in user memory, we - * check that it is less than the rule count - * originally given (as the user buffer size), - * which has been range-checked. - */ - if (get_user(actual_rule_cnt, &rxnfc->rule_cnt)) - return -EFAULT; - if (actual_rule_cnt < rule_cnt) - rule_cnt = actual_rule_cnt; - if (copy_in_user(&compat_rxnfc->rule_locs[0], - &rxnfc->rule_locs[0], - rule_cnt * sizeof(u32))) - return -EFAULT; - } - } - - return 0; -} - static int compat_siocwandev(struct net *net, struct compat_ifreq __user *uifr32) { compat_uptr_t uptr32; @@ -3399,8 +3277,6 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, return old_bridge_ioctl(argp); case SIOCGIFCONF: return compat_dev_ifconf(net, argp); - case SIOCETHTOOL: - return ethtool_ioctl(net, argp); case SIOCWANDEV: return compat_siocwandev(net, argp); case SIOCGIFMAP: @@ -3413,6 +3289,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, return sock->ops->gettstamp(sock, argp, cmd == SIOCGSTAMP_OLD, !COMPAT_USE_64BIT_TIME); + case SIOCETHTOOL: case SIOCBONDSLAVEINFOQUERY: case SIOCBONDINFOQUERY: case SIOCSHWTSTAMP: From patchwork Fri Nov 6 17:32:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 320812 Delivered-To: patch@linaro.org Received: by 2002:a92:7b12:0:0:0:0:0 with SMTP id w18csp1488429ilc; Fri, 6 Nov 2020 09:33:00 -0800 (PST) X-Google-Smtp-Source: ABdhPJxjQQ0iuSlhY1eH4dyTic8Z9XXZ6ICTVx1d9UV8oWT8dQiEiXW9H6E1aPOKojnLTjM40fOI X-Received: by 2002:a17:906:1f13:: with SMTP id w19mr3046470ejj.39.1604683980086; Fri, 06 Nov 2020 09:33:00 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1604683980; cv=none; d=google.com; s=arc-20160816; b=KebytY0f6NLjMraf+OM45h7h4nP0d5AJzo6nPuSoNh86bPSklTYfv+CeTB2COrSFwO 0aqAf70aH4HkrolRx1aq1JKRsc1AnDMKQtovktsGEeiWEqKAmrHW9GgcK7OAxjPWuIMR 28XjIN56tHar6x6mFFW2aXbdbczLj8l+3Htjk7y7WeFeWpZeL+Jq7TpVJtQ0/cY/Udam HYbL9s1JWET0TyvTPZ+YtRVMofsPVZX2t7C0fLWdcQxCUmUvJb+UcEmBhabM5t7EgAh/ sr5M5D6YBzj7TInnIF7/WUvOGWXVi66UtE1/UaK2poIH0ywQW7NvyJsXIlAEAf1gV3F8 2Msg== 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=IWZLCOatoqmQpE9BWlYDZbfpqd4VmpKg+FioZjh79Ls=; b=myzFBwPfPrYptDjYGta50k0Q9ENDVN+vWTgMyhpvCzR1WwCKieqfighspmCZx4Ibj+ t0Qb4gCVM7rUM2FuPDCwhEtmUxAoHnB332qQxEafQYtwS7iX3HXKvarpp2fOBMOZo0Ph lC241c8jk2G5RPiKQXqCjyuOk2R78OFe7QD+tQwp+A0EeubL9CBUpj7iLsmSvDx2Yak0 gxqYz8R24bAft7kD3OL2MDBhF9qwJw04UFbv9vBX8RgmQbs/k/P5pYuMg+cM6zPzLrtz VLH8MQhMe8wyn7vZwf3XZEd4QsIh0uZ/I2X3KShe1A9CeiUMSiJJYFx2tCidCyJuILRK uLPQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=p5ytTdpG; 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 k20si1212333ejj.93.2020.11.06.09.32.59; Fri, 06 Nov 2020 09:33:00 -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=p5ytTdpG; 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 S1727745AbgKFRc4 (ORCPT + 8 others); Fri, 6 Nov 2020 12:32:56 -0500 Received: from mail.kernel.org ([198.145.29.99]:45090 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727610AbgKFRct (ORCPT ); Fri, 6 Nov 2020 12:32:49 -0500 Received: from localhost.localdomain (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 6535B208C7; Fri, 6 Nov 2020 17:32:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604683968; bh=ffEVquCSMWpRc6D61TaaU+vK0F4lMUvh6PRi0Z3P3e8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=p5ytTdpGsTl2aRiL4J/Cnt5cYLRLIxiaufNj6O8Ltgw3MBlbK3YUP93XPSqPoXzRU 9+i47NeCGzxOAJjnMZOP3UacG3HYlY4U/B2+eRo13/B4IwoIFICePXc+XpPpF7KKFV MYU3II+9N4/5LEqtCyTdjfJ6B2r7fHsl9AhOE5Os= From: Arnd Bergmann To: netdev@vger.kernel.org Cc: Arnd Bergmann , "David S. Miller" , Jakub Kicinski , Christoph Hellwig , Jiri Pirko , Taehee Yoo , Eric Dumazet , Alexei Starovoitov , Andrew Lunn , linux-kernel@vger.kernel.org Subject: [PATCH net-next v3 2/4] net: socket: rework SIOC?IFMAP ioctls Date: Fri, 6 Nov 2020 18:32:29 +0100 Message-Id: <20201106173231.3031349-3-arnd@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20201106173231.3031349-1-arnd@kernel.org> References: <20201106173231.3031349-1-arnd@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Arnd Bergmann SIOCGIFMAP and SIOCSIFMAP currently require compat_alloc_user_space() and copy_in_user() for compat mode. Move the compat handling into the location where the structures are actually used, to avoid using those interfaces and get a clearer implementation. Signed-off-by: Arnd Bergmann --- changes in v3: - complete rewrite changes in v2: - fix building with CONFIG_COMPAT disabled (0day bot) - split up dev_ifmap() into more readable helpers (hch) - move rcu_read_unlock() for readability (hch) --- include/linux/compat.h | 18 ++++++------ net/core/dev_ioctl.c | 64 +++++++++++++++++++++++++++++++++--------- net/socket.c | 39 ++----------------------- 3 files changed, 62 insertions(+), 59 deletions(-) -- 2.27.0 diff --git a/include/linux/compat.h b/include/linux/compat.h index 08dbd34bb7a5..47496c5eb5eb 100644 --- a/include/linux/compat.h +++ b/include/linux/compat.h @@ -96,6 +96,15 @@ struct compat_iovec { compat_size_t iov_len; }; +struct compat_ifmap { + compat_ulong_t mem_start; + compat_ulong_t mem_end; + unsigned short base_addr; + unsigned char irq; + unsigned char dma; + unsigned char port; +}; + #ifdef CONFIG_COMPAT #ifndef compat_user_stack_pointer @@ -314,15 +323,6 @@ typedef struct compat_sigevent { } _sigev_un; } compat_sigevent_t; -struct compat_ifmap { - compat_ulong_t mem_start; - compat_ulong_t mem_end; - unsigned short base_addr; - unsigned char irq; - unsigned char dma; - unsigned char port; -}; - struct compat_if_settings { unsigned int type; /* Type of physical device or protocol */ unsigned int size; /* Size of the data allocated by the caller */ diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c index 205e92e604ef..1c6da044754a 100644 --- a/net/core/dev_ioctl.c +++ b/net/core/dev_ioctl.c @@ -98,6 +98,55 @@ int dev_ifconf(struct net *net, struct ifconf *ifc, int size) return 0; } +static int dev_getifmap(struct net_device *dev, struct ifreq *ifr) +{ + struct ifmap *ifmap = &ifr->ifr_map; + struct compat_ifmap *cifmap = (struct compat_ifmap *)&ifr->ifr_map; + + if (in_compat_syscall()) { + cifmap->mem_start = dev->mem_start; + cifmap->mem_end = dev->mem_end; + cifmap->base_addr = dev->base_addr; + cifmap->irq = dev->irq; + cifmap->dma = dev->dma; + cifmap->port = dev->if_port; + + return 0; + } + + ifmap->mem_start = dev->mem_start; + ifmap->mem_end = dev->mem_end; + ifmap->base_addr = dev->base_addr; + ifmap->irq = dev->irq; + ifmap->dma = dev->dma; + ifmap->port = dev->if_port; + + return 0; +} + +static int dev_setifmap(struct net_device *dev, struct ifreq *ifr) +{ + struct compat_ifmap *cifmap = (struct compat_ifmap *)&ifr->ifr_map; + + if (!dev->netdev_ops->ndo_set_config) + return -EOPNOTSUPP; + + if (in_compat_syscall()) { + struct ifmap ifmap = { + .mem_start = cifmap->mem_start, + .mem_end = cifmap->mem_end, + .base_addr = cifmap->base_addr, + .irq = cifmap->irq, + .dma = cifmap->dma, + .port = cifmap->port, + }; + + return dev->netdev_ops->ndo_set_config(dev, &ifmap); + } + + return dev->netdev_ops->ndo_set_config(dev, &ifr->ifr_map); +} + /* * Perform the SIOCxIFxxx calls, inside rcu_read_lock() */ @@ -139,13 +188,7 @@ static int dev_ifsioc_locked(struct net *net, struct ifreq *ifr, unsigned int cm break; case SIOCGIFMAP: - ifr->ifr_map.mem_start = dev->mem_start; - ifr->ifr_map.mem_end = dev->mem_end; - ifr->ifr_map.base_addr = dev->base_addr; - ifr->ifr_map.irq = dev->irq; - ifr->ifr_map.dma = dev->dma; - ifr->ifr_map.port = dev->if_port; - return 0; + return dev_getifmap(dev, ifr); case SIOCGIFINDEX: ifr->ifr_ifindex = dev->ifindex; @@ -286,12 +329,7 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd) return 0; case SIOCSIFMAP: - if (ops->ndo_set_config) { - if (!netif_device_present(dev)) - return -ENODEV; - return ops->ndo_set_config(dev, &ifr->ifr_map); - } - return -EOPNOTSUPP; + return dev_setifmap(dev, ifr); case SIOCADDMULTI: if (!ops->ndo_set_rx_mode || diff --git a/net/socket.c b/net/socket.c index 1670bc86a53c..2b58b1d87cad 100644 --- a/net/socket.c +++ b/net/socket.c @@ -3212,40 +3212,6 @@ static int compat_ifreq_ioctl(struct net *net, struct socket *sock, return err; } -static int compat_sioc_ifmap(struct net *net, unsigned int cmd, - struct compat_ifreq __user *uifr32) -{ - struct ifreq ifr; - struct compat_ifmap __user *uifmap32; - int err; - - uifmap32 = &uifr32->ifr_ifru.ifru_map; - err = copy_from_user(&ifr, uifr32, sizeof(ifr.ifr_name)); - err |= get_user(ifr.ifr_map.mem_start, &uifmap32->mem_start); - err |= get_user(ifr.ifr_map.mem_end, &uifmap32->mem_end); - err |= get_user(ifr.ifr_map.base_addr, &uifmap32->base_addr); - err |= get_user(ifr.ifr_map.irq, &uifmap32->irq); - err |= get_user(ifr.ifr_map.dma, &uifmap32->dma); - err |= get_user(ifr.ifr_map.port, &uifmap32->port); - if (err) - return -EFAULT; - - err = dev_ioctl(net, cmd, &ifr, NULL); - - if (cmd == SIOCGIFMAP && !err) { - err = copy_to_user(uifr32, &ifr, sizeof(ifr.ifr_name)); - err |= put_user(ifr.ifr_map.mem_start, &uifmap32->mem_start); - err |= put_user(ifr.ifr_map.mem_end, &uifmap32->mem_end); - err |= put_user(ifr.ifr_map.base_addr, &uifmap32->base_addr); - err |= put_user(ifr.ifr_map.irq, &uifmap32->irq); - err |= put_user(ifr.ifr_map.dma, &uifmap32->dma); - err |= put_user(ifr.ifr_map.port, &uifmap32->port); - if (err) - err = -EFAULT; - } - return err; -} - /* Since old style bridge ioctl's endup using SIOCDEVPRIVATE * for some operations; this forces use of the newer bridge-utils that * use compatible ioctls @@ -3279,9 +3245,6 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, return compat_dev_ifconf(net, argp); case SIOCWANDEV: return compat_siocwandev(net, argp); - case SIOCGIFMAP: - case SIOCSIFMAP: - return compat_sioc_ifmap(net, cmd, argp); case SIOCGSTAMP_OLD: case SIOCGSTAMPNS_OLD: if (!sock->ops->gettstamp) @@ -3313,6 +3276,8 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, case SIOCGIFFLAGS: case SIOCSIFFLAGS: + case SIOCGIFMAP: + case SIOCSIFMAP: case SIOCGIFMETRIC: case SIOCSIFMETRIC: case SIOCGIFMTU: From patchwork Fri Nov 6 17:32:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 320814 Delivered-To: patch@linaro.org Received: by 2002:a92:7b12:0:0:0:0:0 with SMTP id w18csp1488597ilc; Fri, 6 Nov 2020 09:33:10 -0800 (PST) X-Google-Smtp-Source: ABdhPJwSjJPpcelceqVJH4g6QOgs9OCWqyL7elKuUSVj+ozhS/7+eFUS8Akr158o5h4a2tjq7XjC X-Received: by 2002:aa7:c2c3:: with SMTP id m3mr3128900edp.361.1604683990277; Fri, 06 Nov 2020 09:33:10 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1604683990; cv=none; d=google.com; s=arc-20160816; b=AMwKErTtabnCVJy7IPu8oB06CqcNapJpvGq6ZRY87SVTRKI5UZhNbvD++alZiW770G aWlXScG60YFgzyuq4YraK114mMhvlBG64PCYw0YK2XEEgXVUZoRweT3UiJqNuqAzv0L6 LCDiJNnCAbVcLqo9iVImXFoHUGDkB+FDKNVNRyaJ2dIC+CHZh/7RB5hqNjc9BiHSYh6N qZGq6+kfsZi1fOchv+2qkbJ3ZM0nDRfLbprnuFfNlteOYqq29SwZA5UyrMx/Au3JQxJA VEcfkLDCcS5XVgQMhP/dIhpyNcvmRKz3Ipg7vWpnJu0+ZvZmZTDUrf7XmkrSh/ApfueY 2PGw== 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=AIIY3qanZaEzRdH4M2JnTK1YGG8Dwvz/rsr4s1MmwX8=; b=RsfESGtGFx/tOFeW+qXDI7ierha5qRyaoI6c80n/mJePJu0jjbNqp7TW1/syoo6+Pl L3N+nr66K9zW9em3WO34XdCHy9sOn8FwbzY9qI6EIPr7abIz+05E4UyMQt2o55uI9ma5 ToI1UlxXSYjPa7f4jkGnYAlEYgViYWkxgCSSGgEC2xyqFmS1BrriJJPW9ICtxvnsDmGG BTw64M+F61L5PB02lq5zQDIF8vAx1bPZtjHE36G9OIkzHb+s6eAuM06yXVKy8aldrVA0 BOvQaiKnd7oCMc46+pl5FJpdnoa8fg6rV8BPAwwsXVQSQe4MQProLGb2HKXXPjmf29P6 /Lbw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=Z0nJsLaq; 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 k20si1212333ejj.93.2020.11.06.09.33.09; Fri, 06 Nov 2020 09:33:10 -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=Z0nJsLaq; 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 S1727722AbgKFRcz (ORCPT + 8 others); Fri, 6 Nov 2020 12:32:55 -0500 Received: from mail.kernel.org ([198.145.29.99]:45158 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727674AbgKFRcw (ORCPT ); Fri, 6 Nov 2020 12:32:52 -0500 Received: from localhost.localdomain (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 0F1B52151B; Fri, 6 Nov 2020 17:32:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604683971; bh=yEza5hTzjZgoh6zJHNuUl5eqkfjxO5JNXXNYliXmprw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Z0nJsLaqRdODljru/MdCAClQc4u6miA5F8vH9JiRrL1/S+JfZXgV5CFyR5staPRrf KEsmfmewdolwHjtNmiAJVGklIpDlshwORotCCOfOucSQQytFA0kdhSVZoBaXQRxOxM UjcxZqhZ8W2NJRpgx9nO33nuFT0hetEdEfo+k5EA= From: Arnd Bergmann To: netdev@vger.kernel.org Cc: Arnd Bergmann , "David S. Miller" , Jakub Kicinski , Christoph Hellwig , Jiri Pirko , Taehee Yoo , Eric Dumazet , Alexei Starovoitov , Andrew Lunn , linux-kernel@vger.kernel.org Subject: [PATCH net-next v3 3/4] net: socket: simplify dev_ifconf handling Date: Fri, 6 Nov 2020 18:32:30 +0100 Message-Id: <20201106173231.3031349-4-arnd@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20201106173231.3031349-1-arnd@kernel.org> References: <20201106173231.3031349-1-arnd@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Arnd Bergmann The dev_ifconf() calling conventions make compat handling more complicated than necessary, simplify this by moving the in_compat_syscall() check into the function. The implementation can be simplified further, based on the knowledge that the dynamic registration is only ever used for IPv4. Signed-off-by: Arnd Bergmann --- include/linux/inetdevice.h | 8 ++++ include/linux/netdevice.h | 10 +---- net/core/dev_ioctl.c | 92 +++++++++++++++----------------------- net/ipv4/devinet.c | 4 +- net/socket.c | 59 ++++++------------------ 5 files changed, 60 insertions(+), 113 deletions(-) -- 2.27.0 diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h index 3515ca64e638..e8a957ca80f8 100644 --- a/include/linux/inetdevice.h +++ b/include/linux/inetdevice.h @@ -178,6 +178,14 @@ static inline struct net_device *ip_dev_find(struct net *net, __be32 addr) int inet_addr_onlink(struct in_device *in_dev, __be32 a, __be32 b); int devinet_ioctl(struct net *net, unsigned int cmd, struct ifreq *); +#ifdef CONFIG_INET +int inet_gifconf(struct net_device *dev, char __user *buf, int len, int size); +#else +static inline inet_gifconf(struct net_device *dev, char __user *buf, int len, int size) +{ + return 0; +} +#endif void devinet_init(void); struct in_device *inetdev_by_index(struct net *, int); __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope); diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 964b494b0e8d..b54d5308087d 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -3137,14 +3137,6 @@ static inline bool dev_validate_header(const struct net_device *dev, return false; } -typedef int gifconf_func_t(struct net_device * dev, char __user * bufptr, - int len, int size); -int register_gifconf(unsigned int family, gifconf_func_t *gifconf); -static inline int unregister_gifconf(unsigned int family) -{ - return register_gifconf(family, NULL); -} - #ifdef CONFIG_NET_FLOW_LIMIT #define FLOW_LIMIT_HISTORY (1 << 7) /* must be ^2 and !overflow buckets */ struct sd_flow_limit { @@ -3837,7 +3829,7 @@ void netdev_rx_handler_unregister(struct net_device *dev); bool dev_valid_name(const char *name); int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr, bool *need_copyout); -int dev_ifconf(struct net *net, struct ifconf *, int); +int dev_ifconf(struct net *net, struct ifconf __user *ifc); int dev_ethtool(struct net *net, struct ifreq *); unsigned int dev_get_flags(const struct net_device *); int __dev_change_flags(struct net_device *dev, unsigned int flags, diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c index 1c6da044754a..5822737a78f4 100644 --- a/net/core/dev_ioctl.c +++ b/net/core/dev_ioctl.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #include #include +#include #include #include #include @@ -25,77 +26,56 @@ static int dev_ifname(struct net *net, struct ifreq *ifr) return netdev_get_name(net, ifr->ifr_name, ifr->ifr_ifindex); } -static gifconf_func_t *gifconf_list[NPROTO]; - -/** - * register_gifconf - register a SIOCGIF handler - * @family: Address family - * @gifconf: Function handler - * - * Register protocol dependent address dumping routines. The handler - * that is passed must not be freed or reused until it has been replaced - * by another handler. - */ -int register_gifconf(unsigned int family, gifconf_func_t *gifconf) -{ - if (family >= NPROTO) - return -EINVAL; - gifconf_list[family] = gifconf; - return 0; -} -EXPORT_SYMBOL(register_gifconf); - /* * Perform a SIOCGIFCONF call. This structure will change * size eventually, and there is nothing I can do about it. * Thus we will need a 'compatibility mode'. */ - -int dev_ifconf(struct net *net, struct ifconf *ifc, int size) +int dev_ifconf(struct net *net, struct ifconf __user *uifc) { struct net_device *dev; - char __user *pos; - int len; - int total; - int i; + void __user *pos; + size_t size; + int len, total = 0, done; - /* - * Fetch the caller's info block. - */ + /* both the ifconf and the ifreq structures are slightly different */ + if (in_compat_syscall()) { + struct compat_ifconf ifc32; - pos = ifc->ifc_buf; - len = ifc->ifc_len; + if (copy_from_user(&ifc32, uifc, sizeof(struct compat_ifconf))) + return -EFAULT; - /* - * Loop over the interfaces, and write an info block for each. - */ + pos = compat_ptr(ifc32.ifcbuf); + len = ifc32.ifc_len; + size = sizeof(struct compat_ifreq); + } else { + struct ifconf ifc; - total = 0; + if (copy_from_user(&ifc, uifc, sizeof(struct ifconf))) + return -EFAULT; + + pos = ifc.ifc_buf; + len = ifc.ifc_len; + size = sizeof(struct ifreq); + } + + /* Loop over the interfaces, and write an info block for each. */ + rtnl_lock(); for_each_netdev(net, dev) { - for (i = 0; i < NPROTO; i++) { - if (gifconf_list[i]) { - int done; - if (!pos) - done = gifconf_list[i](dev, NULL, 0, size); - else - done = gifconf_list[i](dev, pos + total, - len - total, size); - if (done < 0) - return -EFAULT; - total += done; - } + if (!pos) + done = inet_gifconf(dev, NULL, 0, size); + else + done = inet_gifconf(dev, pos + total, + len - total, size); + if (done < 0) { + rtnl_unlock(); + return -EFAULT; } + total += done; } + rtnl_unlock(); - /* - * All done. Write the updated control block back to the caller. - */ - ifc->ifc_len = total; - - /* - * Both BSD and Solaris return 0 here, so we do too. - */ - return 0; + return put_user(total, &uifc->ifc_len); } static int dev_getifmap(struct net_device *dev, struct ifreq *ifr) diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index 123a6d39438f..3e910dedcb80 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -1244,7 +1244,7 @@ int devinet_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr) return ret; } -static int inet_gifconf(struct net_device *dev, char __user *buf, int len, int size) +int inet_gifconf(struct net_device *dev, char __user *buf, int len, int size) { struct in_device *in_dev = __in_dev_get_rtnl(dev); const struct in_ifaddr *ifa; @@ -2762,8 +2762,6 @@ void __init devinet_init(void) INIT_HLIST_HEAD(&inet_addr_lst[i]); register_pernet_subsys(&devinet_ops); - - register_gifconf(PF_INET, inet_gifconf); register_netdevice_notifier(&ip_netdev_notifier); queue_delayed_work(system_power_efficient_wq, &check_lifetime_work, 0); diff --git a/net/socket.c b/net/socket.c index 2b58b1d87cad..f26ef299b6c5 100644 --- a/net/socket.c +++ b/net/socket.c @@ -1041,6 +1041,8 @@ EXPORT_SYMBOL(dlci_ioctl_set); static long sock_do_ioctl(struct net *net, struct socket *sock, unsigned int cmd, unsigned long arg) { + struct ifreq ifr; + bool need_copyout; int err; void __user *argp = (void __user *)arg; @@ -1053,25 +1055,13 @@ static long sock_do_ioctl(struct net *net, struct socket *sock, if (err != -ENOIOCTLCMD) return err; - if (cmd == SIOCGIFCONF) { - struct ifconf ifc; - if (copy_from_user(&ifc, argp, sizeof(struct ifconf))) - return -EFAULT; - rtnl_lock(); - err = dev_ifconf(net, &ifc, sizeof(struct ifreq)); - rtnl_unlock(); - if (!err && copy_to_user(argp, &ifc, sizeof(struct ifconf))) - err = -EFAULT; - } else { - struct ifreq ifr; - bool need_copyout; - if (copy_from_user(&ifr, argp, sizeof(struct ifreq))) + if (copy_from_user(&ifr, argp, sizeof(struct ifreq))) + return -EFAULT; + err = dev_ioctl(net, cmd, &ifr, &need_copyout); + if (!err && need_copyout) + if (copy_to_user(argp, &ifr, sizeof(struct ifreq))) return -EFAULT; - err = dev_ioctl(net, cmd, &ifr, &need_copyout); - if (!err && need_copyout) - if (copy_to_user(argp, &ifr, sizeof(struct ifreq))) - return -EFAULT; - } + return err; } @@ -1194,6 +1184,11 @@ static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg) cmd == SIOCGSTAMP_NEW, false); break; + + case SIOCGIFCONF: + err = dev_ifconf(net, argp); + break; + default: err = sock_do_ioctl(net, sock, cmd, arg); break; @@ -3098,31 +3093,6 @@ void socket_seq_show(struct seq_file *seq) #endif /* CONFIG_PROC_FS */ #ifdef CONFIG_COMPAT -static int compat_dev_ifconf(struct net *net, struct compat_ifconf __user *uifc32) -{ - struct compat_ifconf ifc32; - struct ifconf ifc; - int err; - - if (copy_from_user(&ifc32, uifc32, sizeof(struct compat_ifconf))) - return -EFAULT; - - ifc.ifc_len = ifc32.ifc_len; - ifc.ifc_req = compat_ptr(ifc32.ifcbuf); - - rtnl_lock(); - err = dev_ifconf(net, &ifc, sizeof(struct compat_ifreq)); - rtnl_unlock(); - if (err) - return err; - - ifc32.ifc_len = ifc.ifc_len; - if (copy_to_user(uifc32, &ifc32, sizeof(struct compat_ifconf))) - return -EFAULT; - - return 0; -} - static int compat_siocwandev(struct net *net, struct compat_ifreq __user *uifr32) { compat_uptr_t uptr32; @@ -3241,8 +3211,6 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, case SIOCSIFBR: case SIOCGIFBR: return old_bridge_ioctl(argp); - case SIOCGIFCONF: - return compat_dev_ifconf(net, argp); case SIOCWANDEV: return compat_siocwandev(net, argp); case SIOCGSTAMP_OLD: @@ -3272,6 +3240,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, case SIOCGSKNS: case SIOCGSTAMP_NEW: case SIOCGSTAMPNS_NEW: + case SIOCGIFCONF: return sock_ioctl(file, cmd, arg); case SIOCGIFFLAGS: From patchwork Fri Nov 6 17:32:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 320813 Delivered-To: patch@linaro.org Received: by 2002:a92:7b12:0:0:0:0:0 with SMTP id w18csp1488543ilc; Fri, 6 Nov 2020 09:33:07 -0800 (PST) X-Google-Smtp-Source: ABdhPJyVQf68jXjhfwlSx8geT00seuD9JcmeL7TMd/1pnvURGQWjmmt5/EBhu4XWIYictzt0CZyU X-Received: by 2002:a17:906:ad85:: with SMTP id la5mr3227912ejb.423.1604683987128; Fri, 06 Nov 2020 09:33:07 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1604683987; cv=none; d=google.com; s=arc-20160816; b=L7VBxVb/QkblY+YRoEX99v7flGIv0xJRKbOZZWgbVf3XMeot5+dIqd82M+Hkl0OaIk h4P8vxX+t0quYtIEYr7gQKVopvM2HZEXIy+7Qkqssa9BhDWV4LkKDFL+rgPWa+/EelWj ondWcV5UUNv+UFfYtCc77z1IFptuh5EvY/487YIai30oByWdm6RLmdNUGU3s6VW6+hof XgSxd8h2sCJvSUjpkfgRzvZkzrgYIDwvwVa149+b2mWByAwZn1I4ffu5ktjVrMIzU4GN U2iWBAP5bVEsaNVukJmNSiM1DCcSbtKI3DkCMal05Z8guL5vnzT4g3LP8SfxGWVvbAAo a2Hg== 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=vEzFwglAPgpMU2B0y9T1+vknPFtGk9BsyoEiEbex+Sg=; b=st/LFGiQIat8fELvv78nMMY2I0WywdAIcjqtddhbCp0dzEtu8dudZjDY0juJavdedA 5DknXA9j8f3umLCFLLpom1DuHKmeqA54QJHeSLxySkIfO44ibM0gy35Hrf2wl4Jo36VN 9rgtWHH+dfXUMMHUtsaBoXwka25H61Onuv4QwjCAkmWt8AigICSrDszBctFqFFuc02y7 Qo/ZZRBp8iyLO4n4mQRflXizHCuKgAovXnb97UeuMPFMfkySAG4WIrj0gCEBLQs/H33m uJn+5SPAdJF08suQ+NzZbKoHPIxsP6CKLoWZZFPqsJjpNieedq1ua2Pg4O3MhGfvhfj0 W7HQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=n6TsnMr6; 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 k20si1212333ejj.93.2020.11.06.09.33.06; Fri, 06 Nov 2020 09:33:07 -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=n6TsnMr6; 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 S1727779AbgKFRdD (ORCPT + 8 others); Fri, 6 Nov 2020 12:33:03 -0500 Received: from mail.kernel.org ([198.145.29.99]:45252 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727699AbgKFRcz (ORCPT ); Fri, 6 Nov 2020 12:32:55 -0500 Received: from localhost.localdomain (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 AE637208C7; Fri, 6 Nov 2020 17:32:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604683973; bh=Sn0an9I985hou8WPDGVTMqYUnZ7RLggcEPn1f0frA9I=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=n6TsnMr6Gn6CHP9zahGatqeJYyrIY0m6SCtkGPepm/ashBCuNdXOkwEdt+Q0vZTzd dHQA9TMqoXGV81qiXBeo87Ld0o5ImShQJUbnMXmOu+IFb4X1REsTqW9+c/aeoXJw2H MZcP9I+bgrNMz8hj4HaRdaVG2BlDfc1C6CxSeoSc= From: Arnd Bergmann To: netdev@vger.kernel.org Cc: Arnd Bergmann , "David S. Miller" , Jakub Kicinski , Christoph Hellwig , Jiri Pirko , Taehee Yoo , Eric Dumazet , Alexei Starovoitov , Andrew Lunn , linux-kernel@vger.kernel.org Subject: [PATCH net-next v3 4/4] net: socket: rework compat_ifreq_ioctl() Date: Fri, 6 Nov 2020 18:32:31 +0100 Message-Id: <20201106173231.3031349-5-arnd@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20201106173231.3031349-1-arnd@kernel.org> References: <20201106173231.3031349-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/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 | 102 ++++++++++++++++++++++++-------------- 6 files changed, 76 insertions(+), 46 deletions(-) -- 2.27.0 diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index b54d5308087d..d1b7fb1241b3 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -3827,6 +3827,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 1d48708c5a2e..3b3eccf90180 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 957aa9263ba4..ad430b401e00 100644 --- a/net/qrtr/qrtr.c +++ b/net/qrtr/qrtr.c @@ -1134,14 +1134,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 f26ef299b6c5..6917835d2f3e 100644 --- a/net/socket.c +++ b/net/socket.c @@ -3092,6 +3092,55 @@ 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) { @@ -3100,7 +3149,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)) @@ -3112,7 +3161,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; @@ -3138,47 +3187,26 @@ 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; }