Re: [PATCH net-next 1/2] net: add sockopt_expand_out()

[Stanislav Fomichev <sdf.kernel@gmail.com>]

On 09/10, Breno Leitao wrote:
> Add sockopt_expand_out() to grow opt->iter_out mid-air.
> 
> It is a no-op unless the proper size outruns optlen (i.e, some
> not-well-behaved userspace program calling it).
> 
> In this case, only a user buffer can be longer than optlen says, so
> a kernel-backed optval keeps the bounded iterator and the callback gets
> -EINVAL if it asks to grow.
> 
> This whole quirk is added to:
> 
>   1) Avoid breaking userspace
>   2) Making the quirk explict
>      * Instead of protocol doing implict assumping like this.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  include/linux/net.h | 25 +++++++++++++++++++++++++
>  net/socket.c        |  4 ++--
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/net.h b/include/linux/net.h
> index 470100ae710773..de0ed362b37794 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -70,6 +70,31 @@ static inline int sockopt_init_user(sockopt_t *opt, char __user *optval,
>  	return 0;
>  }
>  
> +/*
> + * Grow optval to @size, for the options whose reply is sized by a count the
> + * caller left in optval rather than by optlen. Those write past optlen today
> + * and userspace relies on it.
> + *
> + * Call it before writing through opt->iter_out: it re-anchors the iterator at
> + * the head of optval. Only a user buffer can be longer than the optlen the
> + * caller declared, so a kernel-backed optval is refused with -EINVAL.
> + */
> +static inline int sockopt_expand_out(sockopt_t *opt, size_t size)
> +{

[..]

> +	if (size <= iov_iter_count(&opt->iter_out))
> +		return 0;
> +
> +	if (WARN_ON_ONCE(!iter_is_ubuf(&opt->iter_out)))
> +		return -EINVAL;

nit: if you end up re-spinning for some reason, maybe swap these two?
I always get confused by the count vs len of iov (iov_iter_count vs
iter_iov_len). Because I think count for ubuf is len because of the
aliasing? (and then, if !iter_is_ubuf check is first, at least iov_iter_count
will 100% be ubuf specific)

Acked-by: Stanislav Fomichev <sdf@fomichev.me>