Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH net-next v6 0/6] remove compat_alloc_user_space()
@ 2021-07-22 14:28 Arnd Bergmann
  2021-07-22 14:28 ` [PATCH net-next v6 1/6] compat: make linux/compat.h available everywhere Arnd Bergmann
                   ` (6 more replies)
  0 siblings, 7 replies; 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>

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.

The idea here is to always push down the compat conversion
deeper into the call stack: rather than pretending to be
native mode with a modified copy of the original data on
the user space stack, have the code that actually works on
the data understand the difference between native and compat
versions.

I have spent a long time looking at all drivers that implement
an ndo_do_ioctl callback to verify that my assumptions are
correct. This has led to a series of ~30 additional patches
that I am not including here but will post separately, fixing
a number of bugs in SIOCDEVPRIVATE ioctls, removing dead
code, and splitting ndo_do_ioctl into multiple new ndo callbacks
for private and ethernet specific commands.

      Arnd

Link: https://lore.kernel.org/netdev/20201124151828.169152-1-arnd@kernel.org/

Changes in v6:
 - Split out and expand linux/compat.h rework
 - Split ifconf change into two patches
 - Rebase on latest net-next/master

Changes in v5:
 - Rebase to v5.14-rc2
 - Fix a few build issues

Changes in v4:
 - build fix without CONFIG_INET
 - build fix without CONFIG_COMPAT
 - style fixes pointed out by hch

Changes in v3:
 - complete rewrite of the series

Arnd Bergmann (6):
  compat: make linux/compat.h available everywhere
  ethtool: improve compat ioctl handling
  net: socket: rework SIOC?IFMAP ioctls
  net: socket: remove register_gifconf
  net: socket: simplify dev_ifconf handling
  net: socket: rework compat_ifreq_ioctl()

 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 ++--
 include/linux/ethtool.h           |   4 -
 include/linux/inetdevice.h        |   9 +
 include/linux/netdevice.h         |  12 +-
 net/appletalk/ddp.c               |   4 +-
 net/core/dev_ioctl.c              | 153 +++++++++-------
 net/ethtool/ioctl.c               | 136 ++++++++++++--
 net/ieee802154/socket.c           |   4 +-
 net/ipv4/af_inet.c                |   6 +-
 net/ipv4/devinet.c                |   4 +-
 net/qrtr/qrtr.c                   |   4 +-
 net/socket.c                      | 292 +++++++-----------------------
 21 files changed, 352 insertions(+), 431 deletions(-)

-- 
2.29.2

Cc: Al Viro <viro@zeniv.linux.org.uk> 
Cc: Andrew Lunn <andrew@lunn.ch> 
Cc: Christoph Hellwig <hch@lst.de>
Cc: David Ahern <dsahern@kernel.org> 
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org> 
Cc: Jakub Kicinski <kuba@kernel.org> 
Cc: Kees Cook <keescook@chromium.org> 
Cc: Marco Elver <elver@google.com> 
Cc: linux-kernel@vger.kernel.org 
Cc: linux-arch@vger.kernel.org 
Cc: netdev@vger.kernel.org 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [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	[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	[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	[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	[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	[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	[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

* 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

* 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

* 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

* 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

end of thread, other threads:[~2021-07-23 23:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 16:22   ` Christoph Hellwig
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
2021-07-22 14:29 ` [PATCH net-next v6 3/6] net: socket: rework SIOC?IFMAP ioctls Arnd Bergmann
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
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
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
2021-07-23 14:40 ` [PATCH net-next v6 0/6] remove compat_alloc_user_space() patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).