lkml.org 
[lkml]   [2022]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.15 012/846] nfc: llcp: fix NULL error pointer dereference on sendmsg() after failed bind()
    Date
    From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

    commit dded08927ca3c31a5c37f8e7f95fe98770475dd4 upstream.

    Syzbot detected a NULL pointer dereference of nfc_llcp_sock->dev pointer
    (which is a 'struct nfc_dev *') with calls to llcp_sock_sendmsg() after
    a failed llcp_sock_bind(). The message being sent is a SOCK_DGRAM.

    KASAN report:

    BUG: KASAN: null-ptr-deref in nfc_alloc_send_skb+0x2d/0xc0
    Read of size 4 at addr 00000000000005c8 by task llcp_sock_nfc_a/899

    CPU: 5 PID: 899 Comm: llcp_sock_nfc_a Not tainted 5.16.0-rc6-next-20211224-00001-gc6437fbf18b0 #125
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-2 04/01/2014
    Call Trace:
    <TASK>
    dump_stack_lvl+0x45/0x59
    ? nfc_alloc_send_skb+0x2d/0xc0
    __kasan_report.cold+0x117/0x11c
    ? mark_lock+0x480/0x4f0
    ? nfc_alloc_send_skb+0x2d/0xc0
    kasan_report+0x38/0x50
    nfc_alloc_send_skb+0x2d/0xc0
    nfc_llcp_send_ui_frame+0x18c/0x2a0
    ? nfc_llcp_send_i_frame+0x230/0x230
    ? __local_bh_enable_ip+0x86/0xe0
    ? llcp_sock_connect+0x470/0x470
    ? llcp_sock_connect+0x470/0x470
    sock_sendmsg+0x8e/0xa0
    ____sys_sendmsg+0x253/0x3f0
    ...

    The issue was visible only with multiple simultaneous calls to bind() and
    sendmsg(), which resulted in most of the bind() calls to fail. The
    bind() was failing on checking if there is available WKS/SDP/SAP
    (respective bit in 'struct nfc_llcp_local' fields). When there was no
    available WKS/SDP/SAP, the bind returned error but the sendmsg() to such
    socket was able to trigger mentioned NULL pointer dereference of
    nfc_llcp_sock->dev.

    The code looks simply racy and currently it protects several paths
    against race with checks for (!nfc_llcp_sock->local) which is NULL-ified
    in error paths of bind(). The llcp_sock_sendmsg() did not have such
    check but called function nfc_llcp_send_ui_frame() had, although not
    protected with lock_sock().

    Therefore the race could look like (same socket is used all the time):
    CPU0 CPU1
    ==== ====
    llcp_sock_bind()
    - lock_sock()
    - success
    - release_sock()
    - return 0
    llcp_sock_sendmsg()
    - lock_sock()
    - release_sock()
    llcp_sock_bind(), same socket
    - lock_sock()
    - error
    - nfc_llcp_send_ui_frame()
    - if (!llcp_sock->local)
    - llcp_sock->local = NULL
    - nfc_put_device(dev)
    - dereference llcp_sock->dev
    - release_sock()
    - return -ERRNO

    The nfc_llcp_send_ui_frame() checked llcp_sock->local outside of the
    lock, which is racy and ineffective check. Instead, its caller
    llcp_sock_sendmsg(), should perform the check inside lock_sock().

    Reported-and-tested-by: syzbot+7f23bcddf626e0593a39@syzkaller.appspotmail.com
    Fixes: b874dec21d1c ("NFC: Implement LLCP connection less Tx path")
    Cc: <stable@vger.kernel.org>
    Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    net/nfc/llcp_sock.c | 5 +++++
    1 file changed, 5 insertions(+)

    --- a/net/nfc/llcp_sock.c
    +++ b/net/nfc/llcp_sock.c
    @@ -789,6 +789,11 @@ static int llcp_sock_sendmsg(struct sock

    lock_sock(sk);

    + if (!llcp_sock->local) {
    + release_sock(sk);
    + return -ENODEV;
    + }
    +
    if (sk->sk_type == SOCK_DGRAM) {
    DECLARE_SOCKADDR(struct sockaddr_nfc_llcp *, addr,
    msg->msg_name);

    \
     
     \ /
      Last update: 2022-01-24 21:44    [W:4.327 / U:0.512 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site