Skip to content

Commit d58df2f

Browse files
swkim101mehmetb0
swkim101
authored and
mehmetb0
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 <luiz.dentz@gmail.com> Signed-off-by: Sungwoo Kim <iam@sung-woo.kim> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> CVE-2024-36968 (backported from commit a5b862c) [juergh: Adjusted context plus: - Add #define HCI_ERROR_INVALID_PARAMETERS to include/net/bluetooth/hci.h, from: c86cc5a ("Bluetooth: hci_event: Fix checking for invalid handle on error status") - Drop modifications of net/bluetooth/iso.c and ISO_LINK (not supported in 5.15) - Change return type of hci_cc_read_buffer_size() and hci_cc_le_read_buffer_size() from void to u8 and return status, from: c8992cf ("Bluetooth: hci_event: Use of a function table to handle Command Complete") - Drop various hunks due to missing functionality in 5.15] Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com> Acked-by: ivanhu <ivan.hu@canonical.com> Acked-by: Guoqing Jiang <guoqing.jiang@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
1 parent 355b8ad commit d58df2f

File tree

6 files changed

+77
-41
lines changed

6 files changed

+77
-41
lines changed

‎include/net/bluetooth/hci.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,7 @@ enum {
565565
#define HCI_ERROR_CONNECTION_TIMEOUT 0x08
566566
#define HCI_ERROR_REJ_LIMITED_RESOURCES 0x0d
567567
#define HCI_ERROR_REJ_BAD_ADDR 0x0f
568+
#define HCI_ERROR_INVALID_PARAMETERS 0x12
568569
#define HCI_ERROR_REMOTE_USER_TERM 0x13
569570
#define HCI_ERROR_REMOTE_LOW_RESOURCES 0x14
570571
#define HCI_ERROR_REMOTE_POWER_OFF 0x15
@@ -1441,6 +1442,15 @@ struct hci_cp_le_set_event_mask {
14411442
__u8 mask[8];
14421443
} __packed;
14431444

1445+
/* BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 4, Part E
1446+
* 7.8.2 LE Read Buffer Size command
1447+
* MAX_LE_MTU is 0xffff.
1448+
* 0 is also valid. It means that no dedicated LE Buffer exists.
1449+
* It should use the HCI_Read_Buffer_Size command and mtu is shared
1450+
* between BR/EDR and LE.
1451+
*/
1452+
#define HCI_MIN_LE_MTU 0x001b
1453+
14441454
#define HCI_OP_LE_READ_BUFFER_SIZE 0x2002
14451455
struct hci_rp_le_read_buffer_size {
14461456
__u8 status;

‎include/net/bluetooth/hci_core.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,7 @@ struct hci_conn {
635635
__u8 adv_instance;
636636
__u16 handle;
637637
__u16 state;
638+
__u16 mtu;
638639
__u8 mode;
639640
__u8 type;
640641
__u8 role;

‎net/bluetooth/hci_conn.c

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -553,11 +553,32 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
553553
{
554554
struct hci_conn *conn;
555555

556+
switch (type) {
557+
case ACL_LINK:
558+
if (!hdev->acl_mtu)
559+
return ERR_PTR(-ECONNREFUSED);
560+
break;
561+
case LE_LINK:
562+
if (hdev->le_mtu && hdev->le_mtu < HCI_MIN_LE_MTU)
563+
return ERR_PTR(-ECONNREFUSED);
564+
if (!hdev->le_mtu && hdev->acl_mtu < HCI_MIN_LE_MTU)
565+
return ERR_PTR(-ECONNREFUSED);
566+
break;
567+
case SCO_LINK:
568+
case ESCO_LINK:
569+
if (!hdev->sco_pkts)
570+
/* Controller does not support SCO or eSCO over HCI */
571+
return ERR_PTR(-ECONNREFUSED);
572+
break;
573+
default:
574+
return ERR_PTR(-ECONNREFUSED);
575+
}
576+
556577
BT_DBG("%s dst %pMR", hdev->name, dst);
557578

558579
conn = kzalloc(sizeof(*conn), GFP_KERNEL);
559580
if (!conn)
560-
return NULL;
581+
return ERR_PTR(-ENOMEM);
561582

562583
bacpy(&conn->dst, dst);
563584
bacpy(&conn->src, &hdev->bdaddr);
@@ -586,20 +607,25 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
586607
switch (type) {
587608
case ACL_LINK:
588609
conn->pkt_type = hdev->pkt_type & ACL_PTYPE_MASK;
610+
conn->mtu = hdev->acl_mtu;
589611
break;
590612
case LE_LINK:
591613
/* conn->src should reflect the local identity address */
592614
hci_copy_identity_address(hdev, &conn->src, &conn->src_type);
615+
conn->mtu = hdev->le_mtu ? hdev->le_mtu : hdev->acl_mtu;
593616
break;
594617
case SCO_LINK:
595618
if (lmp_esco_capable(hdev))
596619
conn->pkt_type = (hdev->esco_type & SCO_ESCO_MASK) |
597620
(hdev->esco_type & EDR_ESCO_MASK);
598621
else
599622
conn->pkt_type = hdev->pkt_type & SCO_PTYPE_MASK;
623+
624+
conn->mtu = hdev->sco_mtu;
600625
break;
601626
case ESCO_LINK:
602627
conn->pkt_type = hdev->esco_type & ~EDR_ESCO_MASK;
628+
conn->mtu = hdev->sco_mtu;
603629
break;
604630
}
605631

@@ -1097,8 +1123,8 @@ struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
10971123
bacpy(&conn->dst, dst);
10981124
} else {
10991125
conn = hci_conn_add(hdev, LE_LINK, dst, role);
1100-
if (!conn)
1101-
return ERR_PTR(-ENOMEM);
1126+
if (IS_ERR(conn))
1127+
return conn;
11021128
hci_conn_hold(conn);
11031129
conn->pending_sec_level = sec_level;
11041130
}
@@ -1262,8 +1288,8 @@ struct hci_conn *hci_connect_le_scan(struct hci_dev *hdev, bdaddr_t *dst,
12621288
BT_DBG("requesting refresh of dst_addr");
12631289

12641290
conn = hci_conn_add(hdev, LE_LINK, dst, HCI_ROLE_MASTER);
1265-
if (!conn)
1266-
return ERR_PTR(-ENOMEM);
1291+
if (IS_ERR(conn))
1292+
return conn;
12671293

12681294
if (hci_explicit_conn_params_set(hdev, dst, dst_type) < 0) {
12691295
hci_conn_del(conn);
@@ -1310,8 +1336,8 @@ struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
13101336
acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst);
13111337
if (!acl) {
13121338
acl = hci_conn_add(hdev, ACL_LINK, dst, HCI_ROLE_MASTER);
1313-
if (!acl)
1314-
return ERR_PTR(-ENOMEM);
1339+
if (IS_ERR(acl))
1340+
return acl;
13151341
}
13161342

13171343
hci_conn_hold(acl);
@@ -1341,9 +1367,9 @@ struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
13411367
sco = hci_conn_hash_lookup_ba(hdev, type, dst);
13421368
if (!sco) {
13431369
sco = hci_conn_add(hdev, type, dst, HCI_ROLE_MASTER);
1344-
if (!sco) {
1370+
if (IS_ERR(sco)) {
13451371
hci_conn_drop(acl);
1346-
return ERR_PTR(-ENOMEM);
1372+
return sco;
13471373
}
13481374
}
13491375

‎net/bluetooth/hci_event.c

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -747,14 +747,14 @@ static void hci_cc_read_flow_control_mode(struct hci_dev *hdev,
747747
hdev->flow_ctl_mode = rp->mode;
748748
}
749749

750-
static void hci_cc_read_buffer_size(struct hci_dev *hdev, struct sk_buff *skb)
750+
static u8 hci_cc_read_buffer_size(struct hci_dev *hdev, struct sk_buff *skb)
751751
{
752752
struct hci_rp_read_buffer_size *rp = (void *) skb->data;
753753

754754
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
755755

756756
if (rp->status)
757-
return;
757+
return rp->status;
758758

759759
hdev->acl_mtu = __le16_to_cpu(rp->acl_mtu);
760760
hdev->sco_mtu = rp->sco_mtu;
@@ -771,6 +771,11 @@ static void hci_cc_read_buffer_size(struct hci_dev *hdev, struct sk_buff *skb)
771771

772772
BT_DBG("%s acl mtu %d:%d sco mtu %d:%d", hdev->name, hdev->acl_mtu,
773773
hdev->acl_pkts, hdev->sco_mtu, hdev->sco_pkts);
774+
775+
if (!hdev->acl_mtu || !hdev->acl_pkts)
776+
return HCI_ERROR_INVALID_PARAMETERS;
777+
778+
return rp->status;
774779
}
775780

776781
static void hci_cc_read_bd_addr(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1035,22 +1040,27 @@ static void hci_cc_pin_code_neg_reply(struct hci_dev *hdev, struct sk_buff *skb)
10351040
hci_dev_unlock(hdev);
10361041
}
10371042

1038-
static void hci_cc_le_read_buffer_size(struct hci_dev *hdev,
1039-
struct sk_buff *skb)
1043+
static u8 hci_cc_le_read_buffer_size(struct hci_dev *hdev,
1044+
struct sk_buff *skb)
10401045
{
10411046
struct hci_rp_le_read_buffer_size *rp = (void *) skb->data;
10421047

10431048
BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
10441049

10451050
if (rp->status)
1046-
return;
1051+
return rp->status;
10471052

10481053
hdev->le_mtu = __le16_to_cpu(rp->le_mtu);
10491054
hdev->le_pkts = rp->le_max_pkt;
10501055

10511056
hdev->le_cnt = hdev->le_pkts;
10521057

10531058
BT_DBG("%s le mtu %d:%d", hdev->name, hdev->le_mtu, hdev->le_pkts);
1059+
1060+
if (hdev->le_mtu && hdev->le_mtu < HCI_MIN_LE_MTU)
1061+
return HCI_ERROR_INVALID_PARAMETERS;
1062+
1063+
return rp->status;
10541064
}
10551065

10561066
static void hci_cc_le_read_local_features(struct hci_dev *hdev,
@@ -1960,8 +1970,8 @@ static void hci_cs_create_conn(struct hci_dev *hdev, __u8 status)
19601970
if (!conn) {
19611971
conn = hci_conn_add(hdev, ACL_LINK, &cp->bdaddr,
19621972
HCI_ROLE_MASTER);
1963-
if (!conn)
1964-
bt_dev_err(hdev, "no memory for new connection");
1973+
if (IS_ERR(conn))
1974+
bt_dev_err(hdev, "connection err: %ld", PTR_ERR(conn));
19651975
}
19661976
}
19671977

@@ -2688,8 +2698,8 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
26882698
BDADDR_BREDR)) {
26892699
conn = hci_conn_add(hdev, ev->link_type, &ev->bdaddr,
26902700
HCI_ROLE_SLAVE);
2691-
if (!conn) {
2692-
bt_dev_err(hdev, "no memory for new conn");
2701+
if (IS_ERR(conn)) {
2702+
bt_dev_err(hdev, "connection err: %ld", PTR_ERR(conn));
26932703
goto unlock;
26942704
}
26952705
} else {
@@ -2871,8 +2881,8 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
28712881
if (!conn) {
28722882
conn = hci_conn_add(hdev, ev->link_type, &ev->bdaddr,
28732883
HCI_ROLE_SLAVE);
2874-
if (!conn) {
2875-
bt_dev_err(hdev, "no memory for new connection");
2884+
if (IS_ERR(conn)) {
2885+
bt_dev_err(hdev, "connection err: %ld", PTR_ERR(conn));
28762886
goto unlock;
28772887
}
28782888
}
@@ -3527,7 +3537,7 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
35273537
break;
35283538

35293539
case HCI_OP_READ_BUFFER_SIZE:
3530-
hci_cc_read_buffer_size(hdev, skb);
3540+
*status = hci_cc_read_buffer_size(hdev, skb);
35313541
break;
35323542

35333543
case HCI_OP_READ_BD_ADDR:
@@ -3599,7 +3609,7 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
35993609
break;
36003610

36013611
case HCI_OP_LE_READ_BUFFER_SIZE:
3602-
hci_cc_le_read_buffer_size(hdev, skb);
3612+
*status = hci_cc_le_read_buffer_size(hdev, skb);
36033613
break;
36043614

36053615
case HCI_OP_LE_READ_LOCAL_FEATURES:
@@ -5323,8 +5333,8 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
53235333
conn = hci_lookup_le_connect(hdev);
53245334
if (!conn) {
53255335
conn = hci_conn_add(hdev, LE_LINK, bdaddr, role);
5326-
if (!conn) {
5327-
bt_dev_err(hdev, "no memory for new connection");
5336+
if (IS_ERR(conn)) {
5337+
bt_dev_err(hdev, "connection err: %ld", PTR_ERR(conn));
53285338
goto unlock;
53295339
}
53305340

‎net/bluetooth/l2cap_core.c

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7247,7 +7247,7 @@ static int l2cap_finish_move(struct l2cap_chan *chan)
72477247
if (chan->hs_hcon)
72487248
chan->conn->mtu = chan->hs_hcon->hdev->block_mtu;
72497249
else
7250-
chan->conn->mtu = chan->conn->hcon->hdev->acl_mtu;
7250+
chan->conn->mtu = chan->conn->hcon->mtu;
72517251

72527252
return l2cap_resegment(chan);
72537253
}
@@ -7318,7 +7318,7 @@ static int l2cap_rx_state_wait_f(struct l2cap_chan *chan,
73187318
if (chan->hs_hcon)
73197319
chan->conn->mtu = chan->hs_hcon->hdev->block_mtu;
73207320
else
7321-
chan->conn->mtu = chan->conn->hcon->hdev->acl_mtu;
7321+
chan->conn->mtu = chan->conn->hcon->mtu;
73227322

73237323
err = l2cap_resegment(chan);
73247324

@@ -7870,18 +7870,7 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
78707870

78717871
BT_DBG("hcon %p conn %p hchan %p", hcon, conn, hchan);
78727872

7873-
switch (hcon->type) {
7874-
case LE_LINK:
7875-
if (hcon->hdev->le_mtu) {
7876-
conn->mtu = hcon->hdev->le_mtu;
7877-
break;
7878-
}
7879-
fallthrough;
7880-
default:
7881-
conn->mtu = hcon->hdev->acl_mtu;
7882-
break;
7883-
}
7884-
7873+
conn->mtu = hcon->mtu;
78857874
conn->feat_mask = 0;
78867875

78877876
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)
@@ -141,9 +140,10 @@ static struct sco_conn *sco_conn_add(struct hci_conn *hcon)
141140

142141
hcon->sco_data = conn;
143142
conn->hcon = hcon;
143+
conn->mtu = hcon->mtu;
144144

145-
if (hdev->sco_mtu > 0)
146-
conn->mtu = hdev->sco_mtu;
145+
if (hcon->mtu > 0)
146+
conn->mtu = hcon->mtu;
147147
else
148148
conn->mtu = 60;
149149

0 commit comments

Comments
 (0)