From patchwork Tue Nov 24 15:18:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 331397 Delivered-To: patch@linaro.org Received: by 2002:a17:907:2110:0:0:0:0 with SMTP id qn16csp4456865ejb; Tue, 24 Nov 2020 07:20:17 -0800 (PST) X-Google-Smtp-Source: ABdhPJz4PZmjdFRsgAEhYJkzR9lRx7rHDxBTl5CPgcI1R2K7K858jJA3uD7ZOPxdwZ0npQ3eT340 X-Received: by 2002:a05:6402:150b:: with SMTP id f11mr4327649edw.332.1606231217309; Tue, 24 Nov 2020 07:20:17 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1606231217; cv=none; d=google.com; s=arc-20160816; b=UksN8SDjSH+mjuHu8nlVjKwNBF6q3/aCentaHUMz6IfG5MtRo8fktVCkDQxj9+0ikT p0+/z9wOIY0iEgDlT5l45LtN/BoSEnoxBhUZt+tepwFeDZ1Fd64m3YL1oLSnEt20O0qS ucFJ8Q31i2Qmk57+WnMo2+qGzxyFv/N4fZNeAcDA4zoKm02vLVwfMh7E/pwFjRYsAgkL Y8bkWmHgdLtiyQHQlcLAA846OJ9q7mygY5MXGl/YHqsqj8n5KQIJwyT+RYwU7Y5XCwNB X6a5EVjVgNNlBCD9k/snHiW/DAYhvT/M1L9QbHovngNWcB4FdWg8w+yR6BAxkxU1I4Qt 283w== 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=jwgSJGKYQR45bPEnjZRQvcGHem/GHEqsgQvmLC8bIxo=; b=BQTaulAIDZf+s77WUOB1GWiFJYLIMQSTOs5LQ7apY5LSrnyMVWMxMGNJw2BInMp1hz 4LePvr+84Xhzpb+l0nx760YpHiO0wqGJJWmeccvXJDWBMZwnvi9q4kp208XYNE0e/Jtt fr3JloWpPJszsx9syouqKU0ru9GZUj4QTJxSHP8fGKa3kh23Vp3hbMJ8R1I86HuKd3lq vr37K35jvVVkjPW/cZQpNcdmBAv/JpvwswyV8dDoO+XwLE98IMWhGs8GNBnCfTvejuSQ Vl8jx2eB3v8iOrC/UWz+BvoCE9qaHlblERa8qU1tiTFD1Ibe8D3v5GaKYlU8ISXwthFs VgnQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b="bCa9d/so"; 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.17; Tue, 24 Nov 2020 07:20:17 -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="bCa9d/so"; 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 S2389388AbgKXPSq (ORCPT + 8 others); Tue, 24 Nov 2020 10:18:46 -0500 Received: from mail.kernel.org ([198.145.29.99]:36480 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732490AbgKXPSo (ORCPT ); Tue, 24 Nov 2020 10:18:44 -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 E055A20738; Tue, 24 Nov 2020 15:18:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1606231123; bh=Sb1Q0AwMq/EVRU/527CzxY3DWSqGXcKeYRSb3+9IMig=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=bCa9d/soB7pvwXb5iElZwiDHNFcqYPutK45XcrZmlJFxH46vOjqD64UWElzsbMwTv qUP0aPNjhQBsDlyiMl+/r4oo2RTC9eAGSgKRHyuCYXQNOl4zTqoOE3MSPkuBggpc8m EX2YgY9MBxiFGxAEvFRC28CKvWLLozKCrs+6UYNM= From: Arnd Bergmann To: netdev@vger.kernel.org Cc: Arnd Bergmann , Christoph Hellwig Subject: [PATCH v4 1/4] ethtool: improve compat ioctl handling Date: Tue, 24 Nov 2020 16:18:25 +0100 Message-Id: <20201124151828.169152-2-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 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 e3da25b51ae4..abeae15bf2d6 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 771688e1b0da..bebd68e97078 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 false; +} + +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 372e6c065d22..60df84a91051 100644 --- a/net/socket.c +++ b/net/socket.c @@ -3109,128 +3109,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; @@ -3385,8 +3263,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: @@ -3399,6 +3275,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 Tue Nov 24 15:18:26 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 331400 Delivered-To: patch@linaro.org Received: by 2002:a17:907:2110:0:0:0:0 with SMTP id qn16csp4456879ejb; Tue, 24 Nov 2020 07:20:17 -0800 (PST) X-Google-Smtp-Source: ABdhPJyN+m2NmXwifDZnSvzFnqSpVf3RtUUmALq8/i9Pbz9iSBN7Kq/esuDXiPi4cwou7x5Dd1MP X-Received: by 2002:a17:906:b143:: with SMTP id bt3mr4818293ejb.318.1606231217727; Tue, 24 Nov 2020 07:20:17 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1606231217; cv=none; d=google.com; s=arc-20160816; b=Mx8rxgRLSFqSqQMg2IcQneTFaR+YQACvmLYXpdTKlbtI13NTZbJEthyhW2T7238Sn0 hFf4A1yIQMOHZL6DXO2YpBs8258w+1CXG4HH7BTHRz9Pg+1ytyDwO38utRUie6vjyagS pIiUYpzwZgRBTknvWiBpdcKDfNfo6ohvz9EfxZXQlRmY++aoOX894qmoggm7BLXC6XVU 1SVkf20aCHrZraT9kIbx4D5+bu0JBugvFCjesX1DLUc+9cguoCfO5nu20vm4qAk9wP6K dk16ygx4p+DBYwm7XLnE9c1sRKtrHNMJ1MwcsrGxRmPGXUc3ZbGjX0Mj5d1zcIENcFxn o6fQ== 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=TMDQwPeRlDkEw1iEDOM7wmFSkTlznxXFOOoXV6N0C6g=; b=eRm/VkjCRboZFAc48A6mA9V1+r0mx4nF0TcxJuCvLWO73RYk8ccjr3mCaFfDvJO00W 0eqZSavQsZ9/ctAKaUMbuV/8OkxI3I4DSwXJW2hzN6vODuEIJQqG3sHgCWCG54QL8s7L 11lwDsbfx2Yd6D1d/gyvqP7vzhuKW/dE7f5lbeb9iLvd3KSTDw97n3MJPlMyyT1hwo4g G5JzZ8JaFJsCZnwDcC5+F4uRMEINhS3vxLRFcm8BSGtH4y9dNFZx3ufJQja+ph0tpj14 I/+JWhZmvYbirLx6PZ33r9d9FQawsb4u6nZ004rgR3bwBI7PTbRksNjRFXAWc3kG2QE7 uyUA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=QNVzaEVE; 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.17; Tue, 24 Nov 2020 07:20:17 -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=QNVzaEVE; 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 S2389415AbgKXPSq (ORCPT + 8 others); Tue, 24 Nov 2020 10:18:46 -0500 Received: from mail.kernel.org ([198.145.29.99]:36486 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387697AbgKXPSp (ORCPT ); Tue, 24 Nov 2020 10:18:45 -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 2BB1D206D8; Tue, 24 Nov 2020 15:18:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1606231124; bh=jJQ0X8vmSY8/4iYrv6r4aPgAmmluD0gJ4P5wes1+3e0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=QNVzaEVEWIq6zu98XyNLUmbZzYEXLh0pybzS9E89an7NmPaNSCIhxinQBwJHmpcys RFaDAJ9kx4qZvZkpyCvmrB2CANsUd4GhW3d0xGYYblG7lC/xKTIWERayMQ1b2ufV9t o5bw0eNiS3B64E+1SF+tgH29uBIDk64BcOvF8xM8= From: Arnd Bergmann To: netdev@vger.kernel.org Cc: Arnd Bergmann Subject: [PATCH v4 2/4] net: socket: rework SIOC?IFMAP ioctls Date: Tue, 24 Nov 2020 16:18:26 +0100 Message-Id: <20201124151828.169152-3-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 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 db8a0ff86f36..de3df6fe65fe 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 60df84a91051..2d32c8576e95 100644 --- a/net/socket.c +++ b/net/socket.c @@ -3198,40 +3198,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 @@ -3265,9 +3231,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) @@ -3297,6 +3260,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 Tue Nov 24 15:18:27 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 331398 Delivered-To: patch@linaro.org Received: by 2002:a17:907:2110:0:0:0:0 with SMTP id qn16csp4456891ejb; Tue, 24 Nov 2020 07:20:18 -0800 (PST) X-Google-Smtp-Source: ABdhPJyxOFR2TNmWdsBlztOGQW7xlkloiTDKOi6Qnl6eYQtURzpfGh7Fvn3dI2w6H3ao9nl84/ah X-Received: by 2002:a17:906:68c4:: with SMTP id y4mr3693448ejr.332.1606231218263; 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=aBTgUdNoY+zA6qrhfG/4Rzqi+z599Zc66EF0UkL2UNqcPbOciWa1OMNnwnb24XqVtY 2Aqr8kfIF1lA/aM73HSeBEjsZIqC28ZpCHrpxCHqwiiqNWl8OhVtiglL6ix1UQa8ioLJ uHPGyCrjrXupkTjs9UWsxk0tO6tToaYVpcR5KKV0Be9lqItvzx29Yb8FrAot1ZRon/Rh oxIe770tqaAnpP/eXrKUvz/9vVu16Q7p0Rw2l4C6bHGn7tPH0eA0Oq2RwX7dLB6AwlA7 3OG9iPe3gs8FKam4CmMLIYb8byCQcBr53ie2e6AxGHz0Gll9nqsGXLD5KhmDX0Tn4IU4 Ch6Q== 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=O/K4G6FSD2PXjy3TequSIRcDE40lx2f/GxhoMgb3Wec=; b=bXKPYTSe3l971plWXpKjOra0qr72jkudqgbJb9b2VUe8S8CLzCFGjQJOep52nbxPTw zQk8rN64nfMRZFmJnX8v3es6x43LPXcWLW/wDqveZagETiq5K2wq1tt/cvIPAPJwO4yh +pW1qp19fRHg6pjpUYvwRoHVA4i8Jx45gQkTN8DuKQjwtOE07HmRgueIzAbjTVbrFfnW h4FfUmZ/zwYz/3xDPqu+zx1840dR7vLzMbYFD1xFMqXyk2QVkN/atjiihmnPjwS/RrTm 0UZz/VClTQUZcreXd82ZbugaNGoXNuPf86m8I8UOllUeAHT7dqHaVJjUzFWQisL03H+J qzXQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=ls1NurtF; 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.17; 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=ls1NurtF; 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 S2389430AbgKXPSs (ORCPT + 8 others); Tue, 24 Nov 2020 10:18:48 -0500 Received: from mail.kernel.org ([198.145.29.99]:36490 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389372AbgKXPSr (ORCPT ); Tue, 24 Nov 2020 10:18:47 -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 43CFE20715; Tue, 24 Nov 2020 15:18:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1606231125; bh=vGtAKcFnyf4V+AdErGElAJ94wwKSpVtzEuuc+JOpH1M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ls1NurtF3JHJ5hEO0mscCox8vsqkQbInQcqGdARsoarT75oaTi6RAKxF7sXo78lqs hH3oiwnZP+aFV7WB4aDP5KuUQSULQ2hrSzExT+arpO2as9xW6mHaH3k4Ujp2XwSoR5 vlmyncL9GSybblOXszxeYNVXc/SyAIOEWYRlIjeY= From: Arnd Bergmann To: netdev@vger.kernel.org Cc: Arnd Bergmann Subject: [PATCH v4 3/4] net: socket: simplify dev_ifconf handling Date: Tue, 24 Nov 2020 16:18:27 +0100 Message-Id: <20201124151828.169152-4-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 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/compat.h | 11 +++-- include/linux/inetdevice.h | 9 ++++ include/linux/netdevice.h | 10 +---- net/core/dev_ioctl.c | 92 +++++++++++++++----------------------- net/ipv4/devinet.c | 4 +- net/socket.c | 59 ++++++------------------ 6 files changed, 66 insertions(+), 119 deletions(-) -- 2.27.0 diff --git a/include/linux/compat.h b/include/linux/compat.h index 47496c5eb5eb..a97f80b704ab 100644 --- a/include/linux/compat.h +++ b/include/linux/compat.h @@ -105,6 +105,11 @@ struct compat_ifmap { unsigned char port; }; +struct compat_ifconf { + compat_int_t ifc_len; /* size of buffer */ + compat_uptr_t ifcbuf; +}; + #ifdef CONFIG_COMPAT #ifndef compat_user_stack_pointer @@ -323,12 +328,6 @@ typedef struct compat_sigevent { } _sigev_un; } compat_sigevent_t; -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" */ diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h index 53aa0343bf69..67e042932681 100644 --- a/include/linux/inetdevice.h +++ b/include/linux/inetdevice.h @@ -178,6 +178,15 @@ 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 int 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 03433a4c929e..e99450a60d8e 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -3163,14 +3163,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 { @@ -3870,7 +3862,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 de3df6fe65fe..de5478b2ef77 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 75f67994fc85..3c51395a49c8 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -1243,7 +1243,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; @@ -2761,8 +2761,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 2d32c8576e95..41158e459f0b 100644 --- a/net/socket.c +++ b/net/socket.c @@ -1029,6 +1029,8 @@ EXPORT_SYMBOL(vlan_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; @@ -1041,25 +1043,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; } @@ -1171,6 +1161,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; @@ -3084,31 +3079,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; @@ -3227,8 +3197,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: @@ -3256,6 +3224,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 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; }