netdev - Re: [bpf PATCH 8/9] bpf: sockmap/tls, tls_push_record can not handle zero length skmsg

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAPhsuW774z68g_Y-C1XU70H-x6S2mr+Hd0-qY02E9aZBJjepkA@mail.gmail.com>
Date:   Thu, 9 Jan 2020 12:08:58 -0800
From:   Song Liu <song@...nel.org>
To:     John Fastabend <john.fastabend@...il.com>
Cc:     bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [bpf PATCH 8/9] bpf: sockmap/tls, tls_push_record can not handle
 zero length skmsg

On Wed, Jan 8, 2020 at 1:17 PM John Fastabend <john.fastabend@...il.com> wrote:
>
> When passed a zero length skmsg tls_push_record() causes a NULL ptr
> deref. To resolve for fixes do a simple length check at start of
> routine.

Could you please include the stack dump for the NULL deref?

Thanks,
Song

>
> To create this case a user can create a BPF program to pop all the
> data off the message then return SK_PASS. Its not a very practical
> or useful thing to do so we mark it unlikely.
>
> Fixes: d3b18ad31f93d ("tls: add bpf support to sk_msg handling")
> Signed-off-by: John Fastabend <john.fastabend@...il.com>
> ---
>  net/tls/tls_sw.c |    3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 21c7725d17ca..0326e916ab01 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -680,6 +680,9 @@ static int tls_push_record(struct sock *sk, int flags,
>         msg_pl = &rec->msg_plaintext;
>         msg_en = &rec->msg_encrypted;
>
> +       if (unlikely(!msg_pl->sg.size))
> +               return 0;
> +
>         split_point = msg_pl->apply_bytes;
>         split = split_point && split_point < msg_pl->sg.size;
>         if (unlikely((!split &&
>