* [PATCH v5 1/4] ethtool: improve compat ioctl handling
2021-07-20 14:24 [PATCH net-next v5 0/4] remove compat_alloc_user_space() Arnd Bergmann
@ 2021-07-20 14:24 ` Arnd Bergmann
2021-07-20 14:24 ` [PATCH v5 2/4] net: socket: rework SIOC?IFMAP ioctls Arnd Bergmann
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Arnd Bergmann @ 2021-07-20 14:24 UTC (permalink / raw)
To: netdev; +Cc: Christoph Hellwig, Arnd Bergmann
From: Arnd Bergmann <arnd@arndb.de>
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 <arnd@arndb.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
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 | 136 +++++++++++++++++++++++++++++++++++-----
net/socket.c | 125 +-----------------------------------
3 files changed, 121 insertions(+), 144 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 232daaec56e4..4711b96dae0c 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -17,8 +17,6 @@
#include <linux/compat.h>
#include <uapi/linux/ethtool.h>
-#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 <linux/rculist.h>
/**
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index baa5d10043cb..6134b180f59f 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -7,6 +7,7 @@
* the information ethtool needs.
*/
+#include <linux/compat.h>
#include <linux/module.h>
#include <linux/types.h>
#include <linux/capability.h>
@@ -807,6 +808,120 @@ static noinline_for_stack int ethtool_get_sset_info(struct net_device *dev,
return ret;
}
+static noinline_for_stack 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.
+ * On non-x86, no conversion should be needed.
+ */
+ BUILD_BUG_ON(!IS_ENABLED(CONFIG_X86_64) &&
+ sizeof(struct compat_ethtool_rxnfc) !=
+ sizeof(struct ethtool_rxnfc));
+ 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 (compat_need_64bit_alignment_fixup())
+ 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 (compat_need_64bit_alignment_fixup()) {
+ 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 +940,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 +948,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 +974,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 +982,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 +1008,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 0b2dad3bdf7f..ec63cf6de33e 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -3152,128 +3152,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;
@@ -3428,8 +3306,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:
@@ -3442,6 +3318,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:
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v5 2/4] net: socket: rework SIOC?IFMAP ioctls
2021-07-20 14:24 [PATCH net-next v5 0/4] remove compat_alloc_user_space() Arnd Bergmann
2021-07-20 14:24 ` [PATCH v5 1/4] ethtool: improve compat ioctl handling Arnd Bergmann
@ 2021-07-20 14:24 ` Arnd Bergmann
2021-07-21 7:28 ` Christoph Hellwig
2021-07-20 14:24 ` [PATCH v5 3/4] net: socket: simplify dev_ifconf handling Arnd Bergmann
2021-07-20 14:24 ` [PATCH v5 4/4] net: socket: rework compat_ifreq_ioctl() Arnd Bergmann
3 siblings, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2021-07-20 14:24 UTC (permalink / raw)
To: netdev; +Cc: Christoph Hellwig, Arnd Bergmann
From: Arnd Bergmann <arnd@arndb.de>
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 <arnd@arndb.de>
---
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(-)
diff --git a/include/linux/compat.h b/include/linux/compat.h
index c699df9b16fd..e6231837aff5 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -95,6 +95,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
@@ -317,15 +326,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 478d032f34ac..336d75d58357 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()
*/
@@ -128,13 +177,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;
@@ -275,12 +318,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 ec63cf6de33e..62005a12ec70 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -3241,40 +3241,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
@@ -3308,9 +3274,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)
@@ -3340,6 +3303,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:
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v5 2/4] net: socket: rework SIOC?IFMAP ioctls
2021-07-20 14:24 ` [PATCH v5 2/4] net: socket: rework SIOC?IFMAP ioctls Arnd Bergmann
@ 2021-07-21 7:28 ` Christoph Hellwig
2021-07-21 8:32 ` Arnd Bergmann
0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2021-07-21 7:28 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: netdev, Christoph Hellwig, Arnd Bergmann
> +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()) {
Any reason that the cifmap declaration is outside this conditional?
> +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()) {
Same here.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v5 2/4] net: socket: rework SIOC?IFMAP ioctls
2021-07-21 7:28 ` Christoph Hellwig
@ 2021-07-21 8:32 ` Arnd Bergmann
0 siblings, 0 replies; 9+ messages in thread
From: Arnd Bergmann @ 2021-07-21 8:32 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Networking, Arnd Bergmann
On Wed, Jul 21, 2021 at 9:30 AM Christoph Hellwig <hch@lst.de> wrote:
>
> > +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()) {
>
> Any reason that the cifmap declaration is outside this conditional?
I was going for the slightly shorter version, as moving it into the block
runs into the 80-character limit. I'll change it.
> > +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()) {
>
> Same here.
>
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
Thanks!
Arnd
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 3/4] net: socket: simplify dev_ifconf handling
2021-07-20 14:24 [PATCH net-next v5 0/4] remove compat_alloc_user_space() Arnd Bergmann
2021-07-20 14:24 ` [PATCH v5 1/4] ethtool: improve compat ioctl handling Arnd Bergmann
2021-07-20 14:24 ` [PATCH v5 2/4] net: socket: rework SIOC?IFMAP ioctls Arnd Bergmann
@ 2021-07-20 14:24 ` Arnd Bergmann
2021-07-21 7:32 ` Christoph Hellwig
2021-07-20 14:24 ` [PATCH v5 4/4] net: socket: rework compat_ifreq_ioctl() Arnd Bergmann
3 siblings, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2021-07-20 14:24 UTC (permalink / raw)
To: netdev; +Cc: Christoph Hellwig, Arnd Bergmann
From: Arnd Bergmann <arnd@arndb.de>
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 <arnd@arndb.de>
---
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(-)
diff --git a/include/linux/compat.h b/include/linux/compat.h
index e6231837aff5..4727c7a3a988 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -104,6 +104,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
@@ -326,12 +331,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 eaf5bb008aa9..c88290a33080 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3289,14 +3289,6 @@ static inline bool dev_has_header(const struct net_device *dev)
return dev->header_ops && dev->header_ops->create;
}
-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 {
@@ -4014,7 +4006,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 336d75d58357..0af7b9f09970 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/kmod.h>
#include <linux/netdevice.h>
+#include <linux/inetdevice.h>
#include <linux/etherdevice.h>
#include <linux/rtnetlink.h>
#include <linux/net_tstamp.h>
@@ -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 73721a4448bd..0683449650e7 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;
@@ -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 62005a12ec70..ecdb7913a3bd 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1088,6 +1088,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;
@@ -1100,25 +1102,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;
}
@@ -1217,6 +1207,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;
@@ -3127,31 +3122,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;
@@ -3270,8 +3240,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:
@@ -3299,6 +3267,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:
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v5 3/4] net: socket: simplify dev_ifconf handling
2021-07-20 14:24 ` [PATCH v5 3/4] net: socket: simplify dev_ifconf handling Arnd Bergmann
@ 2021-07-21 7:32 ` Christoph Hellwig
2021-07-21 9:12 ` Arnd Bergmann
0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2021-07-21 7:32 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: netdev, Christoph Hellwig, Arnd Bergmann
> The implementation can be simplified further, based on the
> knowledge that the dynamic registration is only ever used
> > for IPv4.
I think dropping register_gifconf (which seems like a nice cleanup!)
needs to be a separate prep patch to not make this too confusing.
> index e6231837aff5..4727c7a3a988 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -104,6 +104,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
> @@ -326,12 +331,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 */
> -};
Does this actually compile as-is? It adds a second definition of
compat_ifconf but removes the still used compat_if_settings?
Maybe it would be a better idea to add a prep patch that makes as much
as possible of compat.h available unconditionally instead of all these
little moves.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v5 3/4] net: socket: simplify dev_ifconf handling
2021-07-21 7:32 ` Christoph Hellwig
@ 2021-07-21 9:12 ` Arnd Bergmann
0 siblings, 0 replies; 9+ messages in thread
From: Arnd Bergmann @ 2021-07-21 9:12 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Networking, Arnd Bergmann
On Wed, Jul 21, 2021 at 9:32 AM Christoph Hellwig <hch@lst.de> wrote:
>
> > The implementation can be simplified further, based on the
> > knowledge that the dynamic registration is only ever used
> > > for IPv4.
>
> I think dropping register_gifconf (which seems like a nice cleanup!)
> needs to be a separate prep patch to not make this too confusing.
Right, good idea.
> > index e6231837aff5..4727c7a3a988 100644
> > --- a/include/linux/compat.h
> > +++ b/include/linux/compat.h
> > @@ -104,6 +104,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
> > @@ -326,12 +331,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 */
> > -};
>
> Does this actually compile as-is? It adds a second definition of
> compat_ifconf but removes the still used compat_if_settings?
Indeed it does not. I must have applied a hunk to the wrong patch
in an earlier rebase, and the build bots never picked up on it because
the series was fine in the end.
> Maybe it would be a better idea to add a prep patch that makes as much
> as possible of compat.h available unconditionally instead of all these
> little moves.
Ok, I'll figure something out.
Arnd
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 4/4] net: socket: rework compat_ifreq_ioctl()
2021-07-20 14:24 [PATCH net-next v5 0/4] remove compat_alloc_user_space() Arnd Bergmann
` (2 preceding siblings ...)
2021-07-20 14:24 ` [PATCH v5 3/4] net: socket: simplify dev_ifconf handling Arnd Bergmann
@ 2021-07-20 14:24 ` Arnd Bergmann
3 siblings, 0 replies; 9+ messages in thread
From: Arnd Bergmann @ 2021-07-20 14:24 UTC (permalink / raw)
To: netdev; +Cc: Christoph Hellwig, Arnd Bergmann
From: Arnd Bergmann <arnd@arndb.de>
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 <arnd@arndb.de>
---
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 | 103 ++++++++++++++++++++++++--------------
7 files changed, 103 insertions(+), 73 deletions(-)
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 4727c7a3a988..fa30c3c96cff 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -109,6 +109,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
@@ -331,32 +358,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 c88290a33080..5c39129101c0 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4004,6 +4004,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 8ade5a4ceaf5..bf5736c1d458 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 54648181dd56..0e4d758c2585 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -953,10 +953,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;
@@ -966,7 +966,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 e6f4a6202f82..e71847877248 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -1153,14 +1153,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 ecdb7913a3bd..84de89c1ee9d 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -3121,6 +3121,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)
{
@@ -3129,7 +3177,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))
@@ -3141,7 +3189,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;
@@ -3165,49 +3213,28 @@ static int compat_ifr_data_ioctl(struct net *net, unsigned int cmd,
static int compat_ifreq_ioctl(struct net *net, struct socket *sock,
unsigned int cmd,
+ unsigned long arg,
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;
}
@@ -3310,7 +3337,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
case SIOCBONDRELEASE:
case SIOCBONDSETHWADDR:
case SIOCBONDCHANGEACTIVE:
- return compat_ifreq_ioctl(net, sock, cmd, argp);
+ return compat_ifreq_ioctl(net, sock, cmd, arg, argp);
case SIOCSARP:
case SIOCGARP:
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread