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

[Fernando Fernandez Mancera <fmancera@suse.de>]

On 9/10/26 11:19 PM, Stanislav Fomichev wrote:
> 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@suse.de>
>> ---
>>   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.