Re: [PATCH 03/13 net-next] net: inet: relocate ip_generic_getfrag and guard IPv4 socket logic

On 9/10/26 11:19 PM, Stanislav Fomichev wrote:

On 09/10, Fernando Fernandez Mancera wrote:

quoted

To enable compiling the INET subsystem without IPv4, shared generic
utilities must be relocated and IPv4 socket logic must be guarded for
CONFIG_IPV4.

This patch moves the generic ip_generec_getfrag() from ip_output.c to
af_inet.c. It also introduces CONFIG_IPV4 guards around af_inet.c to
reject IPv4-specific ioctls, protocol registrations and bind requests.
The same guard is added to reject IPv4-mapped IPv6.

Signed-off-by: Fernando Fernandez Mancera <redacted>
---
  net/ipv4/af_inet.c   | 96 +++++++++++++++++++++++++++++++++++++-------
  net/ipv4/ip_output.c | 18 ---------
  net/ipv6/af_inet6.c  |  5 +++
  net/ipv6/datagram.c  | 12 ++++++
  4 files changed, 99 insertions(+), 32 deletions(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index d9421ac38d78..b0c48ba544bf 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c

@@ -129,6 +129,28 @@
  int disable_ipv6_mod;
  EXPORT_SYMBOL(disable_ipv6_mod);
  
+/* Keep the function here for now as it is generic, it should be moved
+ * to a common L3 place
+ */
+int
+ip_generic_getfrag(void *from, char *to, int offset, int len, int odd, struct sk_buff *skb)
+{
+	struct msghdr *msg = from;
+
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		if (!copy_from_iter_full(to, len, &msg->msg_iter))
+			return -EFAULT;
+	} else {
+		__wsum csum = 0;
+
+		if (!csum_and_copy_from_iter_full(to, len, &csum, &msg->msg_iter))
+			return -EFAULT;
+		skb->csum = csum_block_add(skb->csum, csum, odd);
+	}
+	return 0;
+}
+EXPORT_SYMBOL(ip_generic_getfrag);
+
  /* The inetsw table contains everything that inet_create needs to
   * build a new socket.
   */

@@ -425,8 +447,10 @@ int inet_release(struct socket *sock)
  		if (!sk->sk_kern_sock)
  			BPF_CGROUP_RUN_PROG_INET_SOCK_RELEASE(sk);
  
+#if IS_ENABLED(CONFIG_IPV4)
  		/* Applications forget to leave groups before exiting */
  		ip_mc_drop_socket(sk);
+#endif
  
  		/* If linger is set, we don't return until the close
  		 * is complete.  Otherwise we return immediately. The

@@ -478,6 +502,7 @@ EXPORT_SYMBOL(inet_bind);
  int __inet_bind(struct sock *sk, struct sockaddr_unsized *uaddr, int addr_len,
  		u32 flags)
  {
+#if IS_ENABLED(CONFIG_IPV4)
  	struct sockaddr_in *addr = (struct sockaddr_in *)uaddr;
  	struct inet_sock *inet = inet_sk(sk);
  	struct net *net = sock_net(sk);

@@ -570,6 +595,9 @@ int __inet_bind(struct sock *sk, struct sockaddr_unsized *uaddr, int addr_len,
  		release_sock(sk);
  out:
  	return err;
+#else
+	return -EAFNOSUPPORT;
+#endif
  }
  
  int inet_dgram_connect(struct socket *sock, struct sockaddr_unsized *uaddr,

@@ -962,18 +990,24 @@ EXPORT_SYMBOL(inet_shutdown);
  int inet_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
  {
  	struct sock *sk = sock->sk;
-	int err = 0;
-	struct net *net = sock_net(sk);
+#if IS_ENABLED(CONFIG_IPV4)
  	void __user *p = (void __user *)arg;
-	struct ifreq ifr;
+	struct net *net = sock_net(sk);
  	struct rtentry rt;
+	struct ifreq ifr;
+#endif
+	int err = 0;
  
  	switch (cmd) {
  	case SIOCADDRT:
  	case SIOCDELRT:
+#if IS_ENABLED(CONFIG_IPV4)
  		if (copy_from_user(&rt, p, sizeof(struct rtentry)))
  			return -EFAULT;
  		err = ip_rt_ioctl(net, cmd, &rt);
+#else
+		err = -EOPNOTSUPP;
+#endif
  		break;
  	case SIOCRTMSG:
  		err = -EINVAL;

@@ -981,18 +1015,26 @@ int inet_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
  	case SIOCDARP:
  	case SIOCGARP:
  	case SIOCSARP:
+#if IS_ENABLED(CONFIG_IPV4)
  		err = arp_ioctl(net, cmd, (void __user *)arg);
+#else
+		err = -EOPNOTSUPP;
+#endif
  		break;
  	case SIOCGIFADDR:
  	case SIOCGIFBRDADDR:
  	case SIOCGIFNETMASK:
  	case SIOCGIFDSTADDR:
  	case SIOCGIFPFLAGS:
+#if IS_ENABLED(CONFIG_IPV4)
  		if (get_user_ifreq(&ifr, NULL, p))
  			return -EFAULT;
  		err = devinet_ioctl(net, cmd, &ifr);
  		if (!err && put_user_ifreq(&ifr, p))
  			err = -EFAULT;
+#else
+		err = -EOPNOTSUPP;
+#endif
  		break;
  
  	case SIOCSIFADDR:

@@ -1001,9 +1043,13 @@ int inet_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
  	case SIOCSIFDSTADDR:
  	case SIOCSIFPFLAGS:
  	case SIOCSIFFLAGS:
+#if IS_ENABLED(CONFIG_IPV4)
  		if (get_user_ifreq(&ifr, NULL, p))
  			return -EFAULT;
  		err = devinet_ioctl(net, cmd, &ifr);
+#else
+		err = -EOPNOTSUPP;
+#endif

(passing by comment)

Don't we have a coding style rule to avoid ifdef conditional in C code?
Should we add some new devinet4_ioctl/etc wrappers that we can conditionally
compile out in the headers?

I think yes but when it makes sense. Of course, that is something a bit 
hard to judge IMHO. I tried to use it when it makes sense, like for 
stubs or for functions that are protocol agnostic but contains a small 
part related to IPv4/IPV6. I was willing to avoid code duplication as 
much as possible.