Re: [PATCH net-next 1/6] atm: convert to getsockopt_iter

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] getsockopt(..., SO_SETCLP, ...) unconditionally fails with -EFAULT on 64-bit systems due to a size mismatch.
--

commit be3fd8a3e8c34e64e9da9ed374ea4925d11bbbe7
Author: Breno Leitao <leitao@debian.org>

atm: convert to getsockopt_iter

This commit converts the ATM SVC and PVC sockets, along with the shared
vcc_getsockopt() helper, to use the new getsockopt_iter callback with
sockopt_t.

> diff --git a/net/atm/common.c b/net/atm/common.c
> --- a/net/atm/common.c
> +++ b/net/atm/common.c
[ ... ]
> @@ -797,13 +798,13 @@ int vcc_setsockopt(struct socket *sock, int level, int optname,
>  }
>  
>  int vcc_getsockopt(struct socket *sock, int level, int optname,
> -		   char __user *optval, int __user *optlen)
> +		   sockopt_t *opt)
>  {
>  	struct atm_vcc *vcc;
> +	unsigned long val;
>  	int len;
>  
> -	if (get_user(len, optlen))
> -		return -EFAULT;
> +	len = opt->optlen;
>  	if (__SO_LEVEL_MATCH(optname, level) && len != __SO_SIZE(optname))
>  		return -EINVAL;
[ ... ]
>  	case SO_SETCLP:
> -		return put_user(vcc->atm_options & ATM_ATMOPT_CLP ? 1 : 0,
> -				(unsigned long __user *)optval) ? -EFAULT : 0;
> +		val = vcc->atm_options & ATM_ATMOPT_CLP ? 1 : 0;
> +		return copy_to_iter(&val, sizeof(val), &opt->iter_out) !=
> +		       sizeof(val) ? -EFAULT : 0;

On 64-bit systems, val is an unsigned long (8 bytes). However, __SO_SIZE
for SO_SETCLP is 4 bytes, which is enforced by the length check above.

Since the opt->iter_out iterator is restricted to 4 bytes, copy_to_iter()
will truncate the copy to 4 bytes and return 4. Does this mean the
subsequent check against sizeof(val) (8 bytes) will always fail and return
-EFAULT unconditionally?

Additionally, on 64-bit big-endian systems, does copying the first 4 bytes
of an 8-byte unsigned long always copy zeroes, ignoring the actual flag
value?

Would declaring val as an int instead of an unsigned long resolve both the
copy size mismatch and the potential endianness issue here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-getsock_four-v1-0-fe7f0e756fac@debian.org?part=1