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

[Stanislav Fomichev]

On 09/10, Fernando Fernandez Mancera wrote:
> 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 <fmancera@xxxxxxx>
> ---
>  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?