* [PATCH net-next v2 1/2] ethtool: improve compat ioctl handling
@ 2020-09-25 13:22 Arnd Bergmann
2020-09-25 13:22 ` [PATCH net-next v2 2/2] dev_ioctl: split out SIOC?IFMAP ioctls Arnd Bergmann
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Arnd Bergmann @ 2020-09-25 13:22 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski
Cc: Christoph Hellwig, Arnd Bergmann, Andrew Lunn, Florian Fainelli,
Michal Kubecek, Jens Axboe, linux-kernel, netdev
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 | 143 +++++++++++++++++++++++++++++++++++-----
net/socket.c | 125 +----------------------------------
3 files changed, 128 insertions(+), 144 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 060b20f0b20f..f989242ebaf8 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 328d15cd4006..10ef9527fb91 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,127 @@ static noinline_for_stack int ethtool_get_sset_info(struct net_device *dev,
return ret;
}
+static bool ethtool_translate_compat(void)
+{
+#ifdef CONFIG_X86_64
+ /* On x86, translation is needed for i386 but not x32 */
+ return in_ia32_syscall();
+#else
+ BUILD_BUG_ON(sizeof(struct compat_ethtool_rxnfc) !=
+ sizeof(struct ethtool_rxnfc));
+#endif
+ return 0;
+}
+
+static int ethtool_rxnfc_copy_from_compat(struct ethtool_rxnfc *rxnfc,
+ const struct compat_ethtool_rxnfc __user *useraddr,
+ size_t size)
+{
+ struct compat_ethtool_rxnfc crxnfc = {};
+
+ /* We expect there to be holes between fs.m_ext and
+ * fs.ring_cookie and at the end of fs, but nowhere else.
+ */
+ BUILD_BUG_ON(offsetof(struct compat_ethtool_rxnfc, fs.m_ext) +
+ sizeof(useraddr->fs.m_ext) !=
+ offsetof(struct ethtool_rxnfc, fs.m_ext) +
+ sizeof(rxnfc->fs.m_ext));
+ BUILD_BUG_ON(offsetof(struct compat_ethtool_rxnfc, fs.location) -
+ offsetof(struct compat_ethtool_rxnfc, fs.ring_cookie) !=
+ offsetof(struct ethtool_rxnfc, fs.location) -
+ offsetof(struct ethtool_rxnfc, fs.ring_cookie));
+
+ if (copy_from_user(&crxnfc, useraddr, min(size, sizeof(crxnfc))))
+ return -EFAULT;
+
+ *rxnfc = (struct ethtool_rxnfc) {
+ .cmd = crxnfc.cmd,
+ .flow_type = crxnfc.flow_type,
+ .data = crxnfc.data,
+ .fs = {
+ .flow_type = crxnfc.fs.flow_type,
+ .h_u = crxnfc.fs.h_u,
+ .h_ext = crxnfc.fs.h_ext,
+ .m_u = crxnfc.fs.m_u,
+ .m_ext = crxnfc.fs.m_ext,
+ .ring_cookie = crxnfc.fs.ring_cookie,
+ .location = crxnfc.fs.location,
+ },
+ .rule_cnt = crxnfc.rule_cnt,
+ };
+
+ return 0;
+}
+
+static int ethtool_rxnfc_copy_from_user(struct ethtool_rxnfc *rxnfc,
+ const void __user *useraddr, size_t size)
+{
+ if (ethtool_translate_compat())
+ return ethtool_rxnfc_copy_from_compat(rxnfc, useraddr, size);
+
+ if (copy_from_user(&rxnfc, useraddr, size))
+ return -EFAULT;
+
+ return 0;
+}
+
+static int ethtool_rxnfc_copy_to_compat(void __user *useraddr,
+ const struct ethtool_rxnfc *rxnfc,
+ size_t size, const u32 *rule_buf)
+{
+
+ struct compat_ethtool_rxnfc crxnfc;
+
+ memset(&crxnfc, 0, sizeof(crxnfc));
+ crxnfc = (struct compat_ethtool_rxnfc) {
+ .cmd = rxnfc->cmd,
+ .flow_type = rxnfc->flow_type,
+ .data = rxnfc->data,
+ .fs = {
+ .flow_type = rxnfc->fs.flow_type,
+ .h_u = rxnfc->fs.h_u,
+ .h_ext = rxnfc->fs.h_ext,
+ .m_u = rxnfc->fs.m_u,
+ .m_ext = rxnfc->fs.m_ext,
+ .ring_cookie = rxnfc->fs.ring_cookie,
+ .location = rxnfc->fs.location,
+ },
+ .rule_cnt = rxnfc->rule_cnt,
+ };
+
+ if (copy_to_user(useraddr, &crxnfc, min(size, sizeof(crxnfc))))
+ return -EFAULT;
+
+ return 0;
+}
+
+static int ethtool_rxnfc_copy_to_user(void __user *useraddr,
+ const struct ethtool_rxnfc *rxnfc,
+ size_t size, const u32 *rule_buf)
+{
+ int ret;
+
+ if (ethtool_translate_compat()) {
+ ret = ethtool_rxnfc_copy_to_compat(useraddr, rxnfc, size,
+ rule_buf);
+ useraddr += offsetof(struct compat_ethtool_rxnfc, rule_locs);
+ } else {
+ ret = copy_to_user(useraddr, &rxnfc, size);
+ useraddr += offsetof(struct ethtool_rxnfc, rule_locs);
+ }
+
+ if (ret)
+ return -EFAULT;
+
+ if (rule_buf) {
+ if (copy_to_user(useraddr, rule_buf,
+ rxnfc->rule_cnt * sizeof(u32)))
+ return -EFAULT;
+ }
+
+ return 0;
+}
+
static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev,
u32 cmd, void __user *useraddr)
{
@@ -825,7 +947,7 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev,
info_size = (offsetof(struct ethtool_rxnfc, data) +
sizeof(info.data));
- if (copy_from_user(&info, useraddr, info_size))
+ if (ethtool_rxnfc_copy_from_user(&info, useraddr, info_size))
return -EFAULT;
rc = dev->ethtool_ops->set_rxnfc(dev, &info);
@@ -833,7 +955,7 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev,
return rc;
if (cmd == ETHTOOL_SRXCLSRLINS &&
- copy_to_user(useraddr, &info, info_size))
+ ethtool_rxnfc_copy_to_user(useraddr, &info, info_size, NULL))
return -EFAULT;
return 0;
@@ -859,7 +981,7 @@ static noinline_for_stack int ethtool_get_rxnfc(struct net_device *dev,
info_size = (offsetof(struct ethtool_rxnfc, data) +
sizeof(info.data));
- if (copy_from_user(&info, useraddr, info_size))
+ if (ethtool_rxnfc_copy_from_user(&info, useraddr, info_size))
return -EFAULT;
/* If FLOW_RSS was requested then user-space must be using the
@@ -867,7 +989,7 @@ static noinline_for_stack int ethtool_get_rxnfc(struct net_device *dev,
*/
if (cmd == ETHTOOL_GRXFH && info.flow_type & FLOW_RSS) {
info_size = sizeof(info);
- if (copy_from_user(&info, useraddr, info_size))
+ if (ethtool_rxnfc_copy_from_user(&info, useraddr, info_size))
return -EFAULT;
/* Since malicious users may modify the original data,
* we need to check whether FLOW_RSS is still requested.
@@ -893,18 +1015,7 @@ static noinline_for_stack int ethtool_get_rxnfc(struct net_device *dev,
if (ret < 0)
goto err_out;
- ret = -EFAULT;
- if (copy_to_user(useraddr, &info, info_size))
- goto err_out;
-
- if (rule_buf) {
- useraddr += offsetof(struct ethtool_rxnfc, rule_locs);
- if (copy_to_user(useraddr, rule_buf,
- info.rule_cnt * sizeof(u32)))
- goto err_out;
- }
- ret = 0;
-
+ ret = ethtool_rxnfc_copy_to_user(useraddr, &info, info_size, rule_buf);
err_out:
kfree(rule_buf);
diff --git a/net/socket.c b/net/socket.c
index 82262e1922f9..8809db922574 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -3123,128 +3123,6 @@ static int compat_dev_ifconf(struct net *net, struct compat_ifconf __user *uifc3
return 0;
}
-static int ethtool_ioctl(struct net *net, struct compat_ifreq __user *ifr32)
-{
- struct compat_ethtool_rxnfc __user *compat_rxnfc;
- bool convert_in = false, convert_out = false;
- size_t buf_size = 0;
- struct ethtool_rxnfc __user *rxnfc = NULL;
- struct ifreq ifr;
- u32 rule_cnt = 0, actual_rule_cnt;
- u32 ethcmd;
- u32 data;
- int ret;
-
- if (get_user(data, &ifr32->ifr_ifru.ifru_data))
- return -EFAULT;
-
- compat_rxnfc = compat_ptr(data);
-
- if (get_user(ethcmd, &compat_rxnfc->cmd))
- return -EFAULT;
-
- /* Most ethtool structures are defined without padding.
- * Unfortunately struct ethtool_rxnfc is an exception.
- */
- switch (ethcmd) {
- default:
- break;
- case ETHTOOL_GRXCLSRLALL:
- /* Buffer size is variable */
- if (get_user(rule_cnt, &compat_rxnfc->rule_cnt))
- return -EFAULT;
- if (rule_cnt > KMALLOC_MAX_SIZE / sizeof(u32))
- return -ENOMEM;
- buf_size += rule_cnt * sizeof(u32);
- fallthrough;
- case ETHTOOL_GRXRINGS:
- case ETHTOOL_GRXCLSRLCNT:
- case ETHTOOL_GRXCLSRULE:
- case ETHTOOL_SRXCLSRLINS:
- convert_out = true;
- fallthrough;
- case ETHTOOL_SRXCLSRLDEL:
- buf_size += sizeof(struct ethtool_rxnfc);
- convert_in = true;
- rxnfc = compat_alloc_user_space(buf_size);
- break;
- }
-
- if (copy_from_user(&ifr.ifr_name, &ifr32->ifr_name, IFNAMSIZ))
- return -EFAULT;
-
- ifr.ifr_data = convert_in ? rxnfc : (void __user *)compat_rxnfc;
-
- if (convert_in) {
- /* We expect there to be holes between fs.m_ext and
- * fs.ring_cookie and at the end of fs, but nowhere else.
- */
- BUILD_BUG_ON(offsetof(struct compat_ethtool_rxnfc, fs.m_ext) +
- sizeof(compat_rxnfc->fs.m_ext) !=
- offsetof(struct ethtool_rxnfc, fs.m_ext) +
- sizeof(rxnfc->fs.m_ext));
- BUILD_BUG_ON(
- offsetof(struct compat_ethtool_rxnfc, fs.location) -
- offsetof(struct compat_ethtool_rxnfc, fs.ring_cookie) !=
- offsetof(struct ethtool_rxnfc, fs.location) -
- offsetof(struct ethtool_rxnfc, fs.ring_cookie));
-
- if (copy_in_user(rxnfc, compat_rxnfc,
- (void __user *)(&rxnfc->fs.m_ext + 1) -
- (void __user *)rxnfc) ||
- copy_in_user(&rxnfc->fs.ring_cookie,
- &compat_rxnfc->fs.ring_cookie,
- (void __user *)(&rxnfc->fs.location + 1) -
- (void __user *)&rxnfc->fs.ring_cookie))
- return -EFAULT;
- if (ethcmd == ETHTOOL_GRXCLSRLALL) {
- if (put_user(rule_cnt, &rxnfc->rule_cnt))
- return -EFAULT;
- } else if (copy_in_user(&rxnfc->rule_cnt,
- &compat_rxnfc->rule_cnt,
- sizeof(rxnfc->rule_cnt)))
- return -EFAULT;
- }
-
- ret = dev_ioctl(net, SIOCETHTOOL, &ifr, NULL);
- if (ret)
- return ret;
-
- if (convert_out) {
- if (copy_in_user(compat_rxnfc, rxnfc,
- (const void __user *)(&rxnfc->fs.m_ext + 1) -
- (const void __user *)rxnfc) ||
- copy_in_user(&compat_rxnfc->fs.ring_cookie,
- &rxnfc->fs.ring_cookie,
- (const void __user *)(&rxnfc->fs.location + 1) -
- (const void __user *)&rxnfc->fs.ring_cookie) ||
- copy_in_user(&compat_rxnfc->rule_cnt, &rxnfc->rule_cnt,
- sizeof(rxnfc->rule_cnt)))
- return -EFAULT;
-
- if (ethcmd == ETHTOOL_GRXCLSRLALL) {
- /* As an optimisation, we only copy the actual
- * number of rules that the underlying
- * function returned. Since Mallory might
- * change the rule count in user memory, we
- * check that it is less than the rule count
- * originally given (as the user buffer size),
- * which has been range-checked.
- */
- if (get_user(actual_rule_cnt, &rxnfc->rule_cnt))
- return -EFAULT;
- if (actual_rule_cnt < rule_cnt)
- rule_cnt = actual_rule_cnt;
- if (copy_in_user(&compat_rxnfc->rule_locs[0],
- &rxnfc->rule_locs[0],
- rule_cnt * sizeof(u32)))
- return -EFAULT;
- }
- }
-
- return 0;
-}
-
static int compat_siocwandev(struct net *net, struct compat_ifreq __user *uifr32)
{
compat_uptr_t uptr32;
@@ -3399,8 +3277,6 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
return old_bridge_ioctl(argp);
case SIOCGIFCONF:
return compat_dev_ifconf(net, argp);
- case SIOCETHTOOL:
- return ethtool_ioctl(net, argp);
case SIOCWANDEV:
return compat_siocwandev(net, argp);
case SIOCGIFMAP:
@@ -3413,6 +3289,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
return sock->ops->gettstamp(sock, argp, cmd == SIOCGSTAMP_OLD,
!COMPAT_USE_64BIT_TIME);
+ case SIOCETHTOOL:
case SIOCBONDSLAVEINFOQUERY:
case SIOCBONDINFOQUERY:
case SIOCSHWTSTAMP:
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net-next v2 2/2] dev_ioctl: split out SIOC?IFMAP ioctls
2020-09-25 13:22 [PATCH net-next v2 1/2] ethtool: improve compat ioctl handling Arnd Bergmann
@ 2020-09-25 13:22 ` Arnd Bergmann
2020-09-25 18:31 ` [PATCH net-next v2 1/2] ethtool: improve compat ioctl handling kernel test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2020-09-25 13:22 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski
Cc: Christoph Hellwig, Arnd Bergmann, Jens Axboe, linux-kernel, netdev
The ifreq ioctls need a special compat handler at the moment because of
the size difference between the struct on native and compat architectures,
but this difference exists only for one pair of ioctls, SIOCGIFMAP
and SIOCSIFMAP.
Splitting these two out of dev_ioctl() into their own higher level
implementation means the ifreq structure can be redefined in the kernel
to be identical for all applications, avoiding the need for copying the
structure around multiple times, and removing one of the call sites for
compat_alloc_user_space() and copy_in_user().
This should also make it easier for drivers to implement ioctls correct
that take an ifreq __user pointer and need to be careful about the size
difference. This has been a problem in the past, but currently the kernel
does not appear to have any drivers suffering from it.
Note that the user space definition is unchanged, so struct ifreq now
has a different size between user space and kernel, but this is not a
problem when the user space definition is larger, and the only time the
extra members are accessed is in the ifmap ioctls.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
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 +++---
include/linux/netdevice.h | 1 +
include/uapi/linux/if.h | 6 ++
net/core/dev_ioctl.c | 122 ++++++++++++++++++++++++++++++++------
net/socket.c | 93 ++---------------------------
5 files changed, 125 insertions(+), 115 deletions(-)
diff --git a/include/linux/compat.h b/include/linux/compat.h
index b354ce58966e..6359c51b748f 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -91,6 +91,15 @@
static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
#endif /* COMPAT_SYSCALL_DEFINEx */
+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/include/linux/netdevice.h b/include/linux/netdevice.h
index a431c3229cbf..df4124442427 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3810,6 +3810,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_ifmap(struct net *net, struct ifreq __user *ifr, unsigned int cmd);
int dev_ifconf(struct net *net, struct ifconf *, int);
int dev_ethtool(struct net *net, struct ifreq *);
unsigned int dev_get_flags(const struct net_device *);
diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
index 797ba2c1562a..a332d6ae4dc6 100644
--- a/include/uapi/linux/if.h
+++ b/include/uapi/linux/if.h
@@ -247,7 +247,13 @@ struct ifreq {
short ifru_flags;
int ifru_ivalue;
int ifru_mtu;
+#ifndef __KERNEL__
+ /*
+ * ifru_map is rarely used but causes the incompatibility
+ * between native and compat mode.
+ */
struct ifmap ifru_map;
+#endif
char ifru_slave[IFNAMSIZ]; /* Just fits the size */
char ifru_newname[IFNAMSIZ];
void __user * ifru_data;
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 205e92e604ef..79af316e5c19 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -98,6 +98,109 @@ int dev_ifconf(struct net *net, struct ifconf *ifc, int size)
return 0;
}
+static int dev_getifmap(struct net *net, const char *ifname,
+ struct ifreq __user *ifr)
+{
+ struct net_device *dev;
+ struct ifmap ifmap;
+
+ rcu_read_lock();
+ dev = dev_get_by_name_rcu(net, ifname);
+ if (!dev) {
+ rcu_read_unlock();
+ return -ENODEV;
+ }
+
+ memset(&ifmap, 0, sizeof(ifmap));
+ 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;
+ rcu_read_unlock();
+
+ if (in_compat_syscall()) {
+ struct compat_ifmap cifmap;
+
+ memset(&cifmap, 0, sizeof(cifmap));
+ cifmap.mem_start = ifmap.mem_start;
+ cifmap.mem_end = ifmap.mem_end;
+ cifmap.base_addr = ifmap.base_addr;
+ cifmap.irq = ifmap.irq;
+ cifmap.dma = ifmap.dma;
+ cifmap.port = ifmap.port;
+
+ if (copy_to_user(&ifr->ifr_data, &cifmap, sizeof(cifmap)))
+ return -EFAULT;
+ } else {
+ if (copy_to_user(&ifr->ifr_data, &ifmap, sizeof(ifmap)))
+ return -EFAULT;
+ }
+
+ return 0;
+}
+
+static int dev_setifmap(struct net *net, const char *ifname,
+ const struct ifreq __user *ifr)
+{
+ struct net_device *dev;
+ struct ifmap ifmap;
+ int ret;
+
+ if (!capable(CAP_NET_ADMIN) ||
+ !ns_capable(net->user_ns, CAP_NET_ADMIN))
+ return -EPERM;
+
+ if (in_compat_syscall()) {
+ struct compat_ifmap cifmap;
+
+ if (copy_from_user(&cifmap, &ifr->ifr_data, sizeof(cifmap)))
+ return -EFAULT;
+
+ ifmap.mem_start = cifmap.mem_start;
+ ifmap.mem_end = cifmap.mem_end;
+ ifmap.base_addr = cifmap.base_addr;
+ ifmap.irq = cifmap.irq;
+ ifmap.dma = cifmap.dma;
+ ifmap.port = cifmap.port;
+ } else {
+ if (copy_from_user(&ifmap, &ifr->ifr_data, sizeof(ifmap)))
+ return -EFAULT;
+ }
+
+ rtnl_lock();
+ dev = __dev_get_by_name(net, ifname);
+ if (!dev || !netif_device_present(dev))
+ ret = -ENODEV;
+ else if (!dev->netdev_ops->ndo_set_config)
+ ret = -EOPNOTSUPP;
+ else
+ ret = dev->netdev_ops->ndo_set_config(dev, &ifmap);
+ rtnl_unlock();
+
+ return ret;
+}
+
+int dev_ifmap(struct net *net, struct ifreq __user *ifr, unsigned int cmd)
+{
+ char ifname[IFNAMSIZ];
+ char *colon;
+
+ if (copy_from_user(ifname, ifr->ifr_name, sizeof(ifname)))
+ return -EFAULT;
+ ifname[IFNAMSIZ-1] = 0;
+ colon = strchr(ifname, ':');
+ if (colon)
+ *colon = 0;
+ dev_load(net, ifname);
+
+ if (cmd == SIOCGIFMAP)
+ return dev_getifmap(net, ifname, ifr);
+
+ return dev_setifmap(net, ifname, ifr);
+}
+
/*
* Perform the SIOCxIFxxx calls, inside rcu_read_lock()
*/
@@ -138,15 +241,6 @@ static int dev_ifsioc_locked(struct net *net, struct ifreq *ifr, unsigned int cm
err = -EINVAL;
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;
-
case SIOCGIFINDEX:
ifr->ifr_ifindex = dev->ifindex;
return 0;
@@ -285,14 +379,6 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
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;
-
case SIOCADDMULTI:
if (!ops->ndo_set_rx_mode ||
ifr->ifr_hwaddr.sa_family != AF_UNSPEC)
@@ -429,7 +515,6 @@ int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr, bool *need_c
case SIOCGIFMTU:
case SIOCGIFHWADDR:
case SIOCGIFSLAVE:
- case SIOCGIFMAP:
case SIOCGIFINDEX:
case SIOCGIFTXQLEN:
dev_load(net, ifr->ifr_name);
@@ -474,7 +559,6 @@ int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr, bool *need_c
* - require strict serialization.
* - do not return a value
*/
- case SIOCSIFMAP:
case SIOCSIFTXQLEN:
if (!capable(CAP_NET_ADMIN))
return -EPERM;
diff --git a/net/socket.c b/net/socket.c
index 8809db922574..4366900356f6 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1194,6 +1194,10 @@ static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg)
cmd == SIOCGSTAMP_NEW,
false);
break;
+ case SIOCGIFMAP:
+ case SIOCSIFMAP:
+ err = dev_ifmap(net, argp, cmd);
+ break;
default:
err = sock_do_ioctl(net, sock, cmd, arg);
break;
@@ -3164,88 +3168,6 @@ static int compat_ifr_data_ioctl(struct net *net, unsigned int cmd,
return dev_ioctl(net, cmd, &ifreq, NULL);
}
-static int compat_ifreq_ioctl(struct net *net, struct socket *sock,
- unsigned int cmd,
- struct compat_ifreq __user *uifr32)
-{
- struct ifreq __user *uifr;
- 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.
- */
-
- uifr = compat_alloc_user_space(sizeof(*uifr));
- if (copy_in_user(uifr, uifr32, sizeof(*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;
-}
-
-static int compat_sioc_ifmap(struct net *net, unsigned int cmd,
- struct compat_ifreq __user *uifr32)
-{
- struct ifreq ifr;
- struct compat_ifmap __user *uifmap32;
- int err;
-
- uifmap32 = &uifr32->ifr_ifru.ifru_map;
- err = copy_from_user(&ifr, uifr32, sizeof(ifr.ifr_name));
- err |= get_user(ifr.ifr_map.mem_start, &uifmap32->mem_start);
- err |= get_user(ifr.ifr_map.mem_end, &uifmap32->mem_end);
- err |= get_user(ifr.ifr_map.base_addr, &uifmap32->base_addr);
- err |= get_user(ifr.ifr_map.irq, &uifmap32->irq);
- err |= get_user(ifr.ifr_map.dma, &uifmap32->dma);
- err |= get_user(ifr.ifr_map.port, &uifmap32->port);
- if (err)
- return -EFAULT;
-
- err = dev_ioctl(net, cmd, &ifr, NULL);
-
- if (cmd == SIOCGIFMAP && !err) {
- err = copy_to_user(uifr32, &ifr, sizeof(ifr.ifr_name));
- err |= put_user(ifr.ifr_map.mem_start, &uifmap32->mem_start);
- err |= put_user(ifr.ifr_map.mem_end, &uifmap32->mem_end);
- err |= put_user(ifr.ifr_map.base_addr, &uifmap32->base_addr);
- err |= put_user(ifr.ifr_map.irq, &uifmap32->irq);
- err |= put_user(ifr.ifr_map.dma, &uifmap32->dma);
- err |= put_user(ifr.ifr_map.port, &uifmap32->port);
- if (err)
- err = -EFAULT;
- }
- return err;
-}
-
/* Since old style bridge ioctl's endup using SIOCDEVPRIVATE
* for some operations; this forces use of the newer bridge-utils that
* use compatible ioctls
@@ -3279,9 +3201,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)
@@ -3296,6 +3215,8 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
case SIOCGHWTSTAMP:
return compat_ifr_data_ioctl(net, cmd, argp);
+ case SIOCGIFMAP:
+ case SIOCSIFMAP:
case FIOSETOWN:
case SIOCSPGRP:
case FIOGETOWN:
@@ -3349,8 +3270,6 @@ 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);
-
case SIOCSARP:
case SIOCGARP:
case SIOCDARP:
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v2 1/2] ethtool: improve compat ioctl handling
2020-09-25 13:22 [PATCH net-next v2 1/2] ethtool: improve compat ioctl handling Arnd Bergmann
2020-09-25 13:22 ` [PATCH net-next v2 2/2] dev_ioctl: split out SIOC?IFMAP ioctls Arnd Bergmann
@ 2020-09-25 18:31 ` kernel test robot
2020-09-25 18:42 ` Arnd Bergmann
2020-09-25 19:36 ` kernel test robot
3 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-09-25 18:31 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 13367 bytes --]
Hi Arnd,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Arnd-Bergmann/ethtool-improve-compat-ioctl-handling/20200925-212440
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git aafe8853f5e2bcbdd231411aec218b8c5dc78437
config: arm64-randconfig-r035-20200925 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project c32e69b2ce7abfb151a87ba363ac9e25abf7d417)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/7a728784f051b3bce6ddedb8005b01d597e0342d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Arnd-Bergmann/ethtool-improve-compat-ioctl-handling/20200925-212440
git checkout 7a728784f051b3bce6ddedb8005b01d597e0342d
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from drivers/net/wireless/marvell/mwifiex/sta_cmd.c:20:
In file included from drivers/net/wireless/marvell/mwifiex/decl.h:28:
In file included from include/linux/ieee80211.h:20:
In file included from include/linux/etherdevice.h:21:
In file included from include/linux/netdevice.h:37:
include/linux/ethtool.h:26:2: error: unknown type name 'compat_u64'
compat_u64 ring_cookie;
^
include/linux/ethtool.h:33:2: error: unknown type name 'compat_u64'
compat_u64 data;
^
>> drivers/net/wireless/marvell/mwifiex/sta_cmd.c:606:45: warning: implicit conversion from 'unsigned long' to '__u16' (aka 'unsigned short') changes value from 18446744073709551614 to 65534 [-Wconstant-conversion]
km->key_param_set.key_info &= cpu_to_le16(~KEY_MCAST);
~~~~~~~~~~~~^~~~~~~~~~~
include/linux/byteorder/generic.h:90:21: note: expanded from macro 'cpu_to_le16'
#define cpu_to_le16 __cpu_to_le16
^
include/uapi/linux/byteorder/big_endian.h:35:53: note: expanded from macro '__cpu_to_le16'
#define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
~~~~~~~~~~^~~
include/uapi/linux/swab.h:107:12: note: expanded from macro '__swab16'
__fswab16(x))
~~~~~~~~~ ^
1 warning and 2 errors generated.
--
In file included from drivers/net/vmxnet3/vmxnet3_drv.c:28:
In file included from include/net/ip6_checksum.h:27:
In file included from include/net/ip.h:28:
In file included from include/net/inet_sock.h:19:
In file included from include/linux/netdevice.h:37:
include/linux/ethtool.h:26:2: error: unknown type name 'compat_u64'
compat_u64 ring_cookie;
^
include/linux/ethtool.h:33:2: error: unknown type name 'compat_u64'
compat_u64 data;
^
>> drivers/net/vmxnet3/vmxnet3_drv.c:3430:29: warning: shift count >= width of type [-Wshift-count-overflow]
if (pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) == 0) {
^~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:139:54: note: expanded from macro 'DMA_BIT_MASK'
#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
^ ~~~
drivers/net/vmxnet3/vmxnet3_drv.c:3431:41: warning: shift count >= width of type [-Wshift-count-overflow]
if (pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64)) != 0) {
^~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:139:54: note: expanded from macro 'DMA_BIT_MASK'
#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
^ ~~~
2 warnings and 2 errors generated.
--
In file included from drivers/net/wireless/intel/iwlwifi/pcie/trans.c:74:
In file included from drivers/net/wireless/intel/iwlwifi/iwl-trans.h:67:
In file included from include/linux/ieee80211.h:20:
In file included from include/linux/etherdevice.h:21:
In file included from include/linux/netdevice.h:37:
include/linux/ethtool.h:26:2: error: unknown type name 'compat_u64'
compat_u64 ring_cookie;
^
include/linux/ethtool.h:33:2: error: unknown type name 'compat_u64'
compat_u64 data;
^
>> drivers/net/wireless/intel/iwlwifi/pcie/trans.c:1923:35: warning: implicit conversion from 'unsigned long long' to 'u32' (aka 'unsigned int') changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
trans_pcie->supported_dma_mask = DMA_BIT_MASK(12);
~ ^~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:139:40: note: expanded from macro 'DMA_BIT_MASK'
#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
^~~~~
drivers/net/wireless/intel/iwlwifi/pcie/trans.c:1925:36: warning: implicit conversion from 'unsigned long long' to 'u32' (aka 'unsigned int') changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
trans_pcie->supported_dma_mask = DMA_BIT_MASK(11);
~ ^~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:139:40: note: expanded from macro 'DMA_BIT_MASK'
#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
^~~~~
2 warnings and 2 errors generated.
vim +606 drivers/net/wireless/marvell/mwifiex/sta_cmd.c
5e6e3a92b9a4c94 drivers/net/wireless/mwifiex/sta_cmd.c Bing Zhao 2011-03-21 588
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 589 /* This function populates key material v2 command
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 590 * to set network key for AES & CMAC AES.
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 591 */
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 592 static int mwifiex_set_aes_key_v2(struct mwifiex_private *priv,
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 593 struct host_cmd_ds_command *cmd,
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 594 struct mwifiex_ds_encrypt_key *enc_key,
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 595 struct host_cmd_ds_802_11_key_material_v2 *km)
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 596 {
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 597 struct mwifiex_adapter *adapter = priv->adapter;
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 598 u16 size, len = KEY_PARAMS_FIXED_LEN;
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 599
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 600 if (enc_key->is_igtk_key) {
acebe8c10a6eabd drivers/net/wireless/mwifiex/sta_cmd.c Zhaoyang Liu 2015-05-12 601 mwifiex_dbg(adapter, INFO,
acebe8c10a6eabd drivers/net/wireless/mwifiex/sta_cmd.c Zhaoyang Liu 2015-05-12 602 "%s: Set CMAC AES Key\n", __func__);
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 603 if (enc_key->is_rx_seq_valid)
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 604 memcpy(km->key_param_set.key_params.cmac_aes.ipn,
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 605 enc_key->pn, enc_key->pn_len);
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 @606 km->key_param_set.key_info &= cpu_to_le16(~KEY_MCAST);
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 607 km->key_param_set.key_info |= cpu_to_le16(KEY_IGTK);
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 608 km->key_param_set.key_type = KEY_TYPE_ID_AES_CMAC;
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 609 km->key_param_set.key_params.cmac_aes.key_len =
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 610 cpu_to_le16(enc_key->key_len);
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 611 memcpy(km->key_param_set.key_params.cmac_aes.key,
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 612 enc_key->key_material, enc_key->key_len);
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 613 len += sizeof(struct mwifiex_cmac_aes_param);
89951db2be53106 drivers/net/wireless/marvell/mwifiex/sta_cmd.c Ganapathi Bhat 2016-09-20 614 } else if (enc_key->is_igtk_def_key) {
89951db2be53106 drivers/net/wireless/marvell/mwifiex/sta_cmd.c Ganapathi Bhat 2016-09-20 615 mwifiex_dbg(adapter, INFO,
89951db2be53106 drivers/net/wireless/marvell/mwifiex/sta_cmd.c Ganapathi Bhat 2016-09-20 616 "%s: Set CMAC default Key index\n", __func__);
89951db2be53106 drivers/net/wireless/marvell/mwifiex/sta_cmd.c Ganapathi Bhat 2016-09-20 617 km->key_param_set.key_type = KEY_TYPE_ID_AES_CMAC_DEF;
89951db2be53106 drivers/net/wireless/marvell/mwifiex/sta_cmd.c Ganapathi Bhat 2016-09-20 618 km->key_param_set.key_idx = enc_key->key_index & KEY_INDEX_MASK;
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 619 } else {
acebe8c10a6eabd drivers/net/wireless/mwifiex/sta_cmd.c Zhaoyang Liu 2015-05-12 620 mwifiex_dbg(adapter, INFO,
acebe8c10a6eabd drivers/net/wireless/mwifiex/sta_cmd.c Zhaoyang Liu 2015-05-12 621 "%s: Set AES Key\n", __func__);
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 622 if (enc_key->is_rx_seq_valid)
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 623 memcpy(km->key_param_set.key_params.aes.pn,
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 624 enc_key->pn, enc_key->pn_len);
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 625 km->key_param_set.key_type = KEY_TYPE_ID_AES;
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 626 km->key_param_set.key_params.aes.key_len =
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 627 cpu_to_le16(enc_key->key_len);
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 628 memcpy(km->key_param_set.key_params.aes.key,
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 629 enc_key->key_material, enc_key->key_len);
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 630 len += sizeof(struct mwifiex_aes_param);
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 631 }
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 632
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 633 km->key_param_set.len = cpu_to_le16(len);
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 634 size = len + sizeof(struct mwifiex_ie_types_header) +
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 635 sizeof(km->action) + S_DS_GEN;
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 636 cmd->size = cpu_to_le16(size);
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 637
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 638 return 0;
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 639 }
e57f1734d87aa0e drivers/net/wireless/mwifiex/sta_cmd.c Avinash Patil 2014-02-07 640
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 38530 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v2 1/2] ethtool: improve compat ioctl handling
2020-09-25 13:22 [PATCH net-next v2 1/2] ethtool: improve compat ioctl handling Arnd Bergmann
2020-09-25 13:22 ` [PATCH net-next v2 2/2] dev_ioctl: split out SIOC?IFMAP ioctls Arnd Bergmann
2020-09-25 18:31 ` [PATCH net-next v2 1/2] ethtool: improve compat ioctl handling kernel test robot
@ 2020-09-25 18:42 ` Arnd Bergmann
2020-09-25 19:36 ` kernel test robot
3 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2020-09-25 18:42 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski
Cc: Christoph Hellwig, Andrew Lunn, Florian Fainelli, Michal Kubecek,
Jens Axboe, linux-kernel, Networking
On Fri, Sep 25, 2020 at 3:22 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> 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>
The kbuild bot found another dependency on a patch that I had in my
testing tree (moving compat_u64). Let's drop both patches for now, I'll
resend once that has been merged.
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v2 1/2] ethtool: improve compat ioctl handling
2020-09-25 13:22 [PATCH net-next v2 1/2] ethtool: improve compat ioctl handling Arnd Bergmann
` (2 preceding siblings ...)
2020-09-25 18:42 ` Arnd Bergmann
@ 2020-09-25 19:36 ` kernel test robot
3 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-09-25 19:36 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 6860 bytes --]
Hi Arnd,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Arnd-Bergmann/ethtool-improve-compat-ioctl-handling/20200925-212440
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git aafe8853f5e2bcbdd231411aec218b8c5dc78437
config: arm64-randconfig-r032-20200925 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project c32e69b2ce7abfb151a87ba363ac9e25abf7d417)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/7a728784f051b3bce6ddedb8005b01d597e0342d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Arnd-Bergmann/ethtool-improve-compat-ioctl-handling/20200925-212440
git checkout 7a728784f051b3bce6ddedb8005b01d597e0342d
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from drivers/net/ethernet/apm/xgene-v2/main.c:10:
In file included from drivers/net/ethernet/apm/xgene-v2/main.h:16:
In file included from include/linux/if_vlan.h:10:
In file included from include/linux/netdevice.h:37:
include/linux/ethtool.h:26:2: error: unknown type name 'compat_u64'
compat_u64 ring_cookie;
^
include/linux/ethtool.h:33:2: error: unknown type name 'compat_u64'
compat_u64 data;
^
>> drivers/net/ethernet/apm/xgene-v2/main.c:661:42: warning: shift count >= width of type [-Wshift-count-overflow]
ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
^~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:139:54: note: expanded from macro 'DMA_BIT_MASK'
#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
^ ~~~
1 warning and 2 errors generated.
vim +661 drivers/net/ethernet/apm/xgene-v2/main.c
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 631
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 632 static int xge_probe(struct platform_device *pdev)
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 633 {
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 634 struct device *dev = &pdev->dev;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 635 struct net_device *ndev;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 636 struct xge_pdata *pdata;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 637 int ret;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 638
1ffa8a7aa51647 Iyappan Subramanian 2017-03-21 639 ndev = alloc_etherdev(sizeof(*pdata));
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 640 if (!ndev)
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 641 return -ENOMEM;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 642
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 643 pdata = netdev_priv(ndev);
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 644
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 645 pdata->pdev = pdev;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 646 pdata->ndev = ndev;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 647 SET_NETDEV_DEV(ndev, dev);
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 648 platform_set_drvdata(pdev, pdata);
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 649 ndev->netdev_ops = &xgene_ndev_ops;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 650
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 651 ndev->features |= NETIF_F_GSO |
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 652 NETIF_F_GRO;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 653
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 654 ret = xge_get_resources(pdata);
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 655 if (ret)
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 656 goto err;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 657
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 658 ndev->hw_features = ndev->features;
617d795c7cb2d1 Iyappan Subramanian 2017-03-21 659 xge_set_ethtool_ops(ndev);
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 660
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 @661 ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 662 if (ret) {
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 663 netdev_err(ndev, "No usable DMA configuration\n");
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 664 goto err;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 665 }
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 666
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 667 ret = xge_init_hw(ndev);
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 668 if (ret)
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 669 goto err;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 670
ea8ab16ab225a6 Iyappan Subramanian 2017-03-21 671 ret = xge_mdio_config(ndev);
ea8ab16ab225a6 Iyappan Subramanian 2017-03-21 672 if (ret)
ea8ab16ab225a6 Iyappan Subramanian 2017-03-21 673 goto err;
ea8ab16ab225a6 Iyappan Subramanian 2017-03-21 674
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 675 netif_napi_add(ndev, &pdata->napi, xge_napi, NAPI_POLL_WEIGHT);
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 676
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 677 ret = register_netdev(ndev);
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 678 if (ret) {
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 679 netdev_err(ndev, "Failed to register netdev\n");
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 680 goto err;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 681 }
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 682
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 683 return 0;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 684
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 685 err:
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 686 free_netdev(ndev);
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 687
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 688 return ret;
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 689 }
3b3f9a75d93186 Iyappan Subramanian 2017-03-07 690
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 41364 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-25 19:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-25 13:22 [PATCH net-next v2 1/2] ethtool: improve compat ioctl handling Arnd Bergmann
2020-09-25 13:22 ` [PATCH net-next v2 2/2] dev_ioctl: split out SIOC?IFMAP ioctls Arnd Bergmann
2020-09-25 18:31 ` [PATCH net-next v2 1/2] ethtool: improve compat ioctl handling kernel test robot
2020-09-25 18:42 ` Arnd Bergmann
2020-09-25 19:36 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.