Skip to content

Commit a5b862c

Browse files
swkim101Vudentz
swkim101
authored and
Vudentz
committed
Bluetooth: L2CAP: Fix div-by-zero in l2cap_le_flowctl_init()
l2cap_le_flowctl_init() can cause both div-by-zero and an integer overflow since hdev->le_mtu may not fall in the valid range. Move MTU from hci_dev to hci_conn to validate MTU and stop the connection process earlier if MTU is invalid. Also, add a missing validation in read_buffer_size() and make it return an error value if the validation fails. Now hci_conn_add() returns ERR_PTR() as it can fail due to the both a kzalloc failure and invalid MTU value. divide error: 0000 [#1] PREEMPT SMP KASAN NOPTI CPU: 0 PID: 67 Comm: kworker/u5:0 Tainted: G W 6.9.0-rc5+ #20 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 Workqueue: hci0 hci_rx_work RIP: 0010:l2cap_le_flowctl_init+0x19e/0x3f0 net/bluetooth/l2cap_core.c:547 Code: e8 17 17 0c 00 66 41 89 9f 84 00 00 00 bf 01 00 00 00 41 b8 02 00 00 00 4c 89 fe 4c 89 e2 89 d9 e8 27 17 0c 00 44 89 f0 31 d2 <66> f7 f3 89 c3 ff c3 4d 8d b7 88 00 00 00 4c 89 f0 48 c1 e8 03 42 RSP: 0018:ffff88810bc0f858 EFLAGS: 00010246 RAX: 00000000000002a0 RBX: 0000000000000000 RCX: dffffc0000000000 RDX: 0000000000000000 RSI: ffff88810bc0f7c0 RDI: ffffc90002dcb66f RBP: ffff88810bc0f880 R08: aa69db2dda70ff01 R09: 0000ffaaaaaaaaaa R10: 0084000000ffaaaa R11: 0000000000000000 R12: ffff88810d65a084 R13: dffffc0000000000 R14: 00000000000002a0 R15: ffff88810d65a000 FS: 0000000000000000(0000) GS:ffff88811ac00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000020000100 CR3: 0000000103268003 CR4: 0000000000770ef0 PKRU: 55555554 Call Trace: <TASK> l2cap_le_connect_req net/bluetooth/l2cap_core.c:4902 [inline] l2cap_le_sig_cmd net/bluetooth/l2cap_core.c:5420 [inline] l2cap_le_sig_channel net/bluetooth/l2cap_core.c:5486 [inline] l2cap_recv_frame+0xe59d/0x11710 net/bluetooth/l2cap_core.c:6809 l2cap_recv_acldata+0x544/0x10a0 net/bluetooth/l2cap_core.c:7506 hci_acldata_packet net/bluetooth/hci_core.c:3939 [inline] hci_rx_work+0x5e5/0xb20 net/bluetooth/hci_core.c:4176 process_one_work kernel/workqueue.c:3254 [inline] process_scheduled_works+0x90f/0x1530 kernel/workqueue.c:3335 worker_thread+0x926/0xe70 kernel/workqueue.c:3416 kthread+0x2e3/0x380 kernel/kthread.c:388 ret_from_fork+0x5c/0x90 arch/x86/kernel/process.c:147 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244 </TASK> Modules linked in: ---[ end trace 0000000000000000 ]--- Fixes: 6ed58ec ("Bluetooth: Use LE buffers for LE traffic") Suggested-by: Luiz Augusto von Dentz <[email protected]> Signed-off-by: Sungwoo Kim <[email protected]> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
1 parent a189f0e commit a5b862c

File tree

7 files changed

+88
-53
lines changed

7 files changed

+88
-53
lines changed

‎include/net/bluetooth/hci.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1665,6 +1665,15 @@ struct hci_cp_le_set_event_mask {
16651665
__u8 mask[8];
16661666
} __packed;
16671667

1668+
/* BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 4, Part E
1669+
* 7.8.2 LE Read Buffer Size command
1670+
* MAX_LE_MTU is 0xffff.
1671+
* 0 is also valid. It means that no dedicated LE Buffer exists.
1672+
* It should use the HCI_Read_Buffer_Size command and mtu is shared
1673+
* between BR/EDR and LE.
1674+
*/
1675+
#define HCI_MIN_LE_MTU 0x001b
1676+
16681677
#define HCI_OP_LE_READ_BUFFER_SIZE 0x2002
16691678
struct hci_rp_le_read_buffer_size {
16701679
__u8 status;

‎include/net/bluetooth/hci_core.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,7 @@ struct hci_conn {
706706
__u16 handle;
707707
__u16 sync_handle;
708708
__u16 state;
709+
__u16 mtu;
709710
__u8 mode;
710711
__u8 type;
711712
__u8 role;

‎net/bluetooth/hci_conn.c

Lines changed: 51 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -904,11 +904,37 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
904904
{
905905
struct hci_conn *conn;
906906

907+
switch (type) {
908+
case ACL_LINK:
909+
if (!hdev->acl_mtu)
910+
return ERR_PTR(-ECONNREFUSED);
911+
break;
912+
case ISO_LINK:
913+
if (hdev->iso_mtu)
914+
/* Dedicated ISO Buffer exists */
915+
break;
916+
fallthrough;
917+
case LE_LINK:
918+
if (hdev->le_mtu && hdev->le_mtu < HCI_MIN_LE_MTU)
919+
return ERR_PTR(-ECONNREFUSED);
920+
if (!hdev->le_mtu && hdev->acl_mtu < HCI_MIN_LE_MTU)
921+
return ERR_PTR(-ECONNREFUSED);
922+
break;
923+
case SCO_LINK:
924+
case ESCO_LINK:
925+
if (!hdev->sco_pkts)
926+
/* Controller does not support SCO or eSCO over HCI */
927+
return ERR_PTR(-ECONNREFUSED);
928+
break;
929+
default:
930+
return ERR_PTR(-ECONNREFUSED);
931+
}
932+
907933
bt_dev_dbg(hdev, "dst %pMR handle 0x%4.4x", dst, handle);
908934

909935
conn = kzalloc(sizeof(*conn), GFP_KERNEL);
910936
if (!conn)
911-
return NULL;
937+
return ERR_PTR(-ENOMEM);
912938

913939
bacpy(&conn->dst, dst);
914940
bacpy(&conn->src, &hdev->bdaddr);
@@ -939,10 +965,12 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
939965
switch (type) {
940966
case ACL_LINK:
941967
conn->pkt_type = hdev->pkt_type & ACL_PTYPE_MASK;
968+
conn->mtu = hdev->acl_mtu;
942969
break;
943970
case LE_LINK:
944971
/* conn->src should reflect the local identity address */
945972
hci_copy_identity_address(hdev, &conn->src, &conn->src_type);
973+
conn->mtu = hdev->le_mtu ? hdev->le_mtu : hdev->acl_mtu;
946974
break;
947975
case ISO_LINK:
948976
/* conn->src should reflect the local identity address */
@@ -954,16 +982,21 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
954982
else if (conn->role == HCI_ROLE_MASTER)
955983
conn->cleanup = cis_cleanup;
956984

985+
conn->mtu = hdev->iso_mtu ? hdev->iso_mtu :
986+
hdev->le_mtu ? hdev->le_mtu : hdev->acl_mtu;
957987
break;
958988
case SCO_LINK:
959989
if (lmp_esco_capable(hdev))
960990
conn->pkt_type = (hdev->esco_type & SCO_ESCO_MASK) |
961991
(hdev->esco_type & EDR_ESCO_MASK);
962992
else
963993
conn->pkt_type = hdev->pkt_type & SCO_PTYPE_MASK;
994+
995+
conn->mtu = hdev->sco_mtu;
964996
break;
965997
case ESCO_LINK:
966998
conn->pkt_type = hdev->esco_type & ~EDR_ESCO_MASK;
999+
conn->mtu = hdev->sco_mtu;
9671000
break;
9681001
}
9691002

@@ -1006,7 +1039,7 @@ struct hci_conn *hci_conn_add_unset(struct hci_dev *hdev, int type,
10061039

10071040
handle = hci_conn_hash_alloc_unset(hdev);
10081041
if (unlikely(handle < 0))
1009-
return NULL;
1042+
return ERR_PTR(-ECONNREFUSED);
10101043

10111044
return hci_conn_add(hdev, type, dst, role, handle);
10121045
}
@@ -1312,8 +1345,8 @@ struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
13121345
bacpy(&conn->dst, dst);
13131346
} else {
13141347
conn = hci_conn_add_unset(hdev, LE_LINK, dst, role);
1315-
if (!conn)
1316-
return ERR_PTR(-ENOMEM);
1348+
if (IS_ERR(conn))
1349+
return conn;
13171350
hci_conn_hold(conn);
13181351
conn->pending_sec_level = sec_level;
13191352
}
@@ -1489,8 +1522,8 @@ static struct hci_conn *hci_add_bis(struct hci_dev *hdev, bdaddr_t *dst,
14891522
return ERR_PTR(-EADDRINUSE);
14901523

14911524
conn = hci_conn_add_unset(hdev, ISO_LINK, dst, HCI_ROLE_MASTER);
1492-
if (!conn)
1493-
return ERR_PTR(-ENOMEM);
1525+
if (IS_ERR(conn))
1526+
return conn;
14941527

14951528
conn->state = BT_CONNECT;
14961529

@@ -1533,8 +1566,8 @@ struct hci_conn *hci_connect_le_scan(struct hci_dev *hdev, bdaddr_t *dst,
15331566
BT_DBG("requesting refresh of dst_addr");
15341567

15351568
conn = hci_conn_add_unset(hdev, LE_LINK, dst, HCI_ROLE_MASTER);
1536-
if (!conn)
1537-
return ERR_PTR(-ENOMEM);
1569+
if (IS_ERR(conn))
1570+
return conn;
15381571

15391572
if (hci_explicit_conn_params_set(hdev, dst, dst_type) < 0) {
15401573
hci_conn_del(conn);
@@ -1581,8 +1614,8 @@ struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
15811614
acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst);
15821615
if (!acl) {
15831616
acl = hci_conn_add_unset(hdev, ACL_LINK, dst, HCI_ROLE_MASTER);
1584-
if (!acl)
1585-
return ERR_PTR(-ENOMEM);
1617+
if (IS_ERR(acl))
1618+
return acl;
15861619
}
15871620

15881621
hci_conn_hold(acl);
@@ -1650,9 +1683,9 @@ struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
16501683
sco = hci_conn_hash_lookup_ba(hdev, type, dst);
16511684
if (!sco) {
16521685
sco = hci_conn_add_unset(hdev, type, dst, HCI_ROLE_MASTER);
1653-
if (!sco) {
1686+
if (IS_ERR(sco)) {
16541687
hci_conn_drop(acl);
1655-
return ERR_PTR(-ENOMEM);
1688+
return sco;
16561689
}
16571690
}
16581691

@@ -1841,8 +1874,8 @@ struct hci_conn *hci_bind_cis(struct hci_dev *hdev, bdaddr_t *dst,
18411874
qos->ucast.cis);
18421875
if (!cis) {
18431876
cis = hci_conn_add_unset(hdev, ISO_LINK, dst, HCI_ROLE_MASTER);
1844-
if (!cis)
1845-
return ERR_PTR(-ENOMEM);
1877+
if (IS_ERR(cis))
1878+
return cis;
18461879
cis->cleanup = cis_cleanup;
18471880
cis->dst_type = dst_type;
18481881
cis->iso_qos.ucast.cig = BT_ISO_QOS_CIG_UNSET;
@@ -1977,14 +2010,8 @@ static void hci_iso_qos_setup(struct hci_dev *hdev, struct hci_conn *conn,
19772010
struct bt_iso_io_qos *qos, __u8 phy)
19782011
{
19792012
/* Only set MTU if PHY is enabled */
1980-
if (!qos->sdu && qos->phy) {
1981-
if (hdev->iso_mtu > 0)
1982-
qos->sdu = hdev->iso_mtu;
1983-
else if (hdev->le_mtu > 0)
1984-
qos->sdu = hdev->le_mtu;
1985-
else
1986-
qos->sdu = hdev->acl_mtu;
1987-
}
2013+
if (!qos->sdu && qos->phy)
2014+
qos->sdu = conn->mtu;
19882015

19892016
/* Use the same PHY as ACL if set to any */
19902017
if (qos->phy == BT_ISO_PHY_ANY)
@@ -2065,8 +2092,8 @@ struct hci_conn *hci_pa_create_sync(struct hci_dev *hdev, bdaddr_t *dst,
20652092
return ERR_PTR(-EBUSY);
20662093

20672094
conn = hci_conn_add_unset(hdev, ISO_LINK, dst, HCI_ROLE_SLAVE);
2068-
if (!conn)
2069-
return ERR_PTR(-ENOMEM);
2095+
if (IS_ERR(conn))
2096+
return conn;
20702097

20712098
conn->iso_qos = *qos;
20722099
conn->state = BT_LISTEN;

‎net/bluetooth/hci_event.c

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -954,6 +954,9 @@ static u8 hci_cc_read_buffer_size(struct hci_dev *hdev, void *data,
954954
BT_DBG("%s acl mtu %d:%d sco mtu %d:%d", hdev->name, hdev->acl_mtu,
955955
hdev->acl_pkts, hdev->sco_mtu, hdev->sco_pkts);
956956

957+
if (!hdev->acl_mtu || !hdev->acl_pkts)
958+
return HCI_ERROR_INVALID_PARAMETERS;
959+
957960
return rp->status;
958961
}
959962

@@ -1263,6 +1266,9 @@ static u8 hci_cc_le_read_buffer_size(struct hci_dev *hdev, void *data,
12631266

12641267
BT_DBG("%s le mtu %d:%d", hdev->name, hdev->le_mtu, hdev->le_pkts);
12651268

1269+
if (hdev->le_mtu && hdev->le_mtu < HCI_MIN_LE_MTU)
1270+
return HCI_ERROR_INVALID_PARAMETERS;
1271+
12661272
return rp->status;
12671273
}
12681274

@@ -2341,8 +2347,8 @@ static void hci_cs_create_conn(struct hci_dev *hdev, __u8 status)
23412347
if (!conn) {
23422348
conn = hci_conn_add_unset(hdev, ACL_LINK, &cp->bdaddr,
23432349
HCI_ROLE_MASTER);
2344-
if (!conn)
2345-
bt_dev_err(hdev, "no memory for new connection");
2350+
if (IS_ERR(conn))
2351+
bt_dev_err(hdev, "connection err: %ld", PTR_ERR(conn));
23462352
}
23472353
}
23482354

@@ -3153,8 +3159,8 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
31533159
BDADDR_BREDR)) {
31543160
conn = hci_conn_add_unset(hdev, ev->link_type,
31553161
&ev->bdaddr, HCI_ROLE_SLAVE);
3156-
if (!conn) {
3157-
bt_dev_err(hdev, "no memory for new conn");
3162+
if (IS_ERR(conn)) {
3163+
bt_dev_err(hdev, "connection err: %ld", PTR_ERR(conn));
31583164
goto unlock;
31593165
}
31603166
} else {
@@ -3342,8 +3348,8 @@ static void hci_conn_request_evt(struct hci_dev *hdev, void *data,
33423348
if (!conn) {
33433349
conn = hci_conn_add_unset(hdev, ev->link_type, &ev->bdaddr,
33443350
HCI_ROLE_SLAVE);
3345-
if (!conn) {
3346-
bt_dev_err(hdev, "no memory for new connection");
3351+
if (IS_ERR(conn)) {
3352+
bt_dev_err(hdev, "connection err: %ld", PTR_ERR(conn));
33473353
goto unlock;
33483354
}
33493355
}
@@ -3820,6 +3826,9 @@ static u8 hci_cc_le_read_buffer_size_v2(struct hci_dev *hdev, void *data,
38203826
BT_DBG("%s acl mtu %d:%d iso mtu %d:%d", hdev->name, hdev->acl_mtu,
38213827
hdev->acl_pkts, hdev->iso_mtu, hdev->iso_pkts);
38223828

3829+
if (hdev->le_mtu && hdev->le_mtu < HCI_MIN_LE_MTU)
3830+
return HCI_ERROR_INVALID_PARAMETERS;
3831+
38233832
return rp->status;
38243833
}
38253834

@@ -5768,8 +5777,8 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
57685777
goto unlock;
57695778

57705779
conn = hci_conn_add_unset(hdev, LE_LINK, bdaddr, role);
5771-
if (!conn) {
5772-
bt_dev_err(hdev, "no memory for new connection");
5780+
if (IS_ERR(conn)) {
5781+
bt_dev_err(hdev, "connection err: %ld", PTR_ERR(conn));
57735782
goto unlock;
57745783
}
57755784

@@ -6497,7 +6506,7 @@ static void hci_le_pa_sync_estabilished_evt(struct hci_dev *hdev, void *data,
64976506
pa_sync = hci_conn_add_unset(hdev, ISO_LINK, BDADDR_ANY,
64986507
HCI_ROLE_SLAVE);
64996508

6500-
if (!pa_sync)
6509+
if (IS_ERR(pa_sync))
65016510
goto unlock;
65026511

65036512
pa_sync->sync_handle = le16_to_cpu(ev->handle);
@@ -6921,7 +6930,7 @@ static void hci_le_cis_req_evt(struct hci_dev *hdev, void *data,
69216930
if (!cis) {
69226931
cis = hci_conn_add(hdev, ISO_LINK, &acl->dst, HCI_ROLE_SLAVE,
69236932
cis_handle);
6924-
if (!cis) {
6933+
if (IS_ERR(cis)) {
69256934
hci_le_reject_cis(hdev, ev->cis_handle);
69266935
goto unlock;
69276936
}
@@ -7030,7 +7039,7 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
70307039
if (!bis) {
70317040
bis = hci_conn_add(hdev, ISO_LINK, BDADDR_ANY,
70327041
HCI_ROLE_SLAVE, handle);
7033-
if (!bis)
7042+
if (IS_ERR(bis))
70347043
continue;
70357044
}
70367045

‎net/bluetooth/iso.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1258,7 +1258,7 @@ static int iso_sock_sendmsg(struct socket *sock, struct msghdr *msg,
12581258
return -ENOTCONN;
12591259
}
12601260

1261-
mtu = iso_pi(sk)->conn->hcon->hdev->iso_mtu;
1261+
mtu = iso_pi(sk)->conn->hcon->mtu;
12621262

12631263
release_sock(sk);
12641264

‎net/bluetooth/l2cap_core.c

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6264,7 +6264,7 @@ static int l2cap_finish_move(struct l2cap_chan *chan)
62646264
BT_DBG("chan %p", chan);
62656265

62666266
chan->rx_state = L2CAP_RX_STATE_RECV;
6267-
chan->conn->mtu = chan->conn->hcon->hdev->acl_mtu;
6267+
chan->conn->mtu = chan->conn->hcon->mtu;
62686268

62696269
return l2cap_resegment(chan);
62706270
}
@@ -6331,7 +6331,7 @@ static int l2cap_rx_state_wait_f(struct l2cap_chan *chan,
63316331
*/
63326332
chan->next_tx_seq = control->reqseq;
63336333
chan->unacked_frames = 0;
6334-
chan->conn->mtu = chan->conn->hcon->hdev->acl_mtu;
6334+
chan->conn->mtu = chan->conn->hcon->mtu;
63356335

63366336
err = l2cap_resegment(chan);
63376337

@@ -6889,18 +6889,7 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
68896889

68906890
BT_DBG("hcon %p conn %p hchan %p", hcon, conn, hchan);
68916891

6892-
switch (hcon->type) {
6893-
case LE_LINK:
6894-
if (hcon->hdev->le_mtu) {
6895-
conn->mtu = hcon->hdev->le_mtu;
6896-
break;
6897-
}
6898-
fallthrough;
6899-
default:
6900-
conn->mtu = hcon->hdev->acl_mtu;
6901-
break;
6902-
}
6903-
6892+
conn->mtu = hcon->mtu;
69046893
conn->feat_mask = 0;
69056894

69066895
conn->local_fixed_chan = L2CAP_FC_SIG_BREDR | L2CAP_FC_CONNLESS;

‎net/bluetooth/sco.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ static void sco_sock_clear_timer(struct sock *sk)
126126
/* ---- SCO connections ---- */
127127
static struct sco_conn *sco_conn_add(struct hci_conn *hcon)
128128
{
129-
struct hci_dev *hdev = hcon->hdev;
130129
struct sco_conn *conn = hcon->sco_data;
131130

132131
if (conn) {
@@ -144,9 +143,10 @@ static struct sco_conn *sco_conn_add(struct hci_conn *hcon)
144143

145144
hcon->sco_data = conn;
146145
conn->hcon = hcon;
146+
conn->mtu = hcon->mtu;
147147

148-
if (hdev->sco_mtu > 0)
149-
conn->mtu = hdev->sco_mtu;
148+
if (hcon->mtu > 0)
149+
conn->mtu = hcon->mtu;
150150
else
151151
conn->mtu = 60;
152152

0 commit comments

Comments
 (0)