netdev - Re: [PATCH net] udp: fix the len check in udp_lib

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADvbK_e0PkKBYAUyg6iYyUwUp+owpv1r9_cnS7pbkLSjwX+VWg@mail.gmail.com>
Date:   Thu, 27 May 2021 15:13:32 -0400
From:   Xin Long <lucien.xin@...il.com>
To:     network dev <netdev@...r.kernel.org>, davem <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     Eric Dumazet <edumazet@...gle.com>,
        David Ahern <dsahern@...il.com>,
        Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net] udp: fix the len check in udp_lib_getsockopt

On Thu, May 27, 2021 at 3:12 PM Xin Long <lucien.xin@...il.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@...hat.com>
> Signed-off-by: Xin Long <lucien.xin@...il.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.