From patchwork Wed Aug 4 11:03:03 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: 12418585 X-Patchwork-Delegate: kuba@kernel.org 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 5D5E7C4338F for ; Wed, 4 Aug 2021 11:03:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3E56F60F43 for ; Wed, 4 Aug 2021 11:03:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237761AbhHDLEE (ORCPT ); Wed, 4 Aug 2021 07:04:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35910 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237728AbhHDLED (ORCPT ); Wed, 4 Aug 2021 07:04:03 -0400 Received: from mail-pj1-x1034.google.com (mail-pj1-x1034.google.com [IPv6:2607:f8b0:4864:20::1034]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DB574C0613D5; Wed, 4 Aug 2021 04:03:50 -0700 (PDT) Received: by mail-pj1-x1034.google.com with SMTP id m10-20020a17090a34cab0290176b52c60ddso2975962pjf.4; Wed, 04 Aug 2021 04:03:50 -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=RGgUEIqv7EX3sOd8lMSiTQDivsxOgCMUFZDXh5+/jjewhAEgrysmVO+ACv84Gf2w+A nQvFXo5L5BeKS/AclyuD/jDVzlU8JHSUx4LkDfkP7IJXcPbZ/iswMHVf1PeLe70q2To6 0njJsUWQCZxsr3L0PIe49RDhHRk+f7qhlKmtQ0la10XXrC2nWaK5lLP4kaUyskd9M7vU PsAbUjFV6XCKlq9spLCty/nSROfI3o8y38vWlv+3ffgaNpczEc3OK1XOje8itSD3U03z diDle0TVvD0uA08W8tWF1IKt+efuaEvVqhyLK7YpVS+wm6pi44fQTcyvrg9wkfNG+ZLj 1s9Q== 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=T60x0ZgDShcNkuGbXjz86CShLXYgULZaNbMePPKDao3XY8i5ZOB4PBF+Z0lBNd3+Nr bkKwsRd+7l2z9TTUGAv14ShN5/Ga1NlsdIVSjB1GieClg8cQ9Lc11nqtSMeH/iOblsgs UtaZjuYnig4/wo6Zrw/ce2edlrKHwxJ2lVvCPYrYCp546YBYBJ7ILdBIX/gGsDFbLEHv MFUTRrmWAfmUypk6f6qJFJCOCNLCIrVerF1VeCNUyxT6NQql+4U42r9Hwb3Xpa8c3Z4K IBZRDgG15H9Eg5jX/Tcpt2nuMDKQWof7ak69FnUkcienvYN1C5HflWKr0o2gROZLQUhy 3RVA== X-Gm-Message-State: AOAM533XhWvI8om8WmY6WOqtZKHsRUkH6htGI/bUw3sxkXm1SlRi7kD7 lOo881IeEHU2TuGo2ER+Yfs= X-Google-Smtp-Source: ABdhPJw+OpqNnmqZmZ5BJkWvuLH5QpMKkTC47+FumpiJAWxcQOjVU/x/c2N0mXuzUKoSrM9tSOIZlw== X-Received: by 2002:a17:90a:c57:: with SMTP id u23mr9270081pje.186.1628075030416; Wed, 04 Aug 2021 04:03:50 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id k4sm2206147pjs.55.2021.08.04.04.03.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Aug 2021 04:03:50 -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 , 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: [PATCH v5 1/6] Bluetooth: schedule SCO timeouts with delayed_work Date: Wed, 4 Aug 2021 19:03:03 +0800 Message-Id: <20210804110308.910744-2-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210804110308.910744-1-desmondcheongzx@gmail.com> References: <20210804110308.910744-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@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 11:03:04 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: 12418587 X-Patchwork-Delegate: kuba@kernel.org 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 146B7C4338F for ; Wed, 4 Aug 2021 11:04:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id ED88661029 for ; Wed, 4 Aug 2021 11:04:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237779AbhHDLEU (ORCPT ); Wed, 4 Aug 2021 07:04:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35992 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237728AbhHDLER (ORCPT ); Wed, 4 Aug 2021 07:04:17 -0400 Received: from mail-pj1-x102d.google.com (mail-pj1-x102d.google.com [IPv6:2607:f8b0:4864:20::102d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 21FABC061798; Wed, 4 Aug 2021 04:04:04 -0700 (PDT) Received: by mail-pj1-x102d.google.com with SMTP id a8so2390588pjk.4; Wed, 04 Aug 2021 04:04:04 -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=p1NaCDuc1w5ZhC0aNsJoBG1EM/w8pyBWr6PtCIdso5t/fX4P430ujCU2rsMtP2G4YS /aKOaWmTl2aUlYnUUBbUQfTTtKwkcdsDdk3kLfaz0iuoOhrh5UMo4+GRZyl85B4qr99j 66treAQ6NnutEHYHmw9I0uhxQYsdeHR5m8GgdlQoEMJUyBZWn0zSHyGsF0kF5uT7kpO6 8PoKjD3nkiRsUTWYxEZpopX09sgzTzW/nTiKoAk6hRGcEYXYgQahZi8FEuA2nYi3UoTB VUbqQt9tGZjlV40Ji3PGuKrdhYRq0vdh5w2pbymNLgc3c7zw7FIYcc4/0wJvShThB6F+ QWrw== 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=q14NWFnD2Ms4qA8W1ocmpbJCdvOH7+R4BvZ2SANbtek0xONO2hwqLqyH4amrTV1aW1 qNsPbtajK/5o6zQGmVRgP+931RaxIdWnEq9EfmrenekoCOru+x40g4a1vMoLrULPbaU+ GIskSpYu0D7N7GNH2djeOkHmCP4kdHN3IXyvK8tBXXXME4CzJz3+PuU30fCSHyRZya8/ QKnKc+GA3s2SqVPOB+IL6cLGKxrxSNXHPtzNenYZ8tvtdNfRhJWDV8GPfLCrMghcKtBK 1PzAFV2Ab9J/cmbh1nzC43Pz4NNFYmznxdRRUobli4FtY9UjeE4IZJ6sstw5v6Sh6Mlt iEow== X-Gm-Message-State: AOAM531OKQ9JDz62/d55E5a8d+sfEpnARtSzxUaqI9unqwqwmNGHBNHg dbxe0aIOD55W0JelnclT9xqDzj6y/ziP/1HLMc4= X-Google-Smtp-Source: ABdhPJy5C2VySxRgUimihXzOoHabJIku6LCsaF4BQkeb5J4mkLavK/Upe40tdEaAxtWp+HRKPl5wUQ== X-Received: by 2002:a17:902:e74d:b029:12b:e3f2:f5d3 with SMTP id p13-20020a170902e74db029012be3f2f5d3mr22029907plf.82.1628075043657; Wed, 04 Aug 2021 04:04:03 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id k4sm2206147pjs.55.2021.08.04.04.04.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Aug 2021 04:04:03 -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 , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [PATCH v5 2/6] Bluetooth: avoid circular locks in sco_sock_connect Date: Wed, 4 Aug 2021 19:03:04 +0800 Message-Id: <20210804110308.910744-3-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210804110308.910744-1-desmondcheongzx@gmail.com> References: <20210804110308.910744-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@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 11:03:05 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: 12418589 X-Patchwork-Delegate: kuba@kernel.org 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 8CF76C4320A for ; Wed, 4 Aug 2021 11:04:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 744AC61040 for ; Wed, 4 Aug 2021 11:04:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237860AbhHDLE2 (ORCPT ); Wed, 4 Aug 2021 07:04:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36020 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237728AbhHDLEW (ORCPT ); Wed, 4 Aug 2021 07:04:22 -0400 Received: from mail-pj1-x1029.google.com (mail-pj1-x1029.google.com [IPv6:2607:f8b0:4864:20::1029]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A51BAC0613D5; Wed, 4 Aug 2021 04:04:09 -0700 (PDT) Received: by mail-pj1-x1029.google.com with SMTP id q17-20020a17090a2e11b02901757deaf2c8so3069167pjd.0; Wed, 04 Aug 2021 04:04:09 -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=Nnd8twf4rEgY2TlB+F0iTD+c7owlni5fLgZW89siB3upvnIKQhe1SVLxBm6ttpBBKQ zYMGghl8Dr6byrvyPW/fGv9Mz8OeXmaE0eux/tauYcYCPVcNUTP5z+Kc7kPM/9pCU4EU 7aIJVDH4IC/egfbrCULR7SNLwJZaQqlFQjAnxNBE24KOsGSydQ8uftEs8A157SgvMfRC 9XYoRlA2Mv2Zkc3lmQSRKjo7KjZsFnRcSk6hJGEGkJWzq9iLY/xr8xUteH0HzQ4cJkNt oLcYfUZpO8KAdwPA7n3pE8lkaLjzNMVFtT9yvEoXjOltgPiqSxE/r7At3QGWGISwcdp9 laaw== 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=ffhusX4xDmYVJN2Z+ToYSWku5tO+Vo+Ez0Ne4hIKde9VitrfoZBW7N7mElv/Fsf8W7 /KfvSU8gh4dKGhR8EI67Bxr6uCbpZ23qx1yHQOIhnYgNjNQJWTgAdLkyZaqoaD/hVhP3 i4bVXETgvtNDKgu4KS/CEtnZjVzEPNG15ArkAxitj9WSWMS5WtxRtfTkQUik7YU+lQQ+ FmACCivYt4EaRWNcJzlu6xHHorrudNE81ymj370xxP4s4yIcG+k0BjDn40XL6diNVA9A 9lH9OEGSBI4D3z59rhY6uv96L32XityDcnpEePTqMy6q++iM0v6x6zW0yzc/eZLhlOlt WegA== X-Gm-Message-State: AOAM532cu3uZGL6r41PLvSZ2I9wCAekhETIIaTUE2I4PeRvSmrcb4n/c eLPwiXNMnOCHI5jB052Edng= X-Google-Smtp-Source: ABdhPJyyxVGnR/0AchNWivoqpbLq+TyM45ed7PlSNndLnq22S/qQ5HnA87pIpqEOgWKXrrN+gBtL0Q== X-Received: by 2002:a62:17d8:0:b029:3b1:c2b0:287c with SMTP id 207-20020a6217d80000b02903b1c2b0287cmr23906446pfx.23.1628075049278; Wed, 04 Aug 2021 04:04:09 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id k4sm2206147pjs.55.2021.08.04.04.04.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Aug 2021 04:04:08 -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 , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [PATCH v5 3/6] Bluetooth: switch to lock_sock in SCO Date: Wed, 4 Aug 2021 19:03:05 +0800 Message-Id: <20210804110308.910744-4-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210804110308.910744-1-desmondcheongzx@gmail.com> References: <20210804110308.910744-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@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 11:03:06 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: 12418591 X-Patchwork-Delegate: kuba@kernel.org 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 106C9C4320E for ; Wed, 4 Aug 2021 11:04:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EA91661108 for ; Wed, 4 Aug 2021 11:04:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237741AbhHDLEf (ORCPT ); Wed, 4 Aug 2021 07:04:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36068 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237861AbhHDLE3 (ORCPT ); Wed, 4 Aug 2021 07:04:29 -0400 Received: from mail-pl1-x630.google.com (mail-pl1-x630.google.com [IPv6:2607:f8b0:4864:20::630]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 73007C06179B; Wed, 4 Aug 2021 04:04:16 -0700 (PDT) Received: by mail-pl1-x630.google.com with SMTP id u16so2618154ple.2; Wed, 04 Aug 2021 04:04:16 -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=ZeyQaebz+I8qQT4Ub6+aQ5HL2PbtkM9sacpF80+Jy0H271i/JeWa/dDeOv10raNNGj AQrWEBvfXjqQKBlptoFQ/AskMbzPCIGXHxHEaKtsfmWzHE4LKFMoA9EWLirBZgdfvNyG klhLqfChZFDcLVl5F/JK3IY/PqUevXvLL02X+29OTWs5j9jnPeZX6tz+K9wkKvDkbiX7 wPZjZ6K2E+10FGA5ofXJ7EJtdYHm5qdIRAXjpSXR3Ec+r0axCxkH0KGTdZHyGRzg5ZCr uOLz/ncGQyGSPIoA90c22XumSeSxWzqGnhYsExyWXTVFDqELt4ZCq0/T9Y5O+F2yzfUf q1RA== 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=uLjlhy4TOSggflNVhQMO/Qh0Zr3/KB6/OZ0pbwvbTyMNrzJ8SmrxWvDUMnafuEeaIG UP11bB4FsKb9e9KOfl5NRc3Q0Pd7Q3xYf5Drux4E0CDPWn2cvZZTkUO2JtRLEh2o8uxz rkldSXvdcLE1jYbTr+PnhA2CfySFBzkdZzJYxp7YJv2rt6HK8bwX4O7vuxntV+L2X9mb +ZMYIFayZBu9zb16xUMsy67Yje1BIZZ4EggNqRTn6zU696E88hjY+MtYvfPwW+HR90dD em7zS5xU3WBQpiK6oJhWAp3XH+7KynBuACOapifVaeeGw1Nlot+8ZqSxtKewND62Kbkb LXSg== X-Gm-Message-State: AOAM531fEpN8J5PTfSvyU+XMM9u39+bXPom+497UNKS9bZdAvsadOcsU cq/PzdV55Xpn3pzvPwL8Ttg= X-Google-Smtp-Source: ABdhPJzUKNNoQeqTm8yohHsjU8iT18AjzrgmdBaUfF7PHoha5iGqKI206C2n/5zO8ebm2SRAk8NvzA== X-Received: by 2002:a17:90a:2c05:: with SMTP id m5mr3335774pjd.32.1628075055960; Wed, 04 Aug 2021 04:04:15 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id k4sm2206147pjs.55.2021.08.04.04.04.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Aug 2021 04:04:15 -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 , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [PATCH v5 4/6] Bluetooth: serialize calls to sco_sock_{set,clear}_timer Date: Wed, 4 Aug 2021 19:03:06 +0800 Message-Id: <20210804110308.910744-5-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210804110308.910744-1-desmondcheongzx@gmail.com> References: <20210804110308.910744-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@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 11:03: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: 12418593 X-Patchwork-Delegate: kuba@kernel.org 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 54E9FC432BE for ; Wed, 4 Aug 2021 11:04:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 357C161037 for ; Wed, 4 Aug 2021 11:04:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237761AbhHDLEp (ORCPT ); Wed, 4 Aug 2021 07:04:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36062 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237858AbhHDLEe (ORCPT ); Wed, 4 Aug 2021 07:04:34 -0400 Received: from mail-pl1-x634.google.com (mail-pl1-x634.google.com [IPv6:2607:f8b0:4864:20::634]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9A39CC061798; Wed, 4 Aug 2021 04:04:21 -0700 (PDT) Received: by mail-pl1-x634.google.com with SMTP id q2so2559683plr.11; Wed, 04 Aug 2021 04:04:21 -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=IM7eV7s8h8oFYEZBdNbbFFs+g+dar5PebK/L8YnXDu3Z5PGgxd77mFB8JWljt0SZnl 2MLFfuX8mwFlVtwZaehF9jl1g/PHF+WPqcycPWDVhYIgrb/bQX12xkS9sqAaV3rBe3da ZdPsjSkjCGoyihie7JzyM8bkgiFjEReMtY0m1kZfbwInoqveOX7pNkT28JxaKz3gpbTd 9pzmWSmVTSQsw+E/UbbCrrNdHt2s9RsXlVtpIAnDlhp9lucv8DMoK2pN6XmzXXvjrowm XH0rRW4hVlGbxyPPP8sVo7LD4fts7oL9rSteKEIbE+iAml+pyQtfskdTsxbc+IwYkKJh LXPA== 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=qvzC51QS0PjB7aP1lrq5Bm2RNkJKms6sP3uILZwggJI7DNagKjm8yzEeFzxxD2QSf0 Cxv6VlRyukaSVRnB3cRgsvpjvhHM6c8zgLaLH8nuVLTNgSaGAKKwQg6yv1WFjtTSZu6n Jm9tiOK0WVskuq081whszyygWqq8oemHn1YzRUgonCCoZ5kXtj+g9Cy/T23NqZsHguzt WVfi1XfuEsj1vvSYB5WMsO+NexsQmZriudMXaRO9zYWaXw5d50/q24GF7QZYYgOc1jrQ SyYR8JoDQ2rjRMvnUj803aVezfurCBvpZxXnSFcMoQytplzNK/qMt3E/kFvwv2UgV26l VzWg== X-Gm-Message-State: AOAM533ZBsjRbnhON9C9R9ldaGMZ/+CiwJXGKI5+EtYtd7OyuVxMBKTV ep1kWxYOLA3eopBd06UZI44= X-Google-Smtp-Source: ABdhPJyGnCZuFwDmG+LmB5XddxwPsrpPy+oRZJaIgcMNVyhKK2wnXpX3hpbc+tN6NIbFhc9hHe0Aow== X-Received: by 2002:a17:90a:c003:: with SMTP id p3mr27366487pjt.14.1628075061236; Wed, 04 Aug 2021 04:04:21 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id k4sm2206147pjs.55.2021.08.04.04.04.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Aug 2021 04:04:20 -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 , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [PATCH v5 5/6] Bluetooth: switch to lock_sock in RFCOMM Date: Wed, 4 Aug 2021 19:03:07 +0800 Message-Id: <20210804110308.910744-6-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210804110308.910744-1-desmondcheongzx@gmail.com> References: <20210804110308.910744-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@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 11:03: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: 12418595 X-Patchwork-Delegate: kuba@kernel.org 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 9DF01C4338F for ; Wed, 4 Aug 2021 11:04:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 77CAA61029 for ; Wed, 4 Aug 2021 11:04:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237949AbhHDLEv (ORCPT ); Wed, 4 Aug 2021 07:04:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36146 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237956AbhHDLEn (ORCPT ); Wed, 4 Aug 2021 07:04:43 -0400 Received: from mail-pj1-x102a.google.com (mail-pj1-x102a.google.com [IPv6:2607:f8b0:4864:20::102a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D63A7C0617BC; Wed, 4 Aug 2021 04:04:28 -0700 (PDT) Received: by mail-pj1-x102a.google.com with SMTP id j1so2395318pjv.3; Wed, 04 Aug 2021 04:04:28 -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=dkqVASp2vIm2+9eG8qv+AApsyYt3ZndFWyEGeg91xxlutOvj87XPa41NO4TcugMcMK qJtVwlAOK9GBt4h0MMZRsUPha8YZWIN0rxsPHOkkk1I81w3VUe07s7cJA5OLVyldzBF1 +45RYhc4C3xNEwMIWp4n6o6miZNZSe3qMNMwtoS3Xp4l54BebRxP6MxrsS622bfEmjQN 6X7nMmR0Jgpot93puCvPpPHMNzZQI7YSpQuGcl99zTiIb0ZCEvxNKwzjUmQUi88LlCn9 uXfYRPoK+Qx3LRgo1GHEWOoBit+QggDmBLBgDMaqbhJBK9BqG+6sFoTe4AHpzwCYW/6+ MfBw== 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=UqaGgVj6CoVK/+DOksEsyuDLM0ELwGYIaMQ7JrVDZkpV7iku6/hJbRoOTh3Vn6qlXz nL9P18xeBdOydkC2Lsn+BgyQYqL4/tjS4kN7P1ElfGYiLFX/+pQ6kXQGRUv/ajQP2JH1 5MNazYsM7rHMUOM7huWSPpFv9WeRki9947Y9D0OVwfIK7TfSvClyYqgu2Q+TI3e4g9CG vgHv99ZeI8k7/YF2FxeDNcif8kz2iGRI0MNo5k2ZX9mmXIsBDKDgtYYWq1lLFvJkB7Yt 2Cjuy/C0koJdXw/ZYoiIxQwg9s1AjMDRDISG3rDE6oPUSexs9bq5GY62TYoK02R1Ap8I HJdA== X-Gm-Message-State: AOAM533YkCf/Bo2mgmoXlSbbkUFUi2ElN2ffjTGcXslCOlCBD2pocLfV /Mw98qZ3rHoISoEWqwLc2xw= X-Google-Smtp-Source: ABdhPJxYdG55SlPSNbYcL/QxOycARXWHFDr5RrHwa46cfambF1NpTtyt6+IEGe95qXWmdnXH+tNQBw== X-Received: by 2002:a63:171d:: with SMTP id x29mr1357294pgl.418.1628075068468; Wed, 04 Aug 2021 04:04:28 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id k4sm2206147pjs.55.2021.08.04.04.04.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Aug 2021 04:04:28 -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 , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org Subject: [PATCH v5 6/6] Bluetooth: fix repeated calls to sco_sock_kill Date: Wed, 4 Aug 2021 19:03:08 +0800 Message-Id: <20210804110308.910744-7-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210804110308.910744-1-desmondcheongzx@gmail.com> References: <20210804110308.910744-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@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,