From: Eric Dumazet <eric.dumazet@gmail.com>
To: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>,
Eric Dumazet <eric.dumazet@gmail.com>,
marcel@holtmann.org, johan.hedberg@gmail.com,
luiz.dentz@gmail.com, davem@davemloft.net, kuba@kernel.org,
sudipm.mukherjee@gmail.com
Cc: 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: Re: [PATCH v6 1/6] Bluetooth: schedule SCO timeouts with delayed_work
Date: Thu, 2 Sep 2021 14:41:21 -0700 [thread overview]
Message-ID: <94942257-927c-efbc-b3fd-44cc097ad71f@gmail.com> (raw)
In-Reply-To: <bad67d05-366b-bebe-cbdb-6555386497de@gmail.com>
On 9/2/21 12:32 PM, Desmond Cheong Zhi Xi wrote:
>
> Hi Eric,
>
> This actually seems to be a pre-existing error in sco_sock_connect that we now hit in sco_sock_timeout.
>
> Any thoughts on the following patch to address the problem?
>
> Link: https://lore.kernel.org/lkml/20210831065601.101185-1-desmondcheongzx@gmail.com/
syzbot is still working on finding a repro, this is obviously not trivial,
because this is a race window.
I think this can happen even with a single SCO connection.
This might be triggered more easily forcing a delay in sco_sock_timeout()
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 98a88158651281c9f75c4e0371044251e976e7ef..71ebe0243fab106c676c308724fe3a3f92a62cbd 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -84,8 +84,14 @@ static void sco_sock_timeout(struct work_struct *work)
sco_conn_lock(conn);
sk = conn->sk;
- if (sk)
+ if (sk) {
+ // lets pretend cpu has been busy (in interrupts) for 100ms
+ int i;
+ for (i=0;i<100000;i++)
+ udelay(1);
+
sock_hold(sk);
+ }
sco_conn_unlock(conn);
if (!sk)
Stack trace tells us that sco_sock_timeout() is running after last reference
on socket has been released.
__refcount_add include/linux/refcount.h:199 [inline]
__refcount_inc include/linux/refcount.h:250 [inline]
refcount_inc include/linux/refcount.h:267 [inline]
sock_hold include/net/sock.h:702 [inline]
sco_sock_timeout+0x216/0x290 net/bluetooth/sco.c:88
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
This is why I suggested to delay sock_put() to make sure this can not happen.
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 98a88158651281c9f75c4e0371044251e976e7ef..bd0222e3f05a6bcb40cffe8405c9dfff98d7afde 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -195,10 +195,11 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
sco_sock_clear_timer(sk);
sco_chan_del(sk, err);
release_sock(sk);
- sock_put(sk);
/* Ensure no more work items will run before freeing conn. */
cancel_delayed_work_sync(&conn->timeout_work);
+
+ sock_put(sk);
}
hcon->sco_data = NULL;
next prev parent reply other threads:[~2021-09-02 21:41 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-10 4:14 [PATCH v6 0/6] Bluetooth: fix locking and socket killing in SCO and RFCOMM Desmond Cheong Zhi Xi
2021-08-10 4:14 ` [PATCH v6 1/6] Bluetooth: schedule SCO timeouts with delayed_work Desmond Cheong Zhi Xi
2021-08-10 5:14 ` Bluetooth: fix locking and socket killing in SCO and RFCOMM bluez.test.bot
2021-08-10 17:51 ` Luiz Augusto von Dentz
2021-09-02 19:17 ` [PATCH v6 1/6] Bluetooth: schedule SCO timeouts with delayed_work Eric Dumazet
2021-09-02 19:32 ` Desmond Cheong Zhi Xi
2021-09-02 21:41 ` Eric Dumazet [this message]
2021-09-02 22:53 ` Desmond Cheong Zhi Xi
2021-09-02 23:05 ` Desmond Cheong Zhi Xi
2021-09-02 23:42 ` Luiz Augusto von Dentz
2021-09-03 3:17 ` Desmond Cheong Zhi Xi
2021-08-10 4:14 ` [PATCH v6 2/6] Bluetooth: avoid circular locks in sco_sock_connect Desmond Cheong Zhi Xi
2021-08-10 4:14 ` [PATCH v6 3/6] Bluetooth: switch to lock_sock in SCO Desmond Cheong Zhi Xi
2021-08-10 4:14 ` [PATCH v6 4/6] Bluetooth: serialize calls to sco_sock_{set,clear}_timer Desmond Cheong Zhi Xi
2021-08-10 4:14 ` [PATCH v6 5/6] Bluetooth: switch to lock_sock in RFCOMM Desmond Cheong Zhi Xi
2021-08-10 4:14 ` [PATCH v6 6/6] Bluetooth: fix repeated calls to sco_sock_kill Desmond Cheong Zhi Xi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=94942257-927c-efbc-b3fd-44cc097ad71f@gmail.com \
--to=eric.dumazet@gmail.com \
--cc=davem@davemloft.net \
--cc=desmondcheongzx@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=johan.hedberg@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-bluetooth@vger.kernel.org \
--cc=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luiz.dentz@gmail.com \
--cc=marcel@holtmann.org \
--cc=netdev@vger.kernel.org \
--cc=skhan@linuxfoundation.org \
--cc=sudipm.mukherjee@gmail.com \
--cc=syzbot+2f6d7c28bb4bf7e82060@syzkaller.appspotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).