lkml.org 
[lkml]   [2017]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.9 16/93] net: dont call strlen() on the user buffer in packet_bind_spkt()
    Date
    4.9-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Alexander Potapenko <glider@google.com>


    [ Upstream commit 540e2894f7905538740aaf122bd8e0548e1c34a4 ]

    KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of
    uninitialized memory in packet_bind_spkt():
    Acked-by: Eric Dumazet <edumazet@google.com>

    ==================================================================
    BUG: KMSAN: use of unitialized memory
    CPU: 0 PID: 1074 Comm: packet Not tainted 4.8.0-rc6+ #1891
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs
    01/01/2011
    0000000000000000 ffff88006b6dfc08 ffffffff82559ae8 ffff88006b6dfb48
    ffffffff818a7c91 ffffffff85b9c870 0000000000000092 ffffffff85b9c550
    0000000000000000 0000000000000092 00000000ec400911 0000000000000002
    Call Trace:
    [< inline >] __dump_stack lib/dump_stack.c:15
    [<ffffffff82559ae8>] dump_stack+0x238/0x290 lib/dump_stack.c:51
    [<ffffffff818a6626>] kmsan_report+0x276/0x2e0 mm/kmsan/kmsan.c:1003
    [<ffffffff818a783b>] __msan_warning+0x5b/0xb0
    mm/kmsan/kmsan_instr.c:424
    [< inline >] strlen lib/string.c:484
    [<ffffffff8259b58d>] strlcpy+0x9d/0x200 lib/string.c:144
    [<ffffffff84b2eca4>] packet_bind_spkt+0x144/0x230
    net/packet/af_packet.c:3132
    [<ffffffff84242e4d>] SYSC_bind+0x40d/0x5f0 net/socket.c:1370
    [<ffffffff84242a22>] SyS_bind+0x82/0xa0 net/socket.c:1356
    [<ffffffff8515991b>] entry_SYSCALL_64_fastpath+0x13/0x8f
    arch/x86/entry/entry_64.o:?
    chained origin: 00000000eba00911
    [<ffffffff810bb787>] save_stack_trace+0x27/0x50
    arch/x86/kernel/stacktrace.c:67
    [< inline >] kmsan_save_stack_with_flags mm/kmsan/kmsan.c:322
    [< inline >] kmsan_save_stack mm/kmsan/kmsan.c:334
    [<ffffffff818a59f8>] kmsan_internal_chain_origin+0x118/0x1e0
    mm/kmsan/kmsan.c:527
    [<ffffffff818a7773>] __msan_set_alloca_origin4+0xc3/0x130
    mm/kmsan/kmsan_instr.c:380
    [<ffffffff84242b69>] SYSC_bind+0x129/0x5f0 net/socket.c:1356
    [<ffffffff84242a22>] SyS_bind+0x82/0xa0 net/socket.c:1356
    [<ffffffff8515991b>] entry_SYSCALL_64_fastpath+0x13/0x8f
    arch/x86/entry/entry_64.o:?
    origin description: ----address@SYSC_bind (origin=00000000eb400911)
    ==================================================================
    (the line numbers are relative to 4.8-rc6, but the bug persists
    upstream)

    , when I run the following program as root:

    =====================================
    #include <string.h>
    #include <sys/socket.h>
    #include <netpacket/packet.h>
    #include <net/ethernet.h>

    int main() {
    struct sockaddr addr;
    memset(&addr, 0xff, sizeof(addr));
    addr.sa_family = AF_PACKET;
    int fd = socket(PF_PACKET, SOCK_PACKET, htons(ETH_P_ALL));
    bind(fd, &addr, sizeof(addr));
    return 0;
    }
    =====================================

    This happens because addr.sa_data copied from the userspace is not
    zero-terminated, and copying it with strlcpy() in packet_bind_spkt()
    results in calling strlen() on the kernel copy of that non-terminated
    buffer.

    Signed-off-by: Alexander Potapenko <glider@google.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    net/packet/af_packet.c | 8 ++++++--
    1 file changed, 6 insertions(+), 2 deletions(-)

    --- a/net/packet/af_packet.c
    +++ b/net/packet/af_packet.c
    @@ -3140,7 +3140,7 @@ static int packet_bind_spkt(struct socke
    int addr_len)
    {
    struct sock *sk = sock->sk;
    - char name[15];
    + char name[sizeof(uaddr->sa_data) + 1];

    /*
    * Check legality
    @@ -3148,7 +3148,11 @@ static int packet_bind_spkt(struct socke

    if (addr_len != sizeof(struct sockaddr))
    return -EINVAL;
    - strlcpy(name, uaddr->sa_data, sizeof(name));
    + /* uaddr->sa_data comes from the userspace, it's not guaranteed to be
    + * zero-terminated.
    + */
    + memcpy(name, uaddr->sa_data, sizeof(uaddr->sa_data));
    + name[sizeof(uaddr->sa_data)] = 0;

    return packet_do_bind(sk, name, 0, pkt_sk(sk)->num);
    }

    \
     
     \ /
      Last update: 2017-03-20 18:55    [W:3.526 / U:13.364 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site