From patchwork Tue Aug 10 04:14: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: 12427829 X-Patchwork-Delegate: luiz.dentz@gmail.com 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,URIBL_BLOCKED,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 A8608C4338F for ; Tue, 10 Aug 2021 04:17:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 88C9B60FC4 for ; Tue, 10 Aug 2021 04:17:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237013AbhHJER6 (ORCPT ); Tue, 10 Aug 2021 00:17:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35020 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237043AbhHJERx (ORCPT ); Tue, 10 Aug 2021 00:17:53 -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 A72B5C06179B; Mon, 9 Aug 2021 21:17:09 -0700 (PDT) Received: by mail-pj1-x102d.google.com with SMTP id mq2-20020a17090b3802b0290178911d298bso3438205pjb.1; Mon, 09 Aug 2021 21:17: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=fV8kQqJajr7zTckOLiIM8CrHI+Va9cP1Jx+NUuT6fI0=; b=eP/qla48apaA1R94v+6gcKfSIyaZimnSLcowdpUvfMR45LGoG5ErDj+egtOXZ1Qs0z j3FMRzGPxr2WSdU3lhDKAPiRQKbZvsjdV7uvfKxWDPhTHCvnFdJp920JX3P6dYszKNWw sd2tHa+UdiT7oy30OCWhrv5DX2G7p1uLlzmDwhXZYwo9sj+XhYhpk5DkuoISNmhrn8Ml G2oNg9xwP5V5dqCZX2UlcFSLOWluIIs3EGueUNJ/oKbIJdYjursXQ5NxMnEZBNsCedQD JiJobZpi9fGsY3p9zNQJFwcPcDzOf/EhPzquv6GvJTnxnuTMtHcQ6nwptZeQq8TstMsl SzjQ== 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=fV8kQqJajr7zTckOLiIM8CrHI+Va9cP1Jx+NUuT6fI0=; b=H5neX3WcEgXYyyVJsZ2rWmLokY9dWng/4IRnjqOjyxjhdSauzh+OsrAR5QBEFV81FD 6Gt+wJPtbS3WbBs5veq9g2Y3mFBOzBbEEV+XlbDv3JUUQ4jC635Xj/p+fwiQyUUFnf+m JgFqtL2VohTXfxlyDTaV02py6bhAPcUywAv1rOrpztL7jVJmh9ZxavFlwYIRDDYZiOfT O6H8xZMdHaUGVpWnAexSGQdxKhC1B0u/zxnFxt3TAcdiWrTTM7o/DJrQ/FQ13gx3oXZ2 jBCZ3gVlWn9MKk5MdlpdsdswzZo5sy18XAtKrGJ2FrkG2fGKYuP/Vm7Lww7tWNKomgkn +XFw== X-Gm-Message-State: AOAM531Cak/2A/q1mYocYdxA1QVd7HjENp+8I6rfeT6LE5hICmmlDqwY qmeyWR+K8+D6weaI78aB08M= X-Google-Smtp-Source: ABdhPJwXY+fVVUnOHCCeh5QEtimaI4OGRAMzhFaG/6FZAzJdf42FsIHD6enk+Rf4bPtxZhY021/McA== X-Received: by 2002:a17:902:c711:b029:12c:9b3c:9986 with SMTP id p17-20020a170902c711b029012c9b3c9986mr12553881plp.44.1628569029261; Mon, 09 Aug 2021 21:17:09 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id b8sm20132478pjo.51.2021.08.09.21.17.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Aug 2021 21:17: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 , 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: [PATCH v6 1/6] Bluetooth: schedule SCO timeouts with delayed_work Date: Tue, 10 Aug 2021 12:14:05 +0800 Message-Id: <20210810041410.142035-2-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210810041410.142035-1-desmondcheongzx@gmail.com> References: <20210810041410.142035-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 | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index ffa2a77a3e4c..62e638f971a9 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,21 @@ static void sco_sock_timeout(struct timer_list *t) static void sco_sock_set_timer(struct sock *sk, long timeout) { + if (!sco_pi(sk)->conn) + return; + 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(&sco_pi(sk)->conn->timeout_work); + schedule_delayed_work(&sco_pi(sk)->conn->timeout_work, timeout); } static void sco_sock_clear_timer(struct sock *sk) { + if (!sco_pi(sk)->conn) + return; + BT_DBG("sock %p state %d", sk, sk->sk_state); - sk_stop_timer(sk, &sk->sk_timer); + cancel_delayed_work(&sco_pi(sk)->conn->timeout_work); } /* ---- SCO connections ---- */ @@ -179,6 +199,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 +216,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 +525,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 Tue Aug 10 04:14: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: 12427831 X-Patchwork-Delegate: luiz.dentz@gmail.com 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 38539C4320A for ; Tue, 10 Aug 2021 04:17:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0D3B96054E for ; Tue, 10 Aug 2021 04:17:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237032AbhHJER7 (ORCPT ); Tue, 10 Aug 2021 00:17:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35026 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237221AbhHJERx (ORCPT ); Tue, 10 Aug 2021 00:17:53 -0400 Received: from mail-pj1-x1033.google.com (mail-pj1-x1033.google.com [IPv6:2607:f8b0:4864:20::1033]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2ED2EC06179F; Mon, 9 Aug 2021 21:17:15 -0700 (PDT) Received: by mail-pj1-x1033.google.com with SMTP id u13-20020a17090abb0db0290177e1d9b3f7so2484140pjr.1; Mon, 09 Aug 2021 21:17:15 -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=IWvKIG9SD/bGbMF6shXMTXm63IsfR0+DR+eSAYUvZ40=; b=D/69BGj8l3P7BWe+EGBQ4OFutdUxFPT8lRu7PwUOeXU6qKLRr7DGVNWmBwNxv3upiZ LjN3wCsQ/NS6E90qkfO3maKKchN9M8G64GJ1lekF7o+F6Hmog43xo8IYsOAWHwhMBX5M rl22iCKV1MLKp4iODeAgSihMCGPn0tchAlbg5VH54e5DbmjvYKD54UyYD1yjEDGQhOVj XvuNYWrbG5lGen5TvhrJiInRWpA46yvsiGHCtqDC1eZhmvd++hkJeRDXYZfBXPWHJQ5t 8P4cXZiGdoQhGAp9dfR65XLucb4iq0ud5cvHNDoJDfpKdCaht9Yx/FBCk2jj6t1ts5y1 LmCQ== 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=IWvKIG9SD/bGbMF6shXMTXm63IsfR0+DR+eSAYUvZ40=; b=OTBtx1Y1sPlifHX0ZqTVwP30iCQfqZn6YN5Bd1T8p/vxSS+RexVSmEwTh644n8qIqt F3hMXBTyF6Y87bBsjWvBVnQ4o1ZnbaqINswKUNNB+0j0MFK3MhyGK4s+C7+bKVx4j4Xj phLq5cFf3BQKJJeE2x7ILlp9CTmeudnnRvuZ9QDI8845pJSthyyqWprNUyA3qups3sH+ U8UD1mTsCBX1Fk6TTTv4vC3rhMLcINRLP0n2bjGGRu2kosIvO3i650iMmrEieF+Oi/v5 kiIqlMuPL0ktPL/ZEyH/C4dAF4BsqZgJa6RTjf6T8HEQbgzlSZrDVaMkAdwbKrK6GUtA jrZw== X-Gm-Message-State: AOAM532K0M9o82Q9+JxrB54aizazDls+o8sC1jzEinPzVYIvDeKossX4 VVs2UTVSrzKCxGcIypN176E= X-Google-Smtp-Source: ABdhPJwWMjha1++IWnizQ3wORteBy41K6YR7ISLDtCundlo8hcKTEmp9gNDou4phQXwUWDiXHi0IeQ== X-Received: by 2002:a63:1504:: with SMTP id v4mr39208pgl.151.1628569034753; Mon, 09 Aug 2021 21:17:14 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id b8sm20132478pjo.51.2021.08.09.21.17.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Aug 2021 21:17:14 -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: [PATCH v6 2/6] Bluetooth: avoid circular locks in sco_sock_connect Date: Tue, 10 Aug 2021 12:14:06 +0800 Message-Id: <20210810041410.142035-3-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210810041410.142035-1-desmondcheongzx@gmail.com> References: <20210810041410.142035-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 62e638f971a9..94a3aa686556 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -237,44 +237,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 */ @@ -282,7 +270,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); @@ -292,9 +280,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; } @@ -589,6 +574,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); @@ -603,12 +589,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 Tue Aug 10 04:14: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: 12427835 X-Patchwork-Delegate: luiz.dentz@gmail.com 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 9DB0DC4320E for ; Tue, 10 Aug 2021 04:17:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 79AC360FC4 for ; Tue, 10 Aug 2021 04:17:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237354AbhHJESB (ORCPT ); Tue, 10 Aug 2021 00:18:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35040 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237231AbhHJER4 (ORCPT ); Tue, 10 Aug 2021 00:17:56 -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 B9D64C0617A0; Mon, 9 Aug 2021 21:17:19 -0700 (PDT) Received: by mail-pj1-x1030.google.com with SMTP id u13-20020a17090abb0db0290177e1d9b3f7so2484346pjr.1; Mon, 09 Aug 2021 21:17:19 -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=PIWCbgM5Czh7o4d4UfSqkTS5fg/4+RhME2M18GpOuag=; b=pwTexxRixX5EhmzkxTh7FMxGNvqV9ze34Qr4Bzn5NPPY6WnQRULpEnoir2DXVPpOxx 6p+jOxunC6evidzVWTlVIcBcuRZNI8S2HWVo+bguvhlEQratAVPWIzj+KmLvqSrriqRG BMMdbiivUWz6RpLUp2RbojwpAugGFaUfOYbN533tcYnHY0GtnP1PHDaOlcAuurZbl9S9 B0GDLf3tbs9KjVEL7LNnG/7WghpodWi/v6cnCfJCra5X30Emz8PyqHBCXo6u+GlkkoU3 bp01ClAEqNzTLcXL1eiQbTqYIwZ3A7T9FM4Hp792CX0/7neCTWArq087lhcAnzkzdmgP JFvQ== 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=PIWCbgM5Czh7o4d4UfSqkTS5fg/4+RhME2M18GpOuag=; b=aFTK4u8s65uuhkDRj5+MsZ5lAyi8xrBzrAV+H3u857taG1jvh0DzZ4o6MaKISA2DH7 +jGPwS1UTZUlu5S6oj61kdJsFQPB9MPAyV5sWmbJ6LPjxP4YyjDN3R/KtOK+FBnlCEzk Rfe8YhVxxeZ4j77s71/VTPO/v4ZFllkjHUEmyKYM/ifBt8rLMklwvzcpySKO5k+CKu5S annmNo23jquhe9Mz38lsdBxORlNbwwCNKOiY1jfBEgFHX7zzUiLVb4ma/5Lt9wJynvJk wCvEHN0i+TCVUL9OtEROVkeCb8XWDypfAbxxe5akG+vxjvSj+X1B0DR57OO4/HSJiFF9 Uc1Q== X-Gm-Message-State: AOAM532Z3RphCQ7NSWd7OjYSlNLmLx+O54s5k5ql89Xyd52JOcvfsjri pwkt2xiEevF5Rw0qiBz9Fgc= X-Google-Smtp-Source: ABdhPJy81yXzWOUbPLBKMsCXP3TKxPGR2kEyBsjPiXG8i5OT0sgiNBZ7/dun7wF8qqDyV1S3rjlrzg== X-Received: by 2002:a17:902:e890:b029:12c:d39d:20bb with SMTP id w16-20020a170902e890b029012cd39d20bbmr23680005plg.83.1628569039325; Mon, 09 Aug 2021 21:17:19 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id b8sm20132478pjo.51.2021.08.09.21.17.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Aug 2021 21:17:18 -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: [PATCH v6 3/6] Bluetooth: switch to lock_sock in SCO Date: Tue, 10 Aug 2021 12:14:07 +0800 Message-Id: <20210810041410.142035-4-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210810041410.142035-1-desmondcheongzx@gmail.com> References: <20210810041410.142035-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 94a3aa686556..68b51e321e82 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); @@ -193,10 +193,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); @@ -1105,10 +1105,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); @@ -1123,12 +1123,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; } @@ -1149,7 +1149,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 Tue Aug 10 04:14: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: 12427833 X-Patchwork-Delegate: luiz.dentz@gmail.com 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 68824C4338F for ; Tue, 10 Aug 2021 04:17:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5197D6054E for ; Tue, 10 Aug 2021 04:17:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237207AbhHJESA (ORCPT ); Tue, 10 Aug 2021 00:18:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35042 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237226AbhHJER4 (ORCPT ); Tue, 10 Aug 2021 00:17:56 -0400 Received: from mail-pl1-x636.google.com (mail-pl1-x636.google.com [IPv6:2607:f8b0:4864:20::636]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 443B9C0617A1; Mon, 9 Aug 2021 21:17:24 -0700 (PDT) Received: by mail-pl1-x636.google.com with SMTP id l11so7605646plk.6; Mon, 09 Aug 2021 21:17:24 -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=SvgciiztrVef8p8zryOlzKMWIbOXm14bnkz3tLt2Ce0=; b=BUnnRx+X4v+1TL2k/V2JSTPEo9aZKCrlBiUXDBNMmj2qULjq+uWnxD26UxR3lMTexU Y1IbO4KeN1RWknPDtprm+kuFAkvtHuy+Rg2EZWtEeeX92R2IW8V/D6ihkMeZaKE4CqQM rm/pVNEGvdwzleQzKqxFhcO9U2aBMac2Ej6S7ejapBsZiG/BOsAKi5S+QIplBduWFvYO zTo++1I0l9Lcin+KeB25+mLD24fuiBwOk5k9G7jO9dRJ3Hy/R/9HfvPscihKBhpdAQy2 6chz9ohNsU/noQUo4hIqjIgaynaQheYVolooSqy5Jsnifoced07RTeUbq4xcSgcwhXAn KeVA== 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=SvgciiztrVef8p8zryOlzKMWIbOXm14bnkz3tLt2Ce0=; b=gV5FvNTr+pg2aE1EccgbfgEciS2PS4oD8jiSYXf9n7QKLc60wjryTds+DEldxwdwml eNDDWkOUcB9X22o/lm/wl1EAa150ss1s0Rt1ofT6Voe/drXKDgNyHmbGU5X/yuXPDGRH gsSOKHIU2SNQrb3V66gfMGeD1k+KZINm9LA0KGmtYOAw7DTypIToFgHb6Fgv3n4/GOu1 FCD4g7pBmcsOV7SSPQKuk46LgxtRx/wQoIArt5p+Y63V8LSvRrrF6Y97DAKxG/BDMnhT prhIDMHECW5/ulVMt7ETDXEWI9IC4CwcnSk+jcU4+JE1xF9tiQFbXLzhc+yELaruUROo ua7Q== X-Gm-Message-State: AOAM530svx9+OEFBKiOR5stKtNXrgTJuiF+1wxDpPut8q690yqfZZWJd rNfRf0vR8BumUy1azWvLjhw= X-Google-Smtp-Source: ABdhPJyST7kdXNemvcmeYieJUDOCfvnVJRXrwbmK/xqHiFmHBNjJ96w9TdavrCr2KZ6DrKZpwpEcxg== X-Received: by 2002:a65:6910:: with SMTP id s16mr37421pgq.270.1628569043859; Mon, 09 Aug 2021 21:17:23 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id b8sm20132478pjo.51.2021.08.09.21.17.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Aug 2021 21:17:23 -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: [PATCH v6 4/6] Bluetooth: serialize calls to sco_sock_{set,clear}_timer Date: Tue, 10 Aug 2021 12:14:08 +0800 Message-Id: <20210810041410.142035-5-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210810041410.142035-1-desmondcheongzx@gmail.com> References: <20210810041410.142035-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 68b51e321e82..77490338f4fa 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -453,8 +453,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); @@ -1104,8 +1104,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 Tue Aug 10 04:14: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: 12427837 X-Patchwork-Delegate: luiz.dentz@gmail.com 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 E051AC4320A for ; Tue, 10 Aug 2021 04:17:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BF7DE6054E for ; Tue, 10 Aug 2021 04:17:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237275AbhHJESC (ORCPT ); Tue, 10 Aug 2021 00:18:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35044 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237239AbhHJER4 (ORCPT ); Tue, 10 Aug 2021 00:17:56 -0400 Received: from mail-pj1-x1031.google.com (mail-pj1-x1031.google.com [IPv6:2607:f8b0:4864:20::1031]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A092CC0617A2; Mon, 9 Aug 2021 21:17:28 -0700 (PDT) Received: by mail-pj1-x1031.google.com with SMTP id nt11so8077384pjb.2; Mon, 09 Aug 2021 21:17: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=UwpAuyr31gkKRzWsz22ieldVxq1Gat0ivFQ8uoQ/1c0=; b=lq2jd3xIHLY37an7XRk+QppmVqTj4iNl7/RA67QNjQEKC+/uBuVqX14eOxEI5zEPFa R2zink3dMmRn6ybKqDCTzCt04j3i7lChM0tXIj2vqUKf5ZdmHPgyRdwpwHJvfWgRcvLL ctg9uozZPXx9+7DBQFZs6Iuog3Szc4d6wMYZnLBKO0C2Av1eMz3TJ2bXa1RJAr12/mrf 5nTgz0rrDIhELW4X2T4Xjtu6hmhnzqRT4CFE0dNMtL75rGicYwDW3yNkNOr3WIgh6rvh eAA0FBfdGJwyLrRJtEyD6WtrXCzEiEJ8bLFG1Y7IkyuurbXSuD8p7dEcD372IOg10VOd 0L3Q== 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=UBZVuwuQnbLOi144eFXUFWcyoNu6+SrE+8EI6NFWoGzGqWVR3E4wH+/such6TAsp0L LNhUQW82xb+zJL0Z88gsZkJRu6JLaIEcDk7FUWHOE2VCLeMH9kA3UHs3R96rFphL5ey4 7syD/IAfaWNIe8NjBKcTJ+1qK1vQUtq2pQP46pVFvkjMGkDOWE4eLF9T6S/IrPMElvlG YNabCfs1K/QOXILCe17OhTYm9SB4Z7kOH6ZODANW99TXdWugxE60VxcZiDGGJKqDO/TM 5eE24otIDBz+c84t8gyKTWLqEeDJ0tZOpoPS5+eEf725drtYr7KQK/vU6UDW/YXhT2fa FmJg== X-Gm-Message-State: AOAM533YRdyM4Ggibl4IDZGnuLt545LOGfnWKZxdkA9AHm552XKQNBQD tap/POYUaju5Lth+puWT1Rg= X-Google-Smtp-Source: ABdhPJwfVFJuhXJPcZXKtGefX5s0S/tksX1yVjiMp1lHAxU9y1q4ypqE+xB6yKezuwfNPblMowqt5w== X-Received: by 2002:a17:90b:3653:: with SMTP id nh19mr2695305pjb.169.1628569048241; Mon, 09 Aug 2021 21:17:28 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id b8sm20132478pjo.51.2021.08.09.21.17.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Aug 2021 21:17:27 -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: [PATCH v6 5/6] Bluetooth: switch to lock_sock in RFCOMM Date: Tue, 10 Aug 2021 12:14:09 +0800 Message-Id: <20210810041410.142035-6-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210810041410.142035-1-desmondcheongzx@gmail.com> References: <20210810041410.142035-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 Tue Aug 10 04:14: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: 12427839 X-Patchwork-Delegate: luiz.dentz@gmail.com 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 97D67C4338F for ; Tue, 10 Aug 2021 04:17:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 723736054E for ; Tue, 10 Aug 2021 04:17:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237399AbhHJESE (ORCPT ); Tue, 10 Aug 2021 00:18:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35046 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237242AbhHJER4 (ORCPT ); Tue, 10 Aug 2021 00:17:56 -0400 Received: from mail-pl1-x62f.google.com (mail-pl1-x62f.google.com [IPv6:2607:f8b0:4864:20::62f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 350CEC0613D3; Mon, 9 Aug 2021 21:17:33 -0700 (PDT) Received: by mail-pl1-x62f.google.com with SMTP id d17so19210145plr.12; Mon, 09 Aug 2021 21:17:33 -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=OIcoax9PQR0R3OnS5s26gNkuo62hz7EbqgNWPiTkgQM=; b=lAH95ClMQCGVsVezjaAKFYn2KOwKLL38WigaMEuHcmdwKJrXLdhdlfBf+6mNuxQv2D G5+p2a0jm3H+rBVWMN5ojVgcHbBG87vq+KGdk/R9FUVvQNKcAXw8PauG+f+8pnhMTiHw uxjHra7w5+S39KfkMVk8Bw5/Cte+uKXNLg5c67WAy6OBxK92uOHn7QgfxNgYgvlLpUA5 S4qEXAWVpWQFwIOaj5iKii0jGzQJeEu7v44R7B9cofNeG8RNuinzGld7YNqQqGRvG7XD UVNetqQlFPfbe+voiQZRr/gEkXHH7JTdYuW5u/J0YaxcRo2RCdeMYvyryjG8qdNprQbh FXyw== 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=OIcoax9PQR0R3OnS5s26gNkuo62hz7EbqgNWPiTkgQM=; b=G00zuEMTMf3Hln5Mt+lQ+JnXiiiM6TZG2qQ/urLlDo3YtS8zTsMK+o9fIDoksuVN/M Ppx/yOJck3mKnyCtFgEiOOxtko7u4rO5zaqdYH/I1jFdx9eUZ22xnLs4LMPvsvQPz9XW QzcaDQNPVMpzykOTDNgq8XVV5rZq+9KQG+5gzFGnQ5xOn2SRztvsrC+BAAFHBW1Fcx4j osiW3tcld5kTzyMWsLuqSRpF6+oM622B35v6zABNJigdijCmuXrqmg6LQixa9ntf+pA7 dMkKf/O+zDrxoozAqO0GeU1sBx7bmKNM7rTFt5B/6K7gJteg6gE/6hQOSWmefe6fEiXN cAPQ== X-Gm-Message-State: AOAM530lueWELQbu8TCacKpik8pUOd7f4DgCvDl76ZGrV5McFCdDiWP1 VJBphrgSRGjvPcJKmqcuQx4= X-Google-Smtp-Source: ABdhPJwfTlSiybFidKyx8OGCgRRGpma1nh/Zbq4L8DCNiIjmWbxmyUIYmQeEEzzwGsDYHxZXCKVe9Q== X-Received: by 2002:a63:8ac2:: with SMTP id y185mr159072pgd.179.1628569052608; Mon, 09 Aug 2021 21:17:32 -0700 (PDT) Received: from localhost.localdomain ([118.200.190.93]) by smtp.gmail.com with ESMTPSA id b8sm20132478pjo.51.2021.08.09.21.17.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Aug 2021 21:17:32 -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: [PATCH v6 6/6] Bluetooth: fix repeated calls to sco_sock_kill Date: Tue, 10 Aug 2021 12:14:10 +0800 Message-Id: <20210810041410.142035-7-desmondcheongzx@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210810041410.142035-1-desmondcheongzx@gmail.com> References: <20210810041410.142035-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 77490338f4fa..98a881586512 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); } @@ -197,7 +195,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. */ @@ -404,8 +401,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); @@ -457,7 +453,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,