• John Fastabend's avatar
    bpf: Sockmap/tls, tls_sw can create a plaintext buf > encrypt buf · d468e477
    John Fastabend authored
    It is possible to build a plaintext buffer using push helper that is larger
    than the allocated encrypt buffer. When this record is pushed to crypto
    layers this can result in a NULL pointer dereference because the crypto
    API expects the encrypt buffer is large enough to fit the plaintext
    buffer. Kernel splat below.
    
    To resolve catch the cases this can happen and split the buffer into two
    records to send individually. Unfortunately, there is still one case to
    handle where the split creates a zero sized buffer. In this case we merge
    the buffers and unmark the split. This happens when apply is zero and user
    pushed data beyond encrypt buffer. This fixes the original case as well
    because the split allocated an encrypt buffer larger than the plaintext
    buffer and the merge simply moves the pointers around so we now have
    a reference to the new (larger) encrypt buffer.
    
    Perhaps its not ideal but it seems the best solution for a fixes branch
    and avoids handling these two cases, (a) apply that needs split and (b)
    non apply case. The are edge cases anyways so optimizing them seems not
    necessary unless someone wants later in next branches.
    
    [  306.719107] BUG: kernel NULL pointer dereference, address: 0000000000000008
    [...]
    [  306.747260] RIP: 0010:scatterwalk_copychunks+0x12f/0x1b0
    [...]
    [  306.770350] Call Trace:
    [  306.770956]  scatterwalk_map_and_copy+0x6c/0x80
    [  306.772026]  gcm_enc_copy_hash+0x4b/0x50
    [  306.772925]  gcm_hash_crypt_remain_continue+0xef/0x110
    [  306.774138]  gcm_hash_crypt_continue+0xa1/0xb0
    [  306.775103]  ? gcm_hash_crypt_continue+0xa1/0xb0
    [  306.776103]  gcm_hash_assoc_remain_continue+0x94/0xa0
    [  306.777170]  gcm_hash_assoc_continue+0x9d/0xb0
    [  306.778239]  gcm_hash_init_continue+0x8f/0xa0
    [  306.779121]  gcm_hash+0x73/0x80
    [  306.779762]  gcm_encrypt_continue+0x6d/0x80
    [  306.780582]  crypto_gcm_encrypt+0xcb/0xe0
    [  306.781474]  crypto_aead_encrypt+0x1f/0x30
    [  306.782353]  tls_push_record+0x3b9/0xb20 [tls]
    [  306.783314]  ? sk_psock_msg_verdict+0x199/0x300
    [  306.784287]  bpf_exec_tx_verdict+0x3f2/0x680 [tls]
    [  306.785357]  tls_sw_sendmsg+0x4a3/0x6a0 [tls]
    
    test_sockmap test signature to trigger bug,
    
    [TEST]: (1, 1, 1, sendmsg, pass,redir,start 1,end 2,pop (1,2),ktls,):
    
    Fixes: d3b18ad3 ("tls: add bpf support to sk_msg handling")
    Signed-off-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Acked-by: default avatarJonathan Lemon <jonathan.lemon@gmail.com>
    Cc: stable@vger.kernel.org
    Link: https://lore.kernel.org/bpf/20200111061206.8028-7-john.fastabend@gmail.com
    d468e477
tls_sw.c 61.2 KB