Re: [PATCH net-next v4 2/7] net: cap sockopt_expand_out() at MAX_RW_COUNT

[Jiayuan Chen]

On 11/3/26 7:01 PM, Breno Leitao wrote:
> sockopt_expand_out() grows the output iterator up to INT_MAX, but the
> MCAST_MSFILTER getters converted by the next two patches rewind it with
> iov_iter_revert() once they know the real reply size. iov_iter_revert()
> refuses to unroll past MAX_RW_COUNT:
>
> 	if (WARN_ON(unroll > MAX_RW_COUNT))
> 		return;
>
> MAX_RW_COUNT is INT_MAX rounded down to a page boundary, so a caller
> that expands into the (MAX_RW_COUNT, INT_MAX] range and then reverts
> hits that WARN_ON, and the iterator is left where it was instead of
> rewound. So does a caller whose optlen already covers such a size.
>
> Cap the size at MAX_RW_COUNT instead, before the early return for an
> optlen that already covers it, so a size that could never be reverted
> later is rejected up front.
>
> Sashiko found this while reviewing the conversions.
>
> Acked-by: Ido Schimmel <idosch@xxxxxxxxxx>
> Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx>


Reviewed-by: Jiayuan Chen <jiayuan.chen@xxxxxxxxx>


> ---
>  include/linux/net.h | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/net.h b/include/linux/net.h
> index e2a866fbcfa5ea..de56f89d475ee2 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -80,16 +80,21 @@ static inline int sockopt_init_user(sockopt_t *opt, char __user *optval,
>   *
>   * 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.
> + * caller declared, so a kernel-backed optval is refused with -EINVAL. So is a
> + * @size above MAX_RW_COUNT, which iov_iter_revert() could not unroll.
>   */
>  static inline int sockopt_expand_out(sockopt_t *opt, size_t size)
>  {
> +	/* iov_iter_revert() refuses to unroll past MAX_RW_COUNT, so a size
> +	 * beyond that could never be reverted back to the fixed part later,
> +	 * even when optlen already covers it.
> +	 */
> +	if (size > MAX_RW_COUNT)
> +		return -EINVAL;
> +
>  	if (size <= (size_t)opt->optlen)
>  		return 0;
>  
> -	if (size > INT_MAX)
> -		return -EINVAL;
> -
>  	/* Re-anchoring reads iter_out.ubuf, so the iterator has to be a user
>  	 * buffer that nothing has written through yet.
>  	 */
>