LinuxLists.cc - [PATCH] net: ipmr: Call ipmr_ioctl() directly from ipmr_sk

2023-07-31 15:21:33

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

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 <[email protected]>
Suggested-by: Willem de Bruijn <[email protected]>
---
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

2023-07-31 19:22:09

by Willem de Bruijn

[permalink] [raw]

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

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 <[email protected]>
> Suggested-by: Willem de Bruijn <[email protected]>
> ---
> 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.

2023-08-09 03:56:16

by kernel test robot

[permalink] [raw]

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

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/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230809/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

2023-08-10 00:40:11

by kernel test robot

[permalink] [raw]