* [PATCH net-next v6 1/6] compat: make linux/compat.h available everywhere
2021-07-22 14:28 [PATCH net-next v6 0/6] remove compat_alloc_user_space() Arnd Bergmann
@ 2021-07-22 14:28 ` Arnd Bergmann
2021-07-22 16:22 ` Christoph Hellwig
2021-07-22 14:28 ` [PATCH net-next v6 2/6] ethtool: improve compat ioctl handling Arnd Bergmann
` (5 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2021-07-22 14:28 UTC (permalink / raw)
To: netdev
Cc: Arnd Bergmann, Al Viro, Andrew Lunn, Christoph Hellwig,
David Ahern, David S. Miller, Eric Dumazet, Hideaki YOSHIFUJI,
Jakub Kicinski, Kees Cook, Marco Elver, linux-kernel, linux-arch
From: Arnd Bergmann <arnd@arndb.de>
Parts of linux/compat.h are under an #ifdef, but we end up
using more of those over time, moving things around bit by
bit.
To get it over with once and for all, make all of this file
uncondititonal now so it can be accessed everywhere. There
are only a few types left that are in asm/compat.h but not
yet in the asm-generic version, so add those in the process.
This requires providing a few more types in asm-generic/compat.h
that were not already there. The only tricky one is
compat_sigset_t, which needs a little help on 32-bit architectures
and for x86.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Changed in v6:
- split out linux/compat.h changes from multiple patches into
a separate change
- Do it all at once
---
arch/arm64/include/asm/compat.h | 14 +++-----------
arch/mips/include/asm/compat.h | 24 +++++++++++------------
arch/parisc/include/asm/compat.h | 14 +++-----------
arch/powerpc/include/asm/compat.h | 11 -----------
arch/s390/include/asm/compat.h | 14 +++-----------
arch/sparc/include/asm/compat.h | 14 +++-----------
arch/x86/include/asm/compat.h | 14 +++-----------
arch/x86/include/asm/signal.h | 1 +
include/asm-generic/compat.h | 17 ++++++++++++++++
include/linux/compat.h | 32 +++++++++++++++----------------
10 files changed, 59 insertions(+), 96 deletions(-)
diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index 23a9fb73c04f..79c1a750e357 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -5,6 +5,9 @@
#ifndef __ASM_COMPAT_H
#define __ASM_COMPAT_H
+#define compat_mode_t compat_mode_t
+typedef u16 compat_mode_t;
+
#include <asm-generic/compat.h>
#ifdef CONFIG_COMPAT
@@ -27,13 +30,9 @@ typedef u16 __compat_uid_t;
typedef u16 __compat_gid_t;
typedef u16 __compat_uid16_t;
typedef u16 __compat_gid16_t;
-typedef u32 __compat_uid32_t;
-typedef u32 __compat_gid32_t;
-typedef u16 compat_mode_t;
typedef u32 compat_dev_t;
typedef s32 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
-typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
struct compat_stat {
@@ -103,13 +102,6 @@ struct compat_statfs {
#define COMPAT_RLIM_INFINITY 0xffffffff
-typedef u32 compat_old_sigset_t;
-
-#define _COMPAT_NSIG 64
-#define _COMPAT_NSIG_BPW 32
-
-typedef u32 compat_sigset_word;
-
#define COMPAT_OFF_T_MAX 0x7fffffff
#define compat_user_stack_pointer() (user_stack_pointer(task_pt_regs(current)))
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index 65975712a22d..53f015a1b0a7 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -9,20 +9,25 @@
#include <asm/page.h>
#include <asm/ptrace.h>
+typedef s32 __compat_uid_t;
+typedef s32 __compat_gid_t;
+typedef __compat_uid_t __compat_uid32_t;
+typedef __compat_gid_t __compat_gid32_t;
+#define __compat_uid32_t __compat_uid32_t
+#define __compat_gid32_t __compat_gid32_t
+
+#define _COMPAT_NSIG 128 /* Don't ask !$@#% ... */
+#define _COMPAT_NSIG_BPW 32
+typedef u32 compat_sigset_word;
+
#include <asm-generic/compat.h>
#define COMPAT_USER_HZ 100
#define COMPAT_UTS_MACHINE "mips\0\0\0"
-typedef s32 __compat_uid_t;
-typedef s32 __compat_gid_t;
-typedef __compat_uid_t __compat_uid32_t;
-typedef __compat_gid_t __compat_gid32_t;
-typedef u32 compat_mode_t;
typedef u32 compat_dev_t;
typedef u32 compat_nlink_t;
typedef s32 compat_ipc_pid_t;
-typedef s32 compat_caddr_t;
typedef struct {
s32 val[2];
} compat_fsid_t;
@@ -89,13 +94,6 @@ struct compat_statfs {
#define COMPAT_RLIM_INFINITY 0x7fffffffUL
-typedef u32 compat_old_sigset_t; /* at least 32 bits */
-
-#define _COMPAT_NSIG 128 /* Don't ask !$@#% ... */
-#define _COMPAT_NSIG_BPW 32
-
-typedef u32 compat_sigset_word;
-
#define COMPAT_OFF_T_MAX 0x7fffffff
static inline void __user *arch_compat_alloc_user_space(long len)
diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h
index 1a609d38f667..b5d90e82b65d 100644
--- a/arch/parisc/include/asm/compat.h
+++ b/arch/parisc/include/asm/compat.h
@@ -8,6 +8,9 @@
#include <linux/sched.h>
#include <linux/thread_info.h>
+#define compat_mode_t compat_mode_t
+typedef u16 compat_mode_t;
+
#include <asm-generic/compat.h>
#define COMPAT_USER_HZ 100
@@ -15,13 +18,9 @@
typedef u32 __compat_uid_t;
typedef u32 __compat_gid_t;
-typedef u32 __compat_uid32_t;
-typedef u32 __compat_gid32_t;
-typedef u16 compat_mode_t;
typedef u32 compat_dev_t;
typedef u16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
-typedef u32 compat_caddr_t;
struct compat_stat {
compat_dev_t st_dev; /* dev_t is 32 bits on parisc */
@@ -96,13 +95,6 @@ struct compat_sigcontext {
#define COMPAT_RLIM_INFINITY 0xffffffff
-typedef u32 compat_old_sigset_t; /* at least 32 bits */
-
-#define _COMPAT_NSIG 64
-#define _COMPAT_NSIG_BPW 32
-
-typedef u32 compat_sigset_word;
-
#define COMPAT_OFF_T_MAX 0x7fffffff
struct compat_ipc64_perm {
diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
index 9191fc29e6ed..e33dcf134cdd 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -19,13 +19,9 @@
typedef u32 __compat_uid_t;
typedef u32 __compat_gid_t;
-typedef u32 __compat_uid32_t;
-typedef u32 __compat_gid32_t;
-typedef u32 compat_mode_t;
typedef u32 compat_dev_t;
typedef s16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
-typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
struct compat_stat {
@@ -85,13 +81,6 @@ struct compat_statfs {
#define COMPAT_RLIM_INFINITY 0xffffffff
-typedef u32 compat_old_sigset_t;
-
-#define _COMPAT_NSIG 64
-#define _COMPAT_NSIG_BPW 32
-
-typedef u32 compat_sigset_word;
-
#define COMPAT_OFF_T_MAX 0x7fffffff
static inline void __user *arch_compat_alloc_user_space(long len)
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index ea5b9c34b7be..8d49505b4a43 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -9,6 +9,9 @@
#include <linux/sched/task_stack.h>
#include <linux/thread_info.h>
+#define compat_mode_t compat_mode_t
+typedef u16 compat_mode_t;
+
#include <asm-generic/compat.h>
#define __TYPE_IS_PTR(t) (!__builtin_types_compatible_p( \
@@ -55,13 +58,9 @@
typedef u16 __compat_uid_t;
typedef u16 __compat_gid_t;
-typedef u32 __compat_uid32_t;
-typedef u32 __compat_gid32_t;
-typedef u16 compat_mode_t;
typedef u16 compat_dev_t;
typedef u16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
-typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
typedef struct {
@@ -155,13 +154,6 @@ struct compat_statfs64 {
#define COMPAT_RLIM_INFINITY 0xffffffff
-typedef u32 compat_old_sigset_t; /* at least 32 bits */
-
-#define _COMPAT_NSIG 64
-#define _COMPAT_NSIG_BPW 32
-
-typedef u32 compat_sigset_word;
-
#define COMPAT_OFF_T_MAX 0x7fffffff
/*
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index b85842cda99f..8b63410e830f 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -6,6 +6,9 @@
*/
#include <linux/types.h>
+#define compat_mode_t compat_mode_t
+typedef u16 compat_mode_t;
+
#include <asm-generic/compat.h>
#define COMPAT_USER_HZ 100
@@ -13,13 +16,9 @@
typedef u16 __compat_uid_t;
typedef u16 __compat_gid_t;
-typedef u32 __compat_uid32_t;
-typedef u32 __compat_gid32_t;
-typedef u16 compat_mode_t;
typedef u16 compat_dev_t;
typedef s16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
-typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
struct compat_stat {
@@ -115,13 +114,6 @@ struct compat_statfs {
#define COMPAT_RLIM_INFINITY 0x7fffffff
-typedef u32 compat_old_sigset_t;
-
-#define _COMPAT_NSIG 64
-#define _COMPAT_NSIG_BPW 32
-
-typedef u32 compat_sigset_word;
-
#define COMPAT_OFF_T_MAX 0x7fffffff
#ifdef CONFIG_COMPAT
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index be09c7eac89f..4ae01cdb99de 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -12,6 +12,9 @@
#include <asm/user32.h>
#include <asm/unistd.h>
+#define compat_mode_t compat_mode_t
+typedef u16 compat_mode_t;
+
#include <asm-generic/compat.h>
#define COMPAT_USER_HZ 100
@@ -19,13 +22,9 @@
typedef u16 __compat_uid_t;
typedef u16 __compat_gid_t;
-typedef u32 __compat_uid32_t;
-typedef u32 __compat_gid32_t;
-typedef u16 compat_mode_t;
typedef u16 compat_dev_t;
typedef u16 compat_nlink_t;
typedef u16 compat_ipc_pid_t;
-typedef u32 compat_caddr_t;
typedef __kernel_fsid_t compat_fsid_t;
struct compat_stat {
@@ -92,13 +91,6 @@ struct compat_statfs {
#define COMPAT_RLIM_INFINITY 0xffffffff
-typedef u32 compat_old_sigset_t; /* at least 32 bits */
-
-#define _COMPAT_NSIG 64
-#define _COMPAT_NSIG_BPW 32
-
-typedef u32 compat_sigset_word;
-
#define COMPAT_OFF_T_MAX 0x7fffffff
struct compat_ipc64_perm {
diff --git a/arch/x86/include/asm/signal.h b/arch/x86/include/asm/signal.h
index 6fd8410a3910..2dfb5fea13af 100644
--- a/arch/x86/include/asm/signal.h
+++ b/arch/x86/include/asm/signal.h
@@ -29,6 +29,7 @@ typedef struct {
#define SA_X32_ABI 0x01000000u
#ifndef CONFIG_COMPAT
+#define compat_sigset_t compat_sigset_t
typedef sigset_t compat_sigset_t;
#endif
diff --git a/include/asm-generic/compat.h b/include/asm-generic/compat.h
index 30f7b18a36f9..d46c0201cc34 100644
--- a/include/asm-generic/compat.h
+++ b/include/asm-generic/compat.h
@@ -20,7 +20,18 @@ typedef u16 compat_ushort_t;
typedef u32 compat_uint_t;
typedef u32 compat_ulong_t;
typedef u32 compat_uptr_t;
+typedef u32 compat_caddr_t;
typedef u32 compat_aio_context_t;
+typedef u32 compat_old_sigset_t;
+
+#ifndef __compat_uid32_t
+typedef u32 __compat_uid32_t;
+typedef u32 __compat_gid32_t;
+#endif
+
+#ifndef compat_mode_t
+typedef u32 compat_mode_t;
+#endif
#ifdef CONFIG_COMPAT_FOR_U64_ALIGNMENT
typedef s64 __attribute__((aligned(4))) compat_s64;
@@ -30,4 +41,10 @@ typedef s64 compat_s64;
typedef u64 compat_u64;
#endif
+#ifndef _COMPAT_NSIG
+typedef u32 compat_sigset_word;
+#define _COMPAT_NSIG _NSIG
+#define _COMPAT_NSIG_BPW 32
+#endif
+
#endif
diff --git a/include/linux/compat.h b/include/linux/compat.h
index c270124e4402..8e0598c7d1d1 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -20,11 +20,8 @@
#include <linux/unistd.h>
#include <asm/compat.h>
-
-#ifdef CONFIG_COMPAT
#include <asm/siginfo.h>
#include <asm/signal.h>
-#endif
#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
/*
@@ -95,8 +92,6 @@ struct compat_iovec {
compat_size_t iov_len;
};
-#ifdef CONFIG_COMPAT
-
#ifndef compat_user_stack_pointer
#define compat_user_stack_pointer() current_user_stack_pointer()
#endif
@@ -131,9 +126,11 @@ struct compat_tms {
#define _COMPAT_NSIG_WORDS (_COMPAT_NSIG / _COMPAT_NSIG_BPW)
+#ifndef compat_sigset_t
typedef struct {
compat_sigset_word sig[_COMPAT_NSIG_WORDS];
} compat_sigset_t;
+#endif
int set_compat_user_sigmask(const compat_sigset_t __user *umask,
size_t sigsetsize);
@@ -384,6 +381,7 @@ struct compat_keyctl_kdf_params {
__u32 __spare[8];
};
+struct compat_stat;
struct compat_statfs;
struct compat_statfs64;
struct compat_old_linux_dirent;
@@ -428,7 +426,7 @@ put_compat_sigset(compat_sigset_t __user *compat, const sigset_t *set,
unsigned int size)
{
/* size <= sizeof(compat_sigset_t) <= sizeof(sigset_t) */
-#ifdef __BIG_ENDIAN
+#if defined(__BIG_ENDIAN) && defined(CONFIG_64BIT)
compat_sigset_t v;
switch (_NSIG_WORDS) {
case 4: v.sig[7] = (set->sig[3] >> 32); v.sig[6] = set->sig[3];
@@ -929,17 +927,6 @@ asmlinkage long compat_sys_socketcall(int call, u32 __user *args);
#endif /* CONFIG_ARCH_HAS_SYSCALL_WRAPPER */
-
-/*
- * For most but not all architectures, "am I in a compat syscall?" and
- * "am I a compat task?" are the same question. For architectures on which
- * they aren't the same question, arch code can override in_compat_syscall.
- */
-
-#ifndef in_compat_syscall
-static inline bool in_compat_syscall(void) { return is_compat_task(); }
-#endif
-
/**
* ns_to_old_timeval32 - Compat version of ns_to_timeval
* @nsec: the nanoseconds value to be converted
@@ -969,6 +956,17 @@ int kcompat_sys_statfs64(const char __user * pathname, compat_size_t sz,
int kcompat_sys_fstatfs64(unsigned int fd, compat_size_t sz,
struct compat_statfs64 __user * buf);
+#ifdef CONFIG_COMPAT
+
+/*
+ * For most but not all architectures, "am I in a compat syscall?" and
+ * "am I a compat task?" are the same question. For architectures on which
+ * they aren't the same question, arch code can override in_compat_syscall.
+ */
+#ifndef in_compat_syscall
+static inline bool in_compat_syscall(void) { return is_compat_task(); }
+#endif
+
#else /* !CONFIG_COMPAT */
#define is_compat_task() (0)
--
2.29.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net-next v6 1/6] compat: make linux/compat.h available everywhere
2021-07-22 14:28 ` [PATCH net-next v6 1/6] compat: make linux/compat.h available everywhere Arnd Bergmann
@ 2021-07-22 16:22 ` Christoph Hellwig
0 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2021-07-22 16:22 UTC (permalink / raw)
To: Arnd Bergmann
Cc: netdev, Arnd Bergmann, Al Viro, Andrew Lunn, Christoph Hellwig,
David Ahern, David S. Miller, Eric Dumazet, Hideaki YOSHIFUJI,
Jakub Kicinski, Kees Cook, Marco Elver, linux-kernel, linux-arch
On Thu, Jul 22, 2021 at 04:28:58PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Parts of linux/compat.h are under an #ifdef, but we end up
> using more of those over time, moving things around bit by
> bit.
>
> To get it over with once and for all, make all of this file
> uncondititonal now so it can be accessed everywhere. There
> are only a few types left that are in asm/compat.h but not
> yet in the asm-generic version, so add those in the process.
>
> This requires providing a few more types in asm-generic/compat.h
> that were not already there. The only tricky one is
> compat_sigset_t, which needs a little help on 32-bit architectures
> and for x86.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v6 2/6] ethtool: improve compat ioctl handling
2021-07-22 14:28 [PATCH net-next v6 0/6] remove compat_alloc_user_space() Arnd Bergmann
2021-07-22 14:28 ` [PATCH net-next v6 1/6] compat: make linux/compat.h available everywhere Arnd Bergmann
@ 2021-07-22 14:28 ` Arnd Bergmann
2021-07-23 23:50 ` Shannon Nelson
2021-07-22 14:29 ` [PATCH net-next v6 3/6] net: socket: rework SIOC?IFMAP ioctls Arnd Bergmann
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2021-07-22 14:28 UTC (permalink / raw)
To: netdev
Cc: Arnd Bergmann, Al Viro, Andrew Lunn, Christoph Hellwig,
David Ahern, David S. Miller, Eric Dumazet, Hideaki YOSHIFUJI,
Jakub Kicinski, Kees Cook, Marco Elver, linux-kernel, linux-arch
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] 13+ messages in thread* Re: [PATCH net-next v6 2/6] ethtool: improve compat ioctl handling
2021-07-22 14:28 ` [PATCH net-next v6 2/6] ethtool: improve compat ioctl handling Arnd Bergmann
@ 2021-07-23 23:50 ` Shannon Nelson
0 siblings, 0 replies; 13+ messages in thread
From: Shannon Nelson @ 2021-07-23 23:50 UTC (permalink / raw)
To: Arnd Bergmann, netdev
Cc: Arnd Bergmann, Al Viro, Andrew Lunn, Christoph Hellwig,
David Ahern, David S. Miller, Eric Dumazet, Hideaki YOSHIFUJI,
Jakub Kicinski, Kees Cook, Marco Elver, linux-kernel, linux-arch
On 7/22/21 7:28 AM, Arnd Bergmann wrote:
> 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>
Something's odd here... I didn't dive in to find the actual problem,
but here's what I did find. This doesn't happen before the patchset,
but does happen after the patchset. I built the kernel on a RHEL 8.4
box using their config file as a basis.
[root@sln-dev ~]# ethtool --version
ethtool version 5.8
[root@sln-dev ~]# ethtool -i eno1
driver: tg3
version: 5.14.0-rc2-net-next-sln+
firmware-version: 5719-v1.46 NCSI v1.5.18.0
expansion-rom-version:
bus-info: 0000:02:00.0
supports-statistics: yes
supports-test: yes
supports-eeprom-access: yes
supports-register-dump: yes
supports-priv-flags: no
[root@sln-dev ~]# ethtool -x eno1
Cannot get RX ring count: Bad address
And we get a stack trace here:
[ 1221.631085] ------------[ cut here ]------------
[ 1221.631105] Buffer overflow detected (8 < 192)!
[ 1221.631125] WARNING: CPU: 27 PID: 2363 at
include/linux/thread_info.h:200 ethtool_rxnfc_copy_to_user+0x2b/0xb0
[ 1221.631150] Modules linked in: xt_CHECKSUM xt_MASQUERADE xt_conntrack
ipt_REJECT nf_reject_ipv4 nft_compat nft_counter nft_chain_nat nf_nat
nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables nfnetlink tun
bridge stp llc intel_rapl_msr intel_rapl_common isst_if_common rpcrdma
rdma_ucm ib_srpt ib_isert iscsi_target_mod target_core_mod nfit ib_iser
libnvdimm libiscsi scsi_transport_iscsi ib_umad ib_ipoib rdma_cm rfkill
x86_pkg_temp_thermal intel_powerclamp iw_cm ib_cm coretemp kvm_intel kvm
sunrpc mlx5_ib irqbypass crct10dif_pclmul crc32_pclmul
ghash_clmulni_intel ib_uverbs rapl ipmi_ssif intel_cstate ib_core
intel_uncore mei_me pcspkr ses mei enclosure acpi_ipmi hpwdt hpilo
ioatdma intel_pch_thermal ipmi_si lpc_ich dca ipmi_devintf
ipmi_msghandler acpi_tad acpi_power_meter ip_tables xfs libcrc32c sd_mod
t10_pi sg mlx5_core mgag200 drm_kms_helper syscopyarea sysfillrect
sysimgblt fb_sys_fops drm mlxfw pci_hyperv_intf ionic tls crc32c_intel
smartpqi serio_raw tg3 scsi_transport_sas
[ 1221.631199] psample i2c_algo_bit wmi dm_mirror dm_region_hash dm_log
dm_mod fuse
[ 1221.631364] CPU: 27 PID: 2363 Comm: ethtool Tainted: G S W
5.14.0-rc2-net-next-sln+ #7
[ 1221.631384] Hardware name: HPE ProLiant DL360 Gen10/ProLiant DL360
Gen10, BIOS U32 01/23/2021
[ 1221.631401] RIP: 0010:ethtool_rxnfc_copy_to_user+0x2b/0xb0
[ 1221.631416] Code: 1f 44 00 00 41 55 65 48 8b 04 25 00 6f 01 00 41 54
55 53 f6 40 10 02 75 23 be 08 00 00 00 48 c7 c7 20 f2 f1 89 e8 32 7d 14
00 <0f> 0b 41 bd f2 ff ff ff 5b 44 89 e8 5d 41 5c 41 5d c3 48 89 fd 48
[ 1221.631451] RSP: 0018:ffffadc906e5bbd8 EFLAGS: 00010286
[ 1221.631463] RAX: 0000000000000000 RBX: ffffadc906e5bc00 RCX:
0000000000000027
[ 1221.631478] RDX: 0000000000000027 RSI: ffff91163f857c80 RDI:
ffff91163f857c88
[ 1221.631492] RBP: 0000000000000000 R08: 0000000000000000 R09:
c0000000ffff7fff
[ 1221.631507] R10: 0000000000000001 R11: ffffadc906e5b9e8 R12:
0000000000000000
[ 1221.631535] R13: 00007ffc5be4c730 R14: 00000000000000c0 R15:
ffff90f83b558000
[ 1221.631551] FS: 00007fb10a942740(0000) GS:ffff91163f840000(0000)
knlGS:0000000000000000
[ 1221.631584] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1221.631598] CR2: 000055ae4763a6f0 CR3: 0000001053530001 CR4:
00000000007706e0
[ 1221.631621] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 1221.631636] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[ 1221.631667] PKRU: 55555554
[ 1221.631676] Call Trace:
[ 1221.631686] ethtool_get_rxnfc+0xe8/0x1a0
[ 1221.631700] dev_ethtool+0xb1a/0x2a20
[ 1221.631711] ? do_set_pte+0xcb/0x110
[ 1221.632313] ? inet_ioctl+0x158/0x1a0
[ 1221.632849] ? page_counter_try_charge+0x57/0xc0
[ 1221.633374] ? __cond_resched+0x15/0x30
[ 1221.633880] dev_ioctl+0xb5/0x4e0
[ 1221.634374] sock_do_ioctl+0x92/0xd0
[ 1221.634867] sock_ioctl+0x246/0x340
[ 1221.635335] __x64_sys_ioctl+0x81/0xc0
[ 1221.635817] do_syscall_64+0x37/0x80
[ 1221.636281] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 1221.636760] RIP: 0033:0x7fb109ed062b
[ 1221.637210] Code: 0f 1e fa 48 8b 05 5d b8 2c 00 64 c7 00 26 00 00 00
48 c7 c0 ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 00 0f
05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 2d b8 2c 00 f7 d8 64 89 01 48
[ 1221.638151] RSP: 002b:00007ffc5be4c6f8 EFLAGS: 00000246 ORIG_RAX:
0000000000000010
[ 1221.638649] RAX: ffffffffffffffda RBX: 0000000000000000 RCX:
00007fb109ed062b
[ 1221.639135] RDX: 00007ffc5be4c870 RSI: 0000000000008946 RDI:
0000000000000003
[ 1221.639647] RBP: 00007ffc5be4c860 R08: 00007ffc5be4c870 R09:
0000000000000001
[ 1221.640154] R10: 000055ae4764a934 R11: 0000000000000246 R12:
000055ae47613b60
[ 1221.640647] R13: 00007ffc5be4c870 R14: 000055ae4764718e R15:
000055ae47647196
[ 1221.641124] ---[ end trace 422c6846895775bd ]---
Cheers,
sln
> ---
> 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:
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v6 3/6] net: socket: rework SIOC?IFMAP ioctls
2021-07-22 14:28 [PATCH net-next v6 0/6] remove compat_alloc_user_space() Arnd Bergmann
2021-07-22 14:28 ` [PATCH net-next v6 1/6] compat: make linux/compat.h available everywhere Arnd Bergmann
2021-07-22 14:28 ` [PATCH net-next v6 2/6] ethtool: improve compat ioctl handling Arnd Bergmann
@ 2021-07-22 14:29 ` Arnd Bergmann
2021-07-22 14:29 ` [PATCH net-next v6 4/6] net: socket: remove register_gifconf Arnd Bergmann
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2021-07-22 14:29 UTC (permalink / raw)
To: netdev
Cc: Arnd Bergmann, Al Viro, Andrew Lunn, Christoph Hellwig,
David Ahern, David S. Miller, Eric Dumazet, Hideaki YOSHIFUJI,
Jakub Kicinski, Kees Cook, Marco Elver, linux-kernel, linux-arch
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.
Reviewed-by: Christoph Hellwig <hch@lst.de>
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)
---
net/core/dev_ioctl.c | 65 +++++++++++++++++++++++++++++++++++---------
net/socket.c | 39 ++------------------------
2 files changed, 54 insertions(+), 50 deletions(-)
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 478d032f34ac..62f45da7ecfe 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -98,6 +98,56 @@ 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;
+
+ if (in_compat_syscall()) {
+ struct compat_ifmap *cifmap = (struct compat_ifmap *)ifmap;
+
+ 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 +178,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 +319,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] 13+ messages in thread* [PATCH net-next v6 4/6] net: socket: remove register_gifconf
2021-07-22 14:28 [PATCH net-next v6 0/6] remove compat_alloc_user_space() Arnd Bergmann
` (2 preceding siblings ...)
2021-07-22 14:29 ` [PATCH net-next v6 3/6] net: socket: rework SIOC?IFMAP ioctls Arnd Bergmann
@ 2021-07-22 14:29 ` Arnd Bergmann
2021-07-22 16:24 ` Christoph Hellwig
2021-07-22 14:29 ` [PATCH net-next v6 5/6] net: socket: simplify dev_ifconf handling Arnd Bergmann
` (2 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2021-07-22 14:29 UTC (permalink / raw)
To: netdev
Cc: Arnd Bergmann, Al Viro, Andrew Lunn, Christoph Hellwig,
David Ahern, David S. Miller, Eric Dumazet, Hideaki YOSHIFUJI,
Jakub Kicinski, Kees Cook, Marco Elver, linux-kernel, linux-arch
From: Arnd Bergmann <arnd@arndb.de>
Since dynamic registration of the gifconf() helper is only used for
IPv4, and this can not be in a loadable module, this can be simplified
noticeably by turning it into a direct function call as a preparation
for cleaning up the compat handling.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
changes in v6:
- split register_gifconf from compat changes
---
include/linux/inetdevice.h | 9 ++++++++
include/linux/netdevice.h | 8 -------
net/core/dev_ioctl.c | 43 +++++++++-----------------------------
net/ipv4/devinet.c | 4 +---
4 files changed, 20 insertions(+), 44 deletions(-)
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 42f6f866d5f3..6630a9f0b0f0 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 {
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 62f45da7ecfe..c22c3dc15ce9 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,26 +26,6 @@ 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.
@@ -72,19 +53,15 @@ int dev_ifconf(struct net *net, struct ifconf *ifc, int size)
total = 0;
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;
- }
- }
+ int done;
+ if (!pos)
+ done = inet_gifconf(dev, NULL, 0, size);
+ else
+ done = inet_gifconf(dev, pos + total,
+ len - total, size);
+ if (done < 0)
+ return -EFAULT;
+ total += done;
}
/*
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 94b648d9eaff..c82aded8da7d 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;
@@ -2766,8 +2766,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);
--
2.29.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net-next v6 4/6] net: socket: remove register_gifconf
2021-07-22 14:29 ` [PATCH net-next v6 4/6] net: socket: remove register_gifconf Arnd Bergmann
@ 2021-07-22 16:24 ` Christoph Hellwig
0 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2021-07-22 16:24 UTC (permalink / raw)
To: Arnd Bergmann
Cc: netdev, Arnd Bergmann, Al Viro, Andrew Lunn, Christoph Hellwig,
David Ahern, David S. Miller, Eric Dumazet, Hideaki YOSHIFUJI,
Jakub Kicinski, Kees Cook, Marco Elver, linux-kernel, linux-arch
On Thu, Jul 22, 2021 at 04:29:01PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Since dynamic registration of the gifconf() helper is only used for
> IPv4, and this can not be in a loadable module, this can be simplified
> noticeably by turning it into a direct function call as a preparation
> for cleaning up the compat handling.
I'd maybe mention that ipv4 must be built-in as that is the bit
required for all the above to make sense.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v6 5/6] net: socket: simplify dev_ifconf handling
2021-07-22 14:28 [PATCH net-next v6 0/6] remove compat_alloc_user_space() Arnd Bergmann
` (3 preceding siblings ...)
2021-07-22 14:29 ` [PATCH net-next v6 4/6] net: socket: remove register_gifconf Arnd Bergmann
@ 2021-07-22 14:29 ` Arnd Bergmann
2021-07-22 16:26 ` Christoph Hellwig
2021-07-22 14:29 ` [PATCH net-next v6 6/6] net: socket: rework compat_ifreq_ioctl() Arnd Bergmann
2021-07-23 14:40 ` [PATCH net-next v6 0/6] remove compat_alloc_user_space() patchwork-bot+netdevbpf
6 siblings, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2021-07-22 14:29 UTC (permalink / raw)
To: netdev
Cc: Arnd Bergmann, Al Viro, Andrew Lunn, Christoph Hellwig,
David Ahern, David S. Miller, Eric Dumazet, Hideaki YOSHIFUJI,
Jakub Kicinski, Kees Cook, Marco Elver, linux-kernel, linux-arch
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.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
changes in v6:
- split register_gifconf from compat changes
---
include/linux/netdevice.h | 2 +-
net/core/dev_ioctl.c | 55 +++++++++++++++++++-----------------
net/socket.c | 59 ++++++++++-----------------------------
3 files changed, 44 insertions(+), 72 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 6630a9f0b0f0..da2c273c7e0a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4008,7 +4008,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 c22c3dc15ce9..950e2fe5d56a 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -31,48 +31,51 @@ static int dev_ifname(struct net *net, struct ifreq *ifr)
* 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;
+
+ if (copy_from_user(&ifc32, uifc, sizeof(struct compat_ifconf)))
+ return -EFAULT;
- pos = ifc->ifc_buf;
- len = ifc->ifc_len;
+ pos = compat_ptr(ifc32.ifcbuf);
+ len = ifc32.ifc_len;
+ size = sizeof(struct compat_ifreq);
+ } else {
+ struct ifconf ifc;
- /*
- * Loop over the interfaces, and write an info block for each.
- */
+ if (copy_from_user(&ifc, uifc, sizeof(struct ifconf)))
+ return -EFAULT;
- total = 0;
+ 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) {
- int done;
if (!pos)
done = inet_gifconf(dev, NULL, 0, size);
else
done = inet_gifconf(dev, pos + total,
len - total, size);
- if (done < 0)
+ 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/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] 13+ messages in thread* Re: [PATCH net-next v6 5/6] net: socket: simplify dev_ifconf handling
2021-07-22 14:29 ` [PATCH net-next v6 5/6] net: socket: simplify dev_ifconf handling Arnd Bergmann
@ 2021-07-22 16:26 ` Christoph Hellwig
0 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2021-07-22 16:26 UTC (permalink / raw)
To: Arnd Bergmann
Cc: netdev, Arnd Bergmann, Al Viro, Andrew Lunn, Christoph Hellwig,
David Ahern, David S. Miller, Eric Dumazet, Hideaki YOSHIFUJI,
Jakub Kicinski, Kees Cook, Marco Elver, linux-kernel, linux-arch
On Thu, Jul 22, 2021 at 04:29:02PM +0200, Arnd Bergmann wrote:
> 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.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v6 6/6] net: socket: rework compat_ifreq_ioctl()
2021-07-22 14:28 [PATCH net-next v6 0/6] remove compat_alloc_user_space() Arnd Bergmann
` (4 preceding siblings ...)
2021-07-22 14:29 ` [PATCH net-next v6 5/6] net: socket: simplify dev_ifconf handling Arnd Bergmann
@ 2021-07-22 14:29 ` Arnd Bergmann
2021-07-22 16:26 ` Christoph Hellwig
2021-07-23 14:40 ` [PATCH net-next v6 0/6] remove compat_alloc_user_space() patchwork-bot+netdevbpf
6 siblings, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2021-07-22 14:29 UTC (permalink / raw)
To: netdev
Cc: Arnd Bergmann, Al Viro, Andrew Lunn, Christoph Hellwig,
David Ahern, David S. Miller, Eric Dumazet, Hideaki YOSHIFUJI,
Jakub Kicinski, Kees Cook, Marco Elver, linux-kernel, linux-arch
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/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 ++++++++++++++++++++++++--------------
6 files changed, 76 insertions(+), 47 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index da2c273c7e0a..c871dc223dfa 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4006,6 +4006,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] 13+ messages in thread* Re: [PATCH net-next v6 6/6] net: socket: rework compat_ifreq_ioctl()
2021-07-22 14:29 ` [PATCH net-next v6 6/6] net: socket: rework compat_ifreq_ioctl() Arnd Bergmann
@ 2021-07-22 16:26 ` Christoph Hellwig
0 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2021-07-22 16:26 UTC (permalink / raw)
To: Arnd Bergmann
Cc: netdev, Arnd Bergmann, Al Viro, Andrew Lunn, Christoph Hellwig,
David Ahern, David S. Miller, Eric Dumazet, Hideaki YOSHIFUJI,
Jakub Kicinski, Kees Cook, Marco Elver, linux-kernel, linux-arch
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v6 0/6] remove compat_alloc_user_space()
2021-07-22 14:28 [PATCH net-next v6 0/6] remove compat_alloc_user_space() Arnd Bergmann
` (5 preceding siblings ...)
2021-07-22 14:29 ` [PATCH net-next v6 6/6] net: socket: rework compat_ifreq_ioctl() Arnd Bergmann
@ 2021-07-23 14:40 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 13+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-07-23 14:40 UTC (permalink / raw)
To: Arnd Bergmann
Cc: netdev, arnd, viro, andrew, hch, dsahern, davem, edumazet,
yoshfuji, kuba, keescook, elver, linux-kernel, linux-arch
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Thu, 22 Jul 2021 16:28:57 +0200 you wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> This is the fifth version of my series, now spanning four patches
> instead of two, with a new approach for handling struct ifreq
> compatibility after I realized that my earlier approach introduces
> additional problems.
>
> [...]
Here is the summary with links:
- [net-next,v6,1/6] compat: make linux/compat.h available everywhere
https://git.kernel.org/netdev/net-next/c/1a33b18b3bd9
- [net-next,v6,2/6] ethtool: improve compat ioctl handling
https://git.kernel.org/netdev/net-next/c/dd98d2895de6
- [net-next,v6,3/6] net: socket: rework SIOC?IFMAP ioctls
https://git.kernel.org/netdev/net-next/c/709566d79209
- [net-next,v6,4/6] net: socket: remove register_gifconf
https://git.kernel.org/netdev/net-next/c/b0e99d03778b
- [net-next,v6,5/6] net: socket: simplify dev_ifconf handling
https://git.kernel.org/netdev/net-next/c/876f0bf9d0d5
- [net-next,v6,6/6] net: socket: rework compat_ifreq_ioctl()
https://git.kernel.org/netdev/net-next/c/29c4964822aa
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 13+ messages in thread