[PATCH] net: ipmr: Call ipmr_ioctl() directly from ipmr_sk_ioctl()

[PATCH] net: ipmr: Call ipmr_ioctl() directly from ipmr_sk_ioctl()

[Gustavo Luiz Duarte]

Call ipmr_ioctl()/ip6mr_ioctl() directly from ipmr_sk_ioctl()/ip6mr_sk_ioctl()
and avoid sk_prot->ioctl function pointer indirection.

Also, delete the sock_ioctl_inout() helper as it is no longer needed.

Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
---
 include/linux/mroute6.h | 33 ++++++++++++++++++++++++++-------
 include/net/sock.h      |  2 --
 net/core/sock.c         | 20 --------------------
 net/ipv4/ipmr.c         | 33 ++++++++++++++++++++++++++-------
 4 files changed, 52 insertions(+), 36 deletions(-)

diff --git a/include/linux/mroute6.h b/include/linux/mroute6.h
index 63ef5191cc57..1ed34264bb72 100644
--- a/include/linux/mroute6.h
+++ b/include/linux/mroute6.h
@@ -103,19 +103,38 @@ extern int ip6mr_sk_done(struct sock *sk);
 static inline int ip6mr_sk_ioctl(struct sock *sk, unsigned int cmd,
 				 void __user *arg)
 {
+	int ret;
+
 	switch (cmd) {
-	/* These userspace buffers will be consumed by ip6mr_ioctl() */
 	case SIOCGETMIFCNT_IN6: {
-		struct sioc_mif_req6 buffer;
+		struct sioc_mif_req6 karg;
+
+		if (copy_from_user(&karg, arg, sizeof(karg)))
+			return -EFAULT;
+
+		ret = ip6mr_ioctl(sk, cmd, &karg);
+		if (ret)
+			return ret;
+
+		if (copy_to_user(arg, &karg, sizeof(karg)))
+			return -EFAULT;
 
-		return sock_ioctl_inout(sk, cmd, arg, &buffer,
-					sizeof(buffer));
+		return 0;
 		}
 	case SIOCGETSGCNT_IN6: {
-		struct sioc_sg_req6 buffer;
+		struct sioc_sg_req6 karg;
+
+		if (copy_from_user(&karg, arg, sizeof(karg)))
+			return -EFAULT;
+
+		ret = ip6mr_ioctl(sk, cmd, &karg);
+		if (ret)
+			return ret;
+
+		if (copy_to_user(arg, &karg, sizeof(karg)))
+			return -EFAULT;
 
-		return sock_ioctl_inout(sk, cmd, arg, &buffer,
-					sizeof(buffer));
+		return 0;
 		}
 	}
 
diff --git a/include/net/sock.h b/include/net/sock.h
index 2eb916d1ff64..0df582e3776a 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2969,8 +2969,6 @@ int sock_get_timeout(long timeo, void *optval, bool old_timeval);
 int sock_copy_user_timeval(struct __kernel_sock_timeval *tv,
 			   sockptr_t optval, int optlen, bool old_timeval);
 
-int sock_ioctl_inout(struct sock *sk, unsigned int cmd,
-		     void __user *arg, void *karg, size_t size);
 int sk_ioctl(struct sock *sk, unsigned int cmd, void __user *arg);
 static inline bool sk_is_readable(struct sock *sk)
 {
diff --git a/net/core/sock.c b/net/core/sock.c
index 9370fd50aa2c..e0002805ecbd 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -4129,26 +4129,6 @@ int sock_bind_add(struct sock *sk, struct sockaddr *addr, int addr_len)
 }
 EXPORT_SYMBOL(sock_bind_add);
 
-/* Copy 'size' bytes from userspace and return `size` back to userspace */
-int sock_ioctl_inout(struct sock *sk, unsigned int cmd,
-		     void __user *arg, void *karg, size_t size)
-{
-	int ret;
-
-	if (copy_from_user(karg, arg, size))
-		return -EFAULT;
-
-	ret = READ_ONCE(sk->sk_prot)->ioctl(sk, cmd, karg);
-	if (ret)
-		return ret;
-
-	if (copy_to_user(arg, karg, size))
-		return -EFAULT;
-
-	return 0;
-}
-EXPORT_SYMBOL(sock_ioctl_inout);
-
 /* This is the most common ioctl prep function, where the result (4 bytes) is
  * copied back to userspace if the ioctl() returns successfully. No input is
  * copied from userspace as input argument.
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 3f0c6d602fb7..0144da36e6c0 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1550,19 +1550,38 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, sockptr_t optval,
 /* Execute if this ioctl is a special mroute ioctl */
 int ipmr_sk_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
 {
+	int ret;
+
 	switch (cmd) {
-	/* These userspace buffers will be consumed by ipmr_ioctl() */
 	case SIOCGETVIFCNT: {
-		struct sioc_vif_req buffer;
+		struct sioc_vif_req karg;
+
+		if (copy_from_user(&karg, arg, sizeof(karg)))
+			return -EFAULT;
+
+		ret = ipmr_ioctl(sk, cmd, &karg);
+		if (ret)
+			return ret;
+
+		if (copy_to_user(arg, &karg, sizeof(karg)))
+			return -EFAULT;
 
-		return sock_ioctl_inout(sk, cmd, arg, &buffer,
-				      sizeof(buffer));
+		return 0;
 		}
 	case SIOCGETSGCNT: {
-		struct sioc_sg_req buffer;
+		struct sioc_sg_req karg;
+
+		if (copy_from_user(&karg, arg, sizeof(karg)))
+			return -EFAULT;
+
+		ret = ipmr_ioctl(sk, cmd, &karg);
+		if (ret)
+			return ret;
 
-		return sock_ioctl_inout(sk, cmd, arg, &buffer,
-				      sizeof(buffer));
+		if (copy_to_user(arg, &karg, sizeof(karg)))
+			return -EFAULT;
+
+		return 0;
 		}
 	}
 	/* return code > 0 means that the ioctl was not executed */
-- 
2.40.1

RE: [PATCH] net: ipmr: Call ipmr_ioctl() directly from ipmr_sk_ioctl()

[Willem de Bruijn]

Gustavo Luiz Duarte wrote:
> Call ipmr_ioctl()/ip6mr_ioctl() directly from ipmr_sk_ioctl()/ip6mr_sk_ioctl()
> and avoid sk_prot->ioctl function pointer indirection.
> 
> Also, delete the sock_ioctl_inout() helper as it is no longer needed.
> 
> Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
> Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> ---
>  include/linux/mroute6.h | 33 ++++++++++++++++++++++++++-------
>  include/net/sock.h      |  2 --
>  net/core/sock.c         | 20 --------------------
>  net/ipv4/ipmr.c         | 33 ++++++++++++++++++++++++++-------
>  4 files changed, 52 insertions(+), 36 deletions(-)

The helper function does save some LoC, it seems.

My comment came during review of the series. Now that that is in,
fine to leave as is, imho.

Re: [PATCH] net: ipmr: Call ipmr_ioctl() directly from ipmr_sk_ioctl()

[kernel test robot]

Hi Gustavo,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]
[also build test ERROR on net-next/main linus/master v6.5-rc5 next-20230808]
[cannot apply to horms-ipvs/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Gustavo-Luiz-Duarte/net-ipmr-Call-ipmr_ioctl-directly-from-ipmr_sk_ioctl/20230731-225918
base:   net/main
patch link:    https://lore.kernel.org/r/20230731145713.178509-1-gustavold%40gmail.com
patch subject: [PATCH] net: ipmr: Call ipmr_ioctl() directly from ipmr_sk_ioctl()
config: x86_64-randconfig-x054-20230808 (https://download.01.org/0day-ci/archive/20230809/202308091122.z4eVvCI5-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230809/202308091122.z4eVvCI5-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308091122.z4eVvCI5-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: net/core/sock.o: in function `ip6mr_sk_ioctl':
>> include/linux/mroute6.h:115: undefined reference to `ip6mr_ioctl'
>> ld: include/linux/mroute6.h:130: undefined reference to `ip6mr_ioctl'


vim +115 include/linux/mroute6.h

    95	
    96	struct rtmsg;
    97	extern int ip6mr_get_route(struct net *net, struct sk_buff *skb,
    98				   struct rtmsg *rtm, u32 portid);
    99	
   100	#ifdef CONFIG_IPV6_MROUTE
   101	bool mroute6_is_socket(struct net *net, struct sk_buff *skb);
   102	extern int ip6mr_sk_done(struct sock *sk);
   103	static inline int ip6mr_sk_ioctl(struct sock *sk, unsigned int cmd,
   104					 void __user *arg)
   105	{
   106		int ret;
   107	
   108		switch (cmd) {
   109		case SIOCGETMIFCNT_IN6: {
   110			struct sioc_mif_req6 karg;
   111	
   112			if (copy_from_user(&karg, arg, sizeof(karg)))
   113				return -EFAULT;
   114	
 > 115			ret = ip6mr_ioctl(sk, cmd, &karg);
   116			if (ret)
   117				return ret;
   118	
   119			if (copy_to_user(arg, &karg, sizeof(karg)))
   120				return -EFAULT;
   121	
   122			return 0;
   123			}
   124		case SIOCGETSGCNT_IN6: {
   125			struct sioc_sg_req6 karg;
   126	
   127			if (copy_from_user(&karg, arg, sizeof(karg)))
   128				return -EFAULT;
   129	
 > 130			ret = ip6mr_ioctl(sk, cmd, &karg);
   131			if (ret)
   132				return ret;
   133	
   134			if (copy_to_user(arg, &karg, sizeof(karg)))
   135				return -EFAULT;
   136	
   137			return 0;
   138			}
   139		}
   140	
   141		return 1;
   142	}
   143	#else
   144	static inline bool mroute6_is_socket(struct net *net, struct sk_buff *skb)
   145	{
   146		return false;
   147	}
   148	static inline int ip6mr_sk_done(struct sock *sk)
   149	{
   150		return 0;
   151	}
   152	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] net: ipmr: Call ipmr_ioctl() directly from ipmr_sk_ioctl()

[kernel test robot]

Hi Gustavo,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]
[also build test ERROR on net-next/main linus/master v6.5-rc5 next-20230809]
[cannot apply to horms-ipvs/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Gustavo-Luiz-Duarte/net-ipmr-Call-ipmr_ioctl-directly-from-ipmr_sk_ioctl/20230731-225918
base:   net/main
patch link:    https://lore.kernel.org/r/20230731145713.178509-1-gustavold%40gmail.com
patch subject: [PATCH] net: ipmr: Call ipmr_ioctl() directly from ipmr_sk_ioctl()
config: i386-randconfig-i011-20230809 (https://download.01.org/0day-ci/archive/20230810/202308100626.gj6rvqoB-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce: (https://download.01.org/0day-ci/archive/20230810/202308100626.gj6rvqoB-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308100626.gj6rvqoB-lkp@intel.com/

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: ip6mr_ioctl
   >>> referenced by mroute6.h:115 (include/linux/mroute6.h:115)
   >>>               net/core/sock.o:(sk_ioctl) in archive vmlinux.a
   >>> referenced by mroute6.h:130 (include/linux/mroute6.h:130)
   >>>               net/core/sock.o:(sk_ioctl) in archive vmlinux.a

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki