2022-09-03 15:43:25

by Tetsuo Handa

[permalink] [raw]
Subject: [PATCH] Bluetooth: L2CAP: initialize delayed works at l2cap_chan_create()

syzbot is reporting cancel_delayed_work() without INIT_DELAYED_WORK() at
l2cap_chan_del() [1], for CONF_NOT_COMPLETE flag (which meant to prevent
l2cap_chan_del() from calling cancel_delayed_work()) is cleared by timer
which fires before l2cap_chan_del() is called by closing file descriptor
created by socket(AF_BLUETOOTH, SOCK_STREAM, BTPROTO_L2CAP).

l2cap_bredr_sig_cmd(L2CAP_CONF_REQ) and l2cap_bredr_sig_cmd(L2CAP_CONF_RSP)
are calling l2cap_ertm_init(chan), and they call l2cap_chan_ready() (which
clears CONF_NOT_COMPLETE flag) only when l2cap_ertm_init(chan) succeeded.

l2cap_sock_init() does not call l2cap_ertm_init(chan), and it instead sets
CONF_NOT_COMPLETE flag by calling l2cap_chan_set_defaults(). However, when
connect() is requested, "command 0x0409 tx timeout" happens after 2 seconds
from connect() request, and CONF_NOT_COMPLETE flag is cleared after 4
seconds from connect() request, for l2cap_conn_start() from
l2cap_info_timeout() callback scheduled by

schedule_delayed_work(&conn->info_timer, L2CAP_INFO_TIMEOUT);

in l2cap_connect() is calling l2cap_chan_ready().

Fix this problem by initializing delayed works used by L2CAP_MODE_ERTM
mode as soon as l2cap_chan_create() allocates a channel, like I did in
commit be8597239379f0f5 ("Bluetooth: initialize skb_queue_head at
l2cap_chan_create()").

Link: https://syzkaller.appspot.com/bug?extid=83672956c7aa6af698b3 [1]
Reported-by: syzbot <[email protected]>
Signed-off-by: Tetsuo Handa <[email protected]>
---
net/bluetooth/l2cap_core.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 2c9de67daadc..770891f68703 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -61,6 +61,9 @@ static void l2cap_send_disconn_req(struct l2cap_chan *chan, int err);

static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
struct sk_buff_head *skbs, u8 event);
+static void l2cap_retrans_timeout(struct work_struct *work);
+static void l2cap_monitor_timeout(struct work_struct *work);
+static void l2cap_ack_timeout(struct work_struct *work);

static inline u8 bdaddr_type(u8 link_type, u8 bdaddr_type)
{
@@ -476,6 +479,9 @@ struct l2cap_chan *l2cap_chan_create(void)
write_unlock(&chan_list_lock);

INIT_DELAYED_WORK(&chan->chan_timer, l2cap_chan_timeout);
+ INIT_DELAYED_WORK(&chan->retrans_timer, l2cap_retrans_timeout);
+ INIT_DELAYED_WORK(&chan->monitor_timer, l2cap_monitor_timeout);
+ INIT_DELAYED_WORK(&chan->ack_timer, l2cap_ack_timeout);

chan->state = BT_OPEN;

@@ -3320,10 +3326,6 @@ int l2cap_ertm_init(struct l2cap_chan *chan)
chan->rx_state = L2CAP_RX_STATE_RECV;
chan->tx_state = L2CAP_TX_STATE_XMIT;

- INIT_DELAYED_WORK(&chan->retrans_timer, l2cap_retrans_timeout);
- INIT_DELAYED_WORK(&chan->monitor_timer, l2cap_monitor_timeout);
- INIT_DELAYED_WORK(&chan->ack_timer, l2cap_ack_timeout);
-
skb_queue_head_init(&chan->srej_q);

err = l2cap_seq_list_init(&chan->srej_list, chan->tx_win);
--
2.34.1


2022-09-03 16:13:56

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: L2CAP: initialize delayed works at l2cap_chan_create()

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=673830

---Test result---

Test Summary:
CheckPatch FAIL 1.92 seconds
GitLint PASS 0.77 seconds
SubjectPrefix PASS 0.62 seconds
BuildKernel PASS 46.71 seconds
BuildKernel32 PASS 41.98 seconds
Incremental Build with patchesPASS 60.74 seconds
TestRunner: Setup PASS 695.46 seconds
TestRunner: l2cap-tester PASS 21.77 seconds
TestRunner: iso-tester PASS 22.27 seconds
TestRunner: bnep-tester PASS 8.61 seconds
TestRunner: mgmt-tester PASS 136.49 seconds
TestRunner: rfcomm-tester PASS 12.60 seconds
TestRunner: sco-tester PASS 12.50 seconds
TestRunner: smp-tester PASS 12.46 seconds
TestRunner: userchan-tester PASS 9.00 seconds

Details
##############################
Test: CheckPatch - FAIL - 1.92 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
Bluetooth: L2CAP: initialize delayed works at l2cap_chan_create()\ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit fatal: unsaf ("ace/src' is owned by someone else)")'
#84:
commit be8597239379f0f5 ("Bluetooth: initialize skb_queue_head at
l2cap_chan_create()").

total: 1 errors, 0 warnings, 0 checks, 28 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12964990.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.




---
Regards,
Linux Bluetooth

2022-09-19 17:46:24

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: L2CAP: initialize delayed works at l2cap_chan_create()

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Sun, 4 Sep 2022 00:32:56 +0900 you wrote:
> syzbot is reporting cancel_delayed_work() without INIT_DELAYED_WORK() at
> l2cap_chan_del() [1], for CONF_NOT_COMPLETE flag (which meant to prevent
> l2cap_chan_del() from calling cancel_delayed_work()) is cleared by timer
> which fires before l2cap_chan_del() is called by closing file descriptor
> created by socket(AF_BLUETOOTH, SOCK_STREAM, BTPROTO_L2CAP).
>
> l2cap_bredr_sig_cmd(L2CAP_CONF_REQ) and l2cap_bredr_sig_cmd(L2CAP_CONF_RSP)
> are calling l2cap_ertm_init(chan), and they call l2cap_chan_ready() (which
> clears CONF_NOT_COMPLETE flag) only when l2cap_ertm_init(chan) succeeded.
>
> [...]

Here is the summary with links:
- Bluetooth: L2CAP: initialize delayed works at l2cap_chan_create()
https://git.kernel.org/bluetooth/bluetooth-next/c/2d2cb3066f2c

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html