Re: [PATCH net] udp: fix the len check in udp_lib_getsockopt

[Xin Long <lucien.xin@gmail.com>]

On Thu, May 27, 2021 at 3:12 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> Currently, when calling UDP's getsockopt, it only makes sure 'len'
> is not less than 0, then copys 'len' bytes back to usespace while
> all data is 'int' type there.
>
> If userspace sets 'len' with a value N (N < sizeof(int)), it will
> only copy N bytes of the data to userspace with no error returned,
> which doesn't seem right. Like in Chen Yi's case where N is 0, it
> called getsockopt and got an incorrect value but with no error
> returned.
>
> The patch is to fix the len check and make sure it's not less than
> sizeof(int). Otherwise, '-EINVAL' is returned, as it does in other
> protocols like SCTP/TIPC.
>
> Reported-by: Chen Yi <yiche@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/ipv4/udp.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 15f5504adf5b..90de2ac70ea9 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -2762,11 +2762,11 @@ int udp_lib_getsockopt(struct sock *sk, int level, int optname,
>         if (get_user(len, optlen))
>                 return -EFAULT;
>
> -       len = min_t(unsigned int, len, sizeof(int));
> -
> -       if (len < 0)
> +       if (len < sizeof(int))
>                 return -EINVAL;
>
> +       len = sizeof(int);
> +
>         switch (optname) {
>         case UDP_CORK:
>                 val = up->corkflag;
> --
> 2.27.0
>

Note I'm not sure if this fix may break any APP, but the current
behavior definitely is not correct and doesn't match the man doc
of getsockopt, so please review.