From patchwork Wed Aug 4 15:47:07 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Desmond Cheong Zhi Xi X-Patchwork-Id: 492369 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D795AC4338F for ; Wed, 4 Aug 2021 15:48:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C0C7660F38 for ; Wed, 4 Aug 2021 15:48:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239389AbhHDPst (ORCPT ); Wed, 4 Aug 2021 11:48:49 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45214 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239383AbhHDPss (ORCPT ); Wed, 4 Aug 2021 11:48:48 -0400 Received: from mail-pl1-x632.google.com (mail-pl1-x632.google.com [IPv6:2607:f8b0:4864:20::632]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D2A8EC06179B; Wed, 4 Aug 2021 08:48:34 -0700 (PDT) Received: by mail-pl1-x632.google.com with SMTP id d1so3472630pll.1; Wed, 04 Aug 2021 08:48:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=srAeYY/EiQIT3OULRf8vlai9yqHxF+kaop05ae2z79I=; b=VYfLn0s4o2Go241cmmAwbVoaNXcWbuvcxb7ttLjpBumxdXVxVaRYGolXGBzKUMpoa6 z9AbtDtq7uF6W/PCu/WNstLyO4+LNSyxw1b5fKI0+YfNQmk144f6XnEoVtf0BeHzFxvn 64H9s1kJgf+ZYm4+M53Yt0YIHGEDygcFguy5LjLf5Nwx0xJjYYdtFV0iP7R1so6oxEAR 2wXVKN2K/Lu8ZdZOUPBzKPNxXZBfa/ghHdWq+Jzx1S/flTyEYlpqYL+T6FTV5n5eJGbN FgF/mbl6UzMwmXV50fFIEN4pHixgITcqX70ZLMItkiJE4iBFjgEJKzrpIdCLynQA+jOn Y/zA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=srAeYY/EiQIT3OULRf8vlai9yqHxF+kaop05ae2z79I=; b=PLhXoc1kWhpi/qw5eYfMviqzZT4e894Akt/JQD8K8B7Z6LF/PYjUOJWOPRS7KyzhL4 bFXnKsjMQnZB5jVH512aGaVo25dNPB+eILIY3zUaxswU7nG+QzT9cN9oJwVFbN5mcjaY j4vCuNZFN8DyxWTmfHSMNIiv4odk502U3cMPNuIeRhbD3tWFYhj+/G9J3kRP0RqCnOvb 2oTsBn+oI1N0pLHUcMWMccfc2HXnleBFUK9wiuoxxPma9zXdQ1EFvKn6eEQKKQh9L1qH ujWhY9WdmltbCcFEO1jopmH15hEY4/fldte6UM8V4kdaIY1MiI07opDxJXnKHuQTD1fD Sw3g== X-Gm-Message-State: AOAM532B23qGvyFmB/BDkL7ukH7To4N09nXX8e2r4EgxLYbFTF4SkOfx 7HHGVYVvM05pxxGYuUJoDwqf2QPfvot3tLSZDJo= X-Google-Smtp-Source: ABdhPJzZmGIFBad16fXXF1kJzEl3JTk0YM8BTmqcbUeRXCJX1FP0+x8vlQzS3cErkbzki47LHW0a3Q== X-Received: by 2002:a17:903:1243:b029:107:eca4:d5bf with SMTP id u3-20020a1709031243b0290107eca4d5bfmr260942plh.15.1628092114325; Wed, 04 Aug 2021 08:48:34 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id b15sm4007274pgj.60.2021.08.04.08.48.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Aug 2021 08:48:33 -0700 (PDT) From: Desmond Cheong Zhi Xi To: marcel@holtmann.org, johan.hedberg@gmail.com, luiz.dentz@gmail.com, davem@davemloft.net, kuba@kernel.org, sudipm.mukherjee@gmail.com Cc: Desmond Cheong Zhi Xi , linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org, syzbot+2f6d7c28bb4bf7e82060@syzkaller.appspotmail.com Subject: [RESEND PATCH v5 1/6] Bluetooth: schedule SCO timeouts with delayed_work Date: Wed, 4 Aug 2021 23:47:07 +0800 Message-Id: <20210804154712.929986-2-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210804154712.929986-1-desmondcheongzx@gmail.com> References: <20210804154712.929986-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org struct sock.sk_timer should be used as a sock cleanup timer. However, SCO uses it to implement sock timeouts. This causes issues because struct sock.sk_timer's callback is run in an IRQ context, and the timer callback function sco_sock_timeout takes a spin lock on the socket. However, other functions such as sco_conn_del and sco_conn_ready take the spin lock with interrupts enabled. This inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} lock usage could lead to deadlocks as reported by Syzbot [1]: CPU0 ---- lock(slock-AF_BLUETOOTH-BTPROTO_SCO); lock(slock-AF_BLUETOOTH-BTPROTO_SCO); To fix this, we use delayed work to implement SCO sock timouts instead. This allows us to avoid taking the spin lock on the socket in an IRQ context, and corrects the misuse of struct sock.sk_timer. As a note, cancel_delayed_work is used instead of cancel_delayed_work_sync in sco_sock_set_timer and sco_sock_clear_timer to avoid a deadlock. In the future, the call to bh_lock_sock inside sco_sock_timeout should be changed to lock_sock to synchronize with other functions using lock_sock. However, since sco_sock_set_timer and sco_sock_clear_timer are sometimes called under the locked socket (in sco_connect and __sco_sock_close), cancel_delayed_work_sync might cause them to sleep until an sco_sock_timeout that has started finishes running. But sco_sock_timeout would also sleep until it can grab the lock_sock. Using cancel_delayed_work is fine because sco_sock_timeout does not change from run to run, hence there is no functional difference between: 1. waiting for a timeout to finish running before scheduling another timeout 2. scheduling another timeout while a timeout is running. Link: https://syzkaller.appspot.com/bug?id=9089d89de0502e120f234ca0fc8a703f7368b31e [1] Reported-by: syzbot+2f6d7c28bb4bf7e82060@syzkaller.appspotmail.com Tested-by: syzbot+2f6d7c28bb4bf7e82060@syzkaller.appspotmail.com Signed-off-by: Desmond Cheong Zhi Xi --- net/bluetooth/sco.c | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index ffa2a77a3e4c..89cb987ca9eb 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -48,6 +48,8 @@ struct sco_conn { spinlock_t lock; struct sock *sk; + struct delayed_work timeout_work; + unsigned int mtu; }; @@ -74,9 +76,20 @@ struct sco_pinfo { #define SCO_CONN_TIMEOUT (HZ * 40) #define SCO_DISCONN_TIMEOUT (HZ * 2) -static void sco_sock_timeout(struct timer_list *t) +static void sco_sock_timeout(struct work_struct *work) { - struct sock *sk = from_timer(sk, t, sk_timer); + struct sco_conn *conn = container_of(work, struct sco_conn, + timeout_work.work); + struct sock *sk; + + sco_conn_lock(conn); + sk = conn->sk; + if (sk) + sock_hold(sk); + sco_conn_unlock(conn); + + if (!sk) + return; BT_DBG("sock %p state %d", sk, sk->sk_state); @@ -91,14 +104,27 @@ static void sco_sock_timeout(struct timer_list *t) static void sco_sock_set_timer(struct sock *sk, long timeout) { + struct delayed_work *work; + + if (!sco_pi(sk)->conn) + return; + work = &sco_pi(sk)->conn->timeout_work; + BT_DBG("sock %p state %d timeout %ld", sk, sk->sk_state, timeout); - sk_reset_timer(sk, &sk->sk_timer, jiffies + timeout); + cancel_delayed_work(work); + schedule_delayed_work(work, timeout); } static void sco_sock_clear_timer(struct sock *sk) { + struct delayed_work *work; + + if (!sco_pi(sk)->conn) + return; + work = &sco_pi(sk)->conn->timeout_work; + BT_DBG("sock %p state %d", sk, sk->sk_state); - sk_stop_timer(sk, &sk->sk_timer); + cancel_delayed_work(work); } /* ---- SCO connections ---- */ @@ -179,6 +205,9 @@ static void sco_conn_del(struct hci_conn *hcon, int err) bh_unlock_sock(sk); sco_sock_kill(sk); sock_put(sk); + + /* Ensure no more work items will run before freeing conn. */ + cancel_delayed_work_sync(&conn->timeout_work); } hcon->sco_data = NULL; @@ -193,6 +222,8 @@ static void __sco_chan_add(struct sco_conn *conn, struct sock *sk, sco_pi(sk)->conn = conn; conn->sk = sk; + INIT_DELAYED_WORK(&conn->timeout_work, sco_sock_timeout); + if (parent) bt_accept_enqueue(parent, sk, true); } @@ -500,8 +531,6 @@ static struct sock *sco_sock_alloc(struct net *net, struct socket *sock, sco_pi(sk)->setting = BT_VOICE_CVSD_16BIT; - timer_setup(&sk->sk_timer, sco_sock_timeout, 0); - bt_sock_link(&sco_sk_list, sk); return sk; } From patchwork Wed Aug 4 15:47:08 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Desmond Cheong Zhi Xi X-Patchwork-Id: 491770 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EBD26C432BE for ; Wed, 4 Aug 2021 15:48:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C3F5E60F25 for ; Wed, 4 Aug 2021 15:48:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239420AbhHDPsz (ORCPT ); Wed, 4 Aug 2021 11:48:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45234 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239409AbhHDPsx (ORCPT ); Wed, 4 Aug 2021 11:48:53 -0400 Received: from mail-pj1-x1035.google.com (mail-pj1-x1035.google.com [IPv6:2607:f8b0:4864:20::1035]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CC27CC0613D5; Wed, 4 Aug 2021 08:48:39 -0700 (PDT) Received: by mail-pj1-x1035.google.com with SMTP id a8so3577655pjk.4; Wed, 04 Aug 2021 08:48:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=vp3C+KtcRQlpP3ktxmRwtjoom3vC9256tobMdzr+KLo=; b=Hv6DWT6UR0vSemrRLFQOA2Dj0WxkNrQKTcBbLwQ8d8W6kU2l8MosJ30oWS1qqwDRqI saE9jm2Ku6U+WdZZhhnuE7KykdQJU/15VNLl40uR1dNQDrL8Ei5HFDMWu2jFhEqmZyvW IXpjATDxHT28TyI6fu5f9IDyU5cxxZ9LsxKJm0Vhlfjo/BUN55Oj4V86JZ26zaYjN13o dUYMisFueGoLkWwH1mlM+zqmMAecobWcts9Bh2/pi63tN/uUfuOwxqkpYgy4Am4FZ+UN 4aEHBUtOLsq9+b2TTA/fPbChyWV+KVlsigqLzitRn6FYqocKcbWJXozgEjDWY1pI/8sZ CUDw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=vp3C+KtcRQlpP3ktxmRwtjoom3vC9256tobMdzr+KLo=; b=P0czZrXRcYRwe9FY902p2KmA0XPi1rHSOrHeW4wV2Dz7KkG5FT/ixaq0c8wz98Xt+k qjPZtC8AbqD1SuTUkD9I3n55uv/M/4lvDNecnUkuKNNShd4FVt3gYk8bCTkwVMjgSjC8 wh6MVGLSUxRbtpn9/uhrBTkWFv27Z0kTuS+YUCWmQuzEAfWK/jqbL4LhWfgPK529dvTq 2Wz8XplPDDcL9h2I2tspCVESv28cfcZr5iGXXHsxCbiLJ0oXanvkdjxfYSstkYeDCINb ggJgWsPHypkNwuZ5EBTBC/76vFLlwxfrpA5IEJR4kZnhP58s/Kw6zjSAl2mAo3P24FS4 t9vg== X-Gm-Message-State: AOAM531CAI82h6XOIcFAUwfr5AgzTBd+T2yzEfyMxMZ3bmGcMkRVNosJ OvsHYjJaj+FUsb9UL5nuzPg= X-Google-Smtp-Source: ABdhPJyeupx/uP0zAlAZwAaicY4yVFhc8T1pwXRpIbK9ymWB8Sd6UXvnmpDAUVVWpUb/C7LtUlpcWA== X-Received: by 2002:a63:1656:: with SMTP id 22mr1266007pgw.163.1628092119370; Wed, 04 Aug 2021 08:48:39 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id b15sm4007274pgj.60.2021.08.04.08.48.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Aug 2021 08:48:38 -0700 (PDT) From: Desmond Cheong Zhi Xi To: marcel@holtmann.org, johan.hedberg@gmail.com, luiz.dentz@gmail.com, davem@davemloft.net, kuba@kernel.org, sudipm.mukherjee@gmail.com Cc: Desmond Cheong Zhi Xi , linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [RESEND PATCH v5 2/6] Bluetooth: avoid circular locks in sco_sock_connect Date: Wed, 4 Aug 2021 23:47:08 +0800 Message-Id: <20210804154712.929986-3-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210804154712.929986-1-desmondcheongzx@gmail.com> References: <20210804154712.929986-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org In a future patch, calls to bh_lock_sock in sco.c should be replaced by lock_sock now that none of the functions are run in IRQ context. However, doing so results in a circular locking dependency: ====================================================== WARNING: possible circular locking dependency detected 5.14.0-rc4-syzkaller #0 Not tainted ------------------------------------------------------ syz-executor.2/14867 is trying to acquire lock: ffff88803e3c1120 (sk_lock-AF_BLUETOOTH-BTPROTO_SCO){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1613 [inline] ffff88803e3c1120 (sk_lock-AF_BLUETOOTH-BTPROTO_SCO){+.+.}-{0:0}, at: sco_conn_del+0x12a/0x2a0 net/bluetooth/sco.c:191 but task is already holding lock: ffffffff8d2dc7c8 (hci_cb_list_lock){+.+.}-{3:3}, at: hci_disconn_cfm include/net/bluetooth/hci_core.h:1497 [inline] ffffffff8d2dc7c8 (hci_cb_list_lock){+.+.}-{3:3}, at: hci_conn_hash_flush+0xda/0x260 net/bluetooth/hci_conn.c:1608 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (hci_cb_list_lock){+.+.}-{3:3}: __mutex_lock_common kernel/locking/mutex.c:959 [inline] __mutex_lock+0x12a/0x10a0 kernel/locking/mutex.c:1104 hci_connect_cfm include/net/bluetooth/hci_core.h:1482 [inline] hci_remote_features_evt net/bluetooth/hci_event.c:3263 [inline] hci_event_packet+0x2f4d/0x7c50 net/bluetooth/hci_event.c:6240 hci_rx_work+0x4f8/0xd30 net/bluetooth/hci_core.c:5122 process_one_work+0x98d/0x1630 kernel/workqueue.c:2276 worker_thread+0x658/0x11f0 kernel/workqueue.c:2422 kthread+0x3e5/0x4d0 kernel/kthread.c:319 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295 -> #1 (&hdev->lock){+.+.}-{3:3}: __mutex_lock_common kernel/locking/mutex.c:959 [inline] __mutex_lock+0x12a/0x10a0 kernel/locking/mutex.c:1104 sco_connect net/bluetooth/sco.c:245 [inline] sco_sock_connect+0x227/0xa10 net/bluetooth/sco.c:601 __sys_connect_file+0x155/0x1a0 net/socket.c:1879 __sys_connect+0x161/0x190 net/socket.c:1896 __do_sys_connect net/socket.c:1906 [inline] __se_sys_connect net/socket.c:1903 [inline] __x64_sys_connect+0x6f/0xb0 net/socket.c:1903 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae -> #0 (sk_lock-AF_BLUETOOTH-BTPROTO_SCO){+.+.}-{0:0}: check_prev_add kernel/locking/lockdep.c:3051 [inline] check_prevs_add kernel/locking/lockdep.c:3174 [inline] validate_chain kernel/locking/lockdep.c:3789 [inline] __lock_acquire+0x2a07/0x54a0 kernel/locking/lockdep.c:5015 lock_acquire kernel/locking/lockdep.c:5625 [inline] lock_acquire+0x1ab/0x510 kernel/locking/lockdep.c:5590 lock_sock_nested+0xca/0x120 net/core/sock.c:3170 lock_sock include/net/sock.h:1613 [inline] sco_conn_del+0x12a/0x2a0 net/bluetooth/sco.c:191 sco_disconn_cfm+0x71/0xb0 net/bluetooth/sco.c:1202 hci_disconn_cfm include/net/bluetooth/hci_core.h:1500 [inline] hci_conn_hash_flush+0x127/0x260 net/bluetooth/hci_conn.c:1608 hci_dev_do_close+0x528/0x1130 net/bluetooth/hci_core.c:1778 hci_unregister_dev+0x1c0/0x5a0 net/bluetooth/hci_core.c:4015 vhci_release+0x70/0xe0 drivers/bluetooth/hci_vhci.c:340 __fput+0x288/0x920 fs/file_table.c:280 task_work_run+0xdd/0x1a0 kernel/task_work.c:164 exit_task_work include/linux/task_work.h:32 [inline] do_exit+0xbd4/0x2a60 kernel/exit.c:825 do_group_exit+0x125/0x310 kernel/exit.c:922 get_signal+0x47f/0x2160 kernel/signal.c:2808 arch_do_signal_or_restart+0x2a9/0x1c40 arch/x86/kernel/signal.c:865 handle_signal_work kernel/entry/common.c:148 [inline] exit_to_user_mode_loop kernel/entry/common.c:172 [inline] exit_to_user_mode_prepare+0x17d/0x290 kernel/entry/common.c:209 __syscall_exit_to_user_mode_work kernel/entry/common.c:291 [inline] syscall_exit_to_user_mode+0x19/0x60 kernel/entry/common.c:302 ret_from_fork+0x15/0x30 arch/x86/entry/entry_64.S:288 other info that might help us debug this: Chain exists of: sk_lock-AF_BLUETOOTH-BTPROTO_SCO --> &hdev->lock --> hci_cb_list_lock Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(hci_cb_list_lock); lock(&hdev->lock); lock(hci_cb_list_lock); lock(sk_lock-AF_BLUETOOTH-BTPROTO_SCO); *** DEADLOCK *** The issue is that the lock hierarchy should go from &hdev->lock --> hci_cb_list_lock --> sk_lock-AF_BLUETOOTH-BTPROTO_SCO. For example, one such call trace is: hci_dev_do_close(): hci_dev_lock(); hci_conn_hash_flush(): hci_disconn_cfm(): mutex_lock(&hci_cb_list_lock); sco_disconn_cfm(): sco_conn_del(): lock_sock(sk); However, in sco_sock_connect, we call lock_sock before calling hci_dev_lock inside sco_connect, thus inverting the lock hierarchy. We fix this by pulling the call to hci_dev_lock out from sco_connect. Signed-off-by: Desmond Cheong Zhi Xi --- net/bluetooth/sco.c | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index 89cb987ca9eb..558f8874b65e 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -243,44 +243,32 @@ static int sco_chan_add(struct sco_conn *conn, struct sock *sk, return err; } -static int sco_connect(struct sock *sk) +static int sco_connect(struct hci_dev *hdev, struct sock *sk) { struct sco_conn *conn; struct hci_conn *hcon; - struct hci_dev *hdev; int err, type; BT_DBG("%pMR -> %pMR", &sco_pi(sk)->src, &sco_pi(sk)->dst); - hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR); - if (!hdev) - return -EHOSTUNREACH; - - hci_dev_lock(hdev); - if (lmp_esco_capable(hdev) && !disable_esco) type = ESCO_LINK; else type = SCO_LINK; if (sco_pi(sk)->setting == BT_VOICE_TRANSPARENT && - (!lmp_transp_capable(hdev) || !lmp_esco_capable(hdev))) { - err = -EOPNOTSUPP; - goto done; - } + (!lmp_transp_capable(hdev) || !lmp_esco_capable(hdev))) + return -EOPNOTSUPP; hcon = hci_connect_sco(hdev, type, &sco_pi(sk)->dst, sco_pi(sk)->setting); - if (IS_ERR(hcon)) { - err = PTR_ERR(hcon); - goto done; - } + if (IS_ERR(hcon)) + return PTR_ERR(hcon); conn = sco_conn_add(hcon); if (!conn) { hci_conn_drop(hcon); - err = -ENOMEM; - goto done; + return -ENOMEM; } /* Update source addr of the socket */ @@ -288,7 +276,7 @@ static int sco_connect(struct sock *sk) err = sco_chan_add(conn, sk, NULL); if (err) - goto done; + return err; if (hcon->state == BT_CONNECTED) { sco_sock_clear_timer(sk); @@ -298,9 +286,6 @@ static int sco_connect(struct sock *sk) sco_sock_set_timer(sk, sk->sk_sndtimeo); } -done: - hci_dev_unlock(hdev); - hci_dev_put(hdev); return err; } @@ -595,6 +580,7 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen { struct sockaddr_sco *sa = (struct sockaddr_sco *) addr; struct sock *sk = sock->sk; + struct hci_dev *hdev; int err; BT_DBG("sk %p", sk); @@ -609,12 +595,19 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen if (sk->sk_type != SOCK_SEQPACKET) return -EINVAL; + hdev = hci_get_route(&sa->sco_bdaddr, &sco_pi(sk)->src, BDADDR_BREDR); + if (!hdev) + return -EHOSTUNREACH; + hci_dev_lock(hdev); + lock_sock(sk); /* Set destination address and psm */ bacpy(&sco_pi(sk)->dst, &sa->sco_bdaddr); - err = sco_connect(sk); + err = sco_connect(hdev, sk); + hci_dev_unlock(hdev); + hci_dev_put(hdev); if (err) goto done; From patchwork Wed Aug 4 15:47:09 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Desmond Cheong Zhi Xi X-Patchwork-Id: 492368 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 92A0DC4320E for ; Wed, 4 Aug 2021 15:48:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7E03561004 for ; Wed, 4 Aug 2021 15:48:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239443AbhHDPtB (ORCPT ); Wed, 4 Aug 2021 11:49:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45254 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239456AbhHDPs6 (ORCPT ); Wed, 4 Aug 2021 11:48:58 -0400 Received: from mail-pj1-x1035.google.com (mail-pj1-x1035.google.com [IPv6:2607:f8b0:4864:20::1035]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9B05FC0613D5; Wed, 4 Aug 2021 08:48:44 -0700 (PDT) Received: by mail-pj1-x1035.google.com with SMTP id pj14-20020a17090b4f4eb029017786cf98f9so4188327pjb.2; Wed, 04 Aug 2021 08:48:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=TRKYkdOIgWJ5EiGOGovL7BoCL888xKaE/ywEpPEztaE=; b=nGMnEBdsX4+MpiNfdvpcYZqkEXFJ4tGdh6hc/ltfFv7odyfHgRyqi3UYjNi9PRnx67 YiVXS1xjUN9bFFrqHRt2ROQzvhtI9acxdZfguDZsnIA/Vf9kKZQGTyjuQXqcZhNvjF7j 9xecdG/MkBcM/0CCWaBmiC5IvKJW+hXe5Hi09/awZOGg2YGlYNTOV3QPiPikv7QzChTA rwTdzsmSNMzcbyTFvR9QM1rcpiKsDcxxGASQRz+brZIY5M7vIVMQyj2zadSk/FXtMfP2 zV8SY7Tr4vL8lZLL7MQ9JZ5oke67NmQnJwH/h9FdEdDq3mrSxfbzRozaDiJJ836PZSt9 +ekQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=TRKYkdOIgWJ5EiGOGovL7BoCL888xKaE/ywEpPEztaE=; b=UxTVURf896xassJYsF4eDci21YCe3rbaXMmFsUAi8Vh2Ab+Q7F7u9/5GUbuARF3oMi gIRKoc7TcUCODDFdR8kUVip+mWtp0Wfg6PrkZ0dOllJ9gHW2Y7kyEF3JKMAd1gBwaXfy kESjfOG5mRaDz4DIHAEyf372aJSzI/7DM9R0ZwlEmGX4L0mpahcl3yWJMllEO/Q+I1cy SlGubvpRFnlDTuIRLntjhUhb4q+TGWTIRbXizxMjvEWC1CiH4tU0H3yAkRrHFsLeP0lO iBKtXKiccU6okRwl06CFNeTURfTRH2bPBRSG9Hpnk6ksoEcC/6Bb3LgDzZjxLgHAZlIm hUeg== X-Gm-Message-State: AOAM533WesUG2opAQbeWR84TzJhmZ2vUqOJBnf/WbgOvUxik58sfA1uJ uHV0szckDgJOxpIB+CT1cf4= X-Google-Smtp-Source: ABdhPJx42+g+x6dssj4TwISrbE5rseWzNenVqgOxng0l8sgY84Zpv57p0+bxmSe2pIZp5b98W4jm0w== X-Received: by 2002:a05:6a00:888:b029:3c3:ff1:38e with SMTP id q8-20020a056a000888b02903c30ff1038emr10350145pfj.17.1628092124217; Wed, 04 Aug 2021 08:48:44 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id b15sm4007274pgj.60.2021.08.04.08.48.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Aug 2021 08:48:43 -0700 (PDT) From: Desmond Cheong Zhi Xi To: marcel@holtmann.org, johan.hedberg@gmail.com, luiz.dentz@gmail.com, davem@davemloft.net, kuba@kernel.org, sudipm.mukherjee@gmail.com Cc: Desmond Cheong Zhi Xi , linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [RESEND PATCH v5 3/6] Bluetooth: switch to lock_sock in SCO Date: Wed, 4 Aug 2021 23:47:09 +0800 Message-Id: <20210804154712.929986-4-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210804154712.929986-1-desmondcheongzx@gmail.com> References: <20210804154712.929986-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Since sco_sock_timeout is now scheduled using delayed work, it is no longer run in SOFTIRQ context. Hence bh_lock_sock is no longer necessary in SCO to synchronise between user contexts and SOFTIRQ processing. As such, calls to bh_lock_sock should be replaced with lock_sock to synchronize with other concurrent processes that use lock_sock. Signed-off-by: Desmond Cheong Zhi Xi --- net/bluetooth/sco.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index 558f8874b65e..1246e6bc09fe 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -93,10 +93,10 @@ static void sco_sock_timeout(struct work_struct *work) BT_DBG("sock %p state %d", sk, sk->sk_state); - bh_lock_sock(sk); + lock_sock(sk); sk->sk_err = ETIMEDOUT; sk->sk_state_change(sk); - bh_unlock_sock(sk); + release_sock(sk); sco_sock_kill(sk); sock_put(sk); @@ -199,10 +199,10 @@ static void sco_conn_del(struct hci_conn *hcon, int err) if (sk) { sock_hold(sk); - bh_lock_sock(sk); + lock_sock(sk); sco_sock_clear_timer(sk); sco_chan_del(sk, err); - bh_unlock_sock(sk); + release_sock(sk); sco_sock_kill(sk); sock_put(sk); @@ -1111,10 +1111,10 @@ static void sco_conn_ready(struct sco_conn *conn) if (sk) { sco_sock_clear_timer(sk); - bh_lock_sock(sk); + lock_sock(sk); sk->sk_state = BT_CONNECTED; sk->sk_state_change(sk); - bh_unlock_sock(sk); + release_sock(sk); } else { sco_conn_lock(conn); @@ -1129,12 +1129,12 @@ static void sco_conn_ready(struct sco_conn *conn) return; } - bh_lock_sock(parent); + lock_sock(parent); sk = sco_sock_alloc(sock_net(parent), NULL, BTPROTO_SCO, GFP_ATOMIC, 0); if (!sk) { - bh_unlock_sock(parent); + release_sock(parent); sco_conn_unlock(conn); return; } @@ -1155,7 +1155,7 @@ static void sco_conn_ready(struct sco_conn *conn) /* Wake up parent */ parent->sk_data_ready(parent); - bh_unlock_sock(parent); + release_sock(parent); sco_conn_unlock(conn); } From patchwork Wed Aug 4 15:47:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Desmond Cheong Zhi Xi X-Patchwork-Id: 491769 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D1D9CC4320A for ; Wed, 4 Aug 2021 15:49:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B980C60F14 for ; Wed, 4 Aug 2021 15:49:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239470AbhHDPtF (ORCPT ); Wed, 4 Aug 2021 11:49:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45296 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239479AbhHDPtC (ORCPT ); Wed, 4 Aug 2021 11:49:02 -0400 Received: from mail-pj1-x1030.google.com (mail-pj1-x1030.google.com [IPv6:2607:f8b0:4864:20::1030]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 55AE3C06179C; Wed, 4 Aug 2021 08:48:49 -0700 (PDT) Received: by mail-pj1-x1030.google.com with SMTP id dw2-20020a17090b0942b0290177cb475142so9345165pjb.2; Wed, 04 Aug 2021 08:48:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=wuePCI4FbrrQHwWiYSThmxmEPxR9cgsWrPSKODj9+7k=; b=Jk784znaiM1kGzC+vnzHdPjoAikvb+/ECITQ/3FZg8UstIATIWcr++9Q07PlDBGGUP NaZtAJZxRgmuzD+K9iINe2MVdgMb/mgSYE/C/Bj76UmTmdwi8fd4jpzmmZwJDsK1FOA/ a6dIAUbVehG2bmaGv8ISzhb4xGtOcaZUzaalkdrxO2XVqvlcDExhETpIyboptcHOh8Cw XGe8dv2qSlS0DpWz90Zb4PazuyjhgB35oqbYaygVIa9pkCAPFPhgyAlKD7EyNJ7bk6Jj b5pKYzVPji6Uy6hl0KdqHbU2d3BwrafaePd/GBBvDmVkIUQ7XGPqigf2ODNoQWg3Ubwz Qe3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=wuePCI4FbrrQHwWiYSThmxmEPxR9cgsWrPSKODj9+7k=; b=EjWL80D/ydqnn5GsSiF/C/599CQpJArwtzlwSHR83zbR0fb+S/WKd//nuFdkHQTxd7 rWAlaYWr6FDWdlcf5axDQ16HkW+lgb2CVM8O7/MCvJdnNAzN0/uvJ7K3r44Mvmkiabt5 /aNpvrhI4zbwbxa9wZMKkt4nDL79Qjy3Oefpx+VxuTen+twXnmUqyqWN67EJP9+XlLTB shelGalwSYclkn/FolGNwhKwBbftgZK4X/7Y1kyajUgYL2iVdLWf4NI+kSZz09ZkQvW7 05YSTgCUsy5rHDwi0ike/xsJrmxI1dnopT9uEmU6G6PqEXdi4dtQBHXzKIr0QQFKlgWn pBjg== X-Gm-Message-State: AOAM533HWHdH3gYDmID0+Kgy9DCj7P0KyKxMnDcdnr4qlyy/z9ugbu2D W7hr7kkPnLHsj3b0oIfPOc8= X-Google-Smtp-Source: ABdhPJzBS90sEosr/ccisMXoi2vHaMJWErjhjg312/0a223eplEcJU9XDsNw708hnm0IgxN4KHWaXA== X-Received: by 2002:a17:90a:3fcc:: with SMTP id u12mr9997143pjm.5.1628092128953; Wed, 04 Aug 2021 08:48:48 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id b15sm4007274pgj.60.2021.08.04.08.48.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Aug 2021 08:48:48 -0700 (PDT) From: Desmond Cheong Zhi Xi To: marcel@holtmann.org, johan.hedberg@gmail.com, luiz.dentz@gmail.com, davem@davemloft.net, kuba@kernel.org, sudipm.mukherjee@gmail.com Cc: Desmond Cheong Zhi Xi , linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [RESEND PATCH v5 4/6] Bluetooth: serialize calls to sco_sock_{set, clear}_timer Date: Wed, 4 Aug 2021 23:47:10 +0800 Message-Id: <20210804154712.929986-5-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210804154712.929986-1-desmondcheongzx@gmail.com> References: <20210804154712.929986-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Currently, calls to sco_sock_set_timer are made under the locked socket, but this does not apply to all calls to sco_sock_clear_timer. Both sco_sock_{set,clear}_timer should be serialized by lock_sock to prevent unexpected concurrent clearing/setting of timers. Additionally, since sco_pi(sk)->conn is only cleared under the locked socket, this change allows us to avoid races between sco_sock_clear_timer and the call to kfree(conn) in sco_conn_del. Signed-off-by: Desmond Cheong Zhi Xi --- net/bluetooth/sco.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index 1246e6bc09fe..418543c390b3 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -459,8 +459,8 @@ static void __sco_sock_close(struct sock *sk) /* Must be called on unlocked socket. */ static void sco_sock_close(struct sock *sk) { - sco_sock_clear_timer(sk); lock_sock(sk); + sco_sock_clear_timer(sk); __sco_sock_close(sk); release_sock(sk); sco_sock_kill(sk); @@ -1110,8 +1110,8 @@ static void sco_conn_ready(struct sco_conn *conn) BT_DBG("conn %p", conn); if (sk) { - sco_sock_clear_timer(sk); lock_sock(sk); + sco_sock_clear_timer(sk); sk->sk_state = BT_CONNECTED; sk->sk_state_change(sk); release_sock(sk); From patchwork Wed Aug 4 15:47:11 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Desmond Cheong Zhi Xi X-Patchwork-Id: 492367 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AFEE8C4320E for ; Wed, 4 Aug 2021 15:49:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 987E460F25 for ; Wed, 4 Aug 2021 15:49:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239273AbhHDPte (ORCPT ); Wed, 4 Aug 2021 11:49:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45320 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239515AbhHDPtH (ORCPT ); Wed, 4 Aug 2021 11:49:07 -0400 Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 204BDC06179A; Wed, 4 Aug 2021 08:48:54 -0700 (PDT) Received: by mail-pj1-x102c.google.com with SMTP id t7-20020a17090a5d87b029017807007f23so6125309pji.5; Wed, 04 Aug 2021 08:48:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=UwpAuyr31gkKRzWsz22ieldVxq1Gat0ivFQ8uoQ/1c0=; b=tEUlbrGx7uWgTk+Yiuzx3PF5upDZnEKvMC7rC8etWlR9ez5J/YDtuPhjqo4v2GebzO VLI3xt14nJAIKyLSCDqvNQwd1FYMjV2tlv/6PW3aqrIe5f4gpFIHZGZkDQuZkJZ8puKl XoMFqpLQy2XENACDC1UY0nf3A70oZt4JwbhUCdMhejjpwDX7FqcLe/o262ewwjaUVxsC iQHBMWJ0UQh7ocqbt5sOKd/Q5rcrB6jOD+XqZueye8QZ8wLWSRWtVdV1Exxq3KHn0ge1 bcagkhmas9zQxiHT7DBSeIyrmlATtbdAcWEl+pemT6dJLiWmDz054ehJ6fsKYF/ShCdX wdeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=UwpAuyr31gkKRzWsz22ieldVxq1Gat0ivFQ8uoQ/1c0=; b=ar5VUxhuaUT8eiziKerwsp0gNwnAiaNGB8YCr/vJksKibrk4LAOT/hcQlF9HL2wttr nZmEn7DgmQmxdgokpU1lWweGo07RQ6efkUl88+ZPrPCT7qHaIigS8H1/0UA6N0EtBAA5 CUraff9PmwFOM4Va0VupnNqcK76o7U4No527+y16sN4Et2vJnIICAdohhXwuZinBMhdB 5Bf6A35ZfwFSp5JaiOaFTNdrwmPRgxoKmGYneQTxCwdlBqaFxuKR4LSYdc/S39knPPd+ lfVOLrScM6bpB5IyMkHIcr4tzan28Ny5ahjrt9w0Uv3wVvs5y9wRzJ2ukydaTyQ8mAsQ YC7Q== X-Gm-Message-State: AOAM533cMSkL+qDztBtG8eRt3/LmjACpXzwaM2ABUAZP5TgZ3YvgN51z W446ScNjcpoXoawGveLxOBc= X-Google-Smtp-Source: ABdhPJy53E2/k6DHqfUA8ASbwd2KosGoA+p9Idp0D8kBhX/RdBw3l0POikQNvzDaf8W3swv+zT9rwQ== X-Received: by 2002:a63:950a:: with SMTP id p10mr76260pgd.362.1628092133695; Wed, 04 Aug 2021 08:48:53 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id b15sm4007274pgj.60.2021.08.04.08.48.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Aug 2021 08:48:53 -0700 (PDT) From: Desmond Cheong Zhi Xi To: marcel@holtmann.org, johan.hedberg@gmail.com, luiz.dentz@gmail.com, davem@davemloft.net, kuba@kernel.org, sudipm.mukherjee@gmail.com Cc: Desmond Cheong Zhi Xi , linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [RESEND PATCH v5 5/6] Bluetooth: switch to lock_sock in RFCOMM Date: Wed, 4 Aug 2021 23:47:11 +0800 Message-Id: <20210804154712.929986-6-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210804154712.929986-1-desmondcheongzx@gmail.com> References: <20210804154712.929986-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Other than rfcomm_sk_state_change and rfcomm_connect_ind, functions in RFCOMM use lock_sock to lock the socket. Since bh_lock_sock and spin_lock_bh do not provide synchronization with lock_sock, these calls should be changed to lock_sock. This is now safe to do because packet processing is now done in a workqueue instead of a tasklet, so bh_lock_sock/spin_lock_bh are no longer necessary to synchronise between user contexts and SOFTIRQ processing. Signed-off-by: Desmond Cheong Zhi Xi --- net/bluetooth/rfcomm/sock.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c index ae6f80730561..2c95bb58f901 100644 --- a/net/bluetooth/rfcomm/sock.c +++ b/net/bluetooth/rfcomm/sock.c @@ -70,7 +70,7 @@ static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err) BT_DBG("dlc %p state %ld err %d", d, d->state, err); - spin_lock_bh(&sk->sk_lock.slock); + lock_sock(sk); if (err) sk->sk_err = err; @@ -91,7 +91,7 @@ static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err) sk->sk_state_change(sk); } - spin_unlock_bh(&sk->sk_lock.slock); + release_sock(sk); if (parent && sock_flag(sk, SOCK_ZAPPED)) { /* We have to drop DLC lock here, otherwise @@ -974,7 +974,7 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc * if (!parent) return 0; - bh_lock_sock(parent); + lock_sock(parent); /* Check for backlog size */ if (sk_acceptq_is_full(parent)) { @@ -1001,7 +1001,7 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc * result = 1; done: - bh_unlock_sock(parent); + release_sock(parent); if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags)) parent->sk_state_change(parent); From patchwork Wed Aug 4 15:47:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Desmond Cheong Zhi Xi X-Patchwork-Id: 491768 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2E8ABC432BE for ; Wed, 4 Aug 2021 15:50:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1578B60F38 for ; Wed, 4 Aug 2021 15:50:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239571AbhHDPuO (ORCPT ); Wed, 4 Aug 2021 11:50:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45386 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239555AbhHDPtX (ORCPT ); Wed, 4 Aug 2021 11:49:23 -0400 Received: from mail-pj1-x1032.google.com (mail-pj1-x1032.google.com [IPv6:2607:f8b0:4864:20::1032]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4676AC0617A0; Wed, 4 Aug 2021 08:48:59 -0700 (PDT) Received: by mail-pj1-x1032.google.com with SMTP id pj14-20020a17090b4f4eb029017786cf98f9so4189184pjb.2; Wed, 04 Aug 2021 08:48:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=zIRbv5ZXxIid872Ol8a8JiBpz6JG4aviAWXZnPlaMDM=; b=rdJHyKNkVdzEDu+i17BA0ESSeSHw7WdgDcmPzNzHC5dNA/1Da4om9QYVZAQ4eTNOjp nE7CuugZzapfrdXloo+i/YUsSvvhPFb8bL/XF5p6FrBQ0OqA2vxwuuZ2sM5q1vxRpRue n0O0otg/gigaHc2qifkS2eW1v/eG19WJXQ/iS+t6g1Esnq/NqkYKxcYuQjkLbTq/6COt CAuJtvNvD3qolCfKL0fDxxGKxq6BLmjxxVeIP6P1H1GHP9ukH7cFRBxdeCgh2Iv4T8tC ukT5oftekjWfwOTmCOFM6YyyGyTXX/3a8wSbvliCRkgp6dtFCLUCCTzM5CrqfO7m9XU5 Do0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=zIRbv5ZXxIid872Ol8a8JiBpz6JG4aviAWXZnPlaMDM=; b=ersTFvuKsw/bo1mk+TXK65cmFfXMPrBNu8rOsAQYYVLdh5iztpnZpocGq1Mi5F8B36 dTSrT5AinLzXTrRanJuI85MjCSlJi1nGCLCi0Sr+TZLn8gA8UjyPnzH8JNVf8cO0CyZs m5Fm+HzReU9yEsQ2tMcJe7pbIT81dTcpzWo4u2dUn2lgF2SZPBMCkcs97yR5zUNEMarX /g9zNBQI9kQ01IBVVxPoKqzaYZZOGx+oWaSbTmZ17m2JcqNNqYWIC17pYIGpSnpx4i0x cUur+GW5D9t15+f/tFfuMsjLNHSNBe8tv+baWHoX7y5QiOj5oqOdnD5OJFhiw0FVTozV uKIA== X-Gm-Message-State: AOAM531hyo+muHu7o/2QdNqD4dyaiVxZQU2dvbDGvfPyLgyf0sI+hKot OXaQaZ8/Ny5z3HLt9Ph2yjg= X-Google-Smtp-Source: ABdhPJwP+rVDzJk6NLCQWzc1MhjnFtm/lsX/BNe/Q5+lIAcE/FyZ6bVOH1EuWhB8m67xqjLUaSJHpA== X-Received: by 2002:a17:90a:bd18:: with SMTP id y24mr10513325pjr.83.1628092138838; Wed, 04 Aug 2021 08:48:58 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id b15sm4007274pgj.60.2021.08.04.08.48.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Aug 2021 08:48:58 -0700 (PDT) From: Desmond Cheong Zhi Xi To: marcel@holtmann.org, johan.hedberg@gmail.com, luiz.dentz@gmail.com, davem@davemloft.net, kuba@kernel.org, sudipm.mukherjee@gmail.com Cc: Desmond Cheong Zhi Xi , linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [RESEND PATCH v5 6/6] Bluetooth: fix repeated calls to sco_sock_kill Date: Wed, 4 Aug 2021 23:47:12 +0800 Message-Id: <20210804154712.929986-7-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210804154712.929986-1-desmondcheongzx@gmail.com> References: <20210804154712.929986-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org In commit 4e1a720d0312 ("Bluetooth: avoid killing an already killed socket"), a check was added to sco_sock_kill to skip killing a socket if the SOCK_DEAD flag was set. This was done after a trace for a use-after-free bug showed that the same sock pointer was being killed twice. Unfortunately, this check prevents sco_sock_kill from running on any socket. sco_sock_kill kills a socket only if it's zapped and orphaned, however sock_orphan announces that the socket is dead before detaching it. i.e., orphaned sockets have the SOCK_DEAD flag set. To fix this, we remove the check for SOCK_DEAD, and avoid repeated calls to sco_sock_kill by removing incorrect calls in: 1. sco_sock_timeout. The socket should not be killed on timeout as further processing is expected to be done. For example, sco_sock_connect sets the timer then waits for the socket to be connected or for an error to be returned. 2. sco_conn_del. This function should clean up resources for the connection, but the socket itself should be cleaned up in sco_sock_release. 3. sco_sock_close. Calls to sco_sock_close in sco_sock_cleanup_listen and sco_sock_release are followed by sco_sock_kill. Hence the duplicated call should be removed. Fixes: 4e1a720d0312 ("Bluetooth: avoid killing an already killed socket") Signed-off-by: Desmond Cheong Zhi Xi --- net/bluetooth/sco.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index 418543c390b3..cf43ccb50573 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -97,8 +97,6 @@ static void sco_sock_timeout(struct work_struct *work) sk->sk_err = ETIMEDOUT; sk->sk_state_change(sk); release_sock(sk); - - sco_sock_kill(sk); sock_put(sk); } @@ -203,7 +201,6 @@ static void sco_conn_del(struct hci_conn *hcon, int err) sco_sock_clear_timer(sk); sco_chan_del(sk, err); release_sock(sk); - sco_sock_kill(sk); sock_put(sk); /* Ensure no more work items will run before freeing conn. */ @@ -410,8 +407,7 @@ static void sco_sock_cleanup_listen(struct sock *parent) */ static void sco_sock_kill(struct sock *sk) { - if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket || - sock_flag(sk, SOCK_DEAD)) + if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket) return; BT_DBG("sk %p state %d", sk, sk->sk_state); @@ -463,7 +459,6 @@ static void sco_sock_close(struct sock *sk) sco_sock_clear_timer(sk); __sco_sock_close(sk); release_sock(sk); - sco_sock_kill(sk); } static void sco_skb_put_cmsg(struct sk_buff *skb, struct msghdr *msg,