Luiz Augusto von Dentz says: ==================== bluetooth pull request for net: - Fixes various issues related to ISO channel/socket support - Fixes issues when building with C=1 - Fix cancel uninitilized work which blocks syzbot to run * tag 'for-net-2022-08-08' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth: Bluetooth: ISO: Fix not using the correct QoS Bluetooth: don't try to cancel uninitialized works at mgmt_index_removed() Bluetooth: ISO: Fix iso_sock_getsockopt for BT_DEFER_SETUP Bluetooth: MGMT: Fixes build warnings with C=1 Bluetooth: hci_event: Fix build warning with C=1 Bluetooth: ISO: Fix memory corruption Bluetooth: Fix null pointer deref on unexpected status event Bluetooth: ISO: Fix info leak in iso_sock_getsockopt() Bluetooth: hci_conn: Fix updating ISO QoS PHY Bluetooth: ISO: unlock on error path in iso_sock_setsockopt() Bluetooth: L2CAP: Fix l2cap_global_chan_by_psm regression ====================
Unified · Split
| @@ -54,7 +54,10 @@ void aosp_do_open(struct hci_dev *hdev) · 53 unmodified lines | ||
| 54 | 54 | /* LE Get Vendor Capabilities Command */ |
| 55 | 55 | skb = __hci_cmd_sync(hdev, hci_opcode_pack(0x3f, 0x153), 0, NULL, |
| 56 | 56 | HCI_CMD_TIMEOUT); |
| 57 | − if (IS_ERR(skb)) { | |
| 57 | + if (IS_ERR_OR_NULL(skb)) { | |
| 58 | + if (!skb) | |
| 59 | + skb = ERR_PTR(-EIO); | |
| 60 | + | |
| 58 | 61 | bt_dev_err(hdev, "AOSP get vendor capabilities (%ld)", |
| 59 | 62 | PTR_ERR(skb)); |
| 60 | 63 | return; |
| @@ -152,7 +155,10 @@ static int enable_quality_report(struct hci_dev *hdev) · 91 unmodified lines | ||
| 152 | 155 | |
| 153 | 156 | skb = __hci_cmd_sync(hdev, BQR_OPCODE, sizeof(cp), &cp, |
| 154 | 157 | HCI_CMD_TIMEOUT); |
| 155 | − if (IS_ERR(skb)) { | |
| 158 | + if (IS_ERR_OR_NULL(skb)) { | |
| 159 | + if (!skb) | |
| 160 | + skb = ERR_PTR(-EIO); | |
| 161 | + | |
| 156 | 162 | bt_dev_err(hdev, "Enabling Android BQR failed (%ld)", |
| 157 | 163 | PTR_ERR(skb)); |
| 158 | 164 | return PTR_ERR(skb); |
| @@ -171,7 +177,10 @@ static int disable_quality_report(struct hci_dev *hdev) · 12 unmodified lines | ||
| 171 | 177 | |
| 172 | 178 | skb = __hci_cmd_sync(hdev, BQR_OPCODE, sizeof(cp), &cp, |
| 173 | 179 | HCI_CMD_TIMEOUT); |
| 174 | − if (IS_ERR(skb)) { | |
| 180 | + if (IS_ERR_OR_NULL(skb)) { | |
| 181 | + if (!skb) | |
| 182 | + skb = ERR_PTR(-EIO); | |
| 183 | + | |
| 175 | 184 | bt_dev_err(hdev, "Disabling Android BQR failed (%ld)", |
| 176 | 185 | PTR_ERR(skb)); |
| 177 | 186 | return PTR_ERR(skb); |
| @@ -1551,8 +1551,8 @@ static void cis_add(struct iso_list_data *d, struct bt_iso_qos *qos) · 1550 unmodified lines | ||
| 1551 | 1551 | cis->cis_id = qos->cis; |
| 1552 | 1552 | cis->c_sdu = cpu_to_le16(qos->out.sdu); |
| 1553 | 1553 | cis->p_sdu = cpu_to_le16(qos->in.sdu); |
| 1554 | − cis->c_phy = qos->out.phy; | |
| 1555 | − cis->p_phy = qos->in.phy; | |
| 1554 | + cis->c_phy = qos->out.phy ? qos->out.phy : qos->in.phy; | |
| 1555 | + cis->p_phy = qos->in.phy ? qos->in.phy : qos->out.phy; | |
| 1556 | 1556 | cis->c_rtn = qos->out.rtn; |
| 1557 | 1557 | cis->p_rtn = qos->in.rtn; |
| 1558 | 1558 | |
| @@ -1735,13 +1735,6 @@ struct hci_conn *hci_bind_cis(struct hci_dev *hdev, bdaddr_t *dst, · 176 unmodified lines | ||
| 1735 | 1735 | if (!qos->in.latency) |
| 1736 | 1736 | qos->in.latency = qos->out.latency; |
| 1737 | 1737 | |
| 1738 | − /* Mirror PHYs that are disabled as SDU will be set to 0 */ | |
| 1739 | − if (!qos->in.phy) | |
| 1740 | − qos->in.phy = qos->out.phy; | |
| 1741 | − | |
| 1742 | − if (!qos->out.phy) | |
| 1743 | − qos->out.phy = qos->in.phy; | |
| 1744 | − | |
| 1745 | 1738 | if (!hci_le_set_cig_params(cis, qos)) { |
| 1746 | 1739 | hci_conn_drop(cis); |
| 1747 | 1740 | return ERR_PTR(-EINVAL); |
| @@ -328,14 +328,17 @@ static u8 hci_cc_delete_stored_link_key(struct hci_dev *hdev, void *data, · 327 unmodified lines | ||
| 328 | 328 | struct sk_buff *skb) |
| 329 | 329 | { |
| 330 | 330 | struct hci_rp_delete_stored_link_key *rp = data; |
| 331 | + u16 num_keys; | |
| 331 | 332 | |
| 332 | 333 | bt_dev_dbg(hdev, "status 0x%2.2x", rp->status); |
| 333 | 334 | |
| 334 | 335 | if (rp->status) |
| 335 | 336 | return rp->status; |
| 336 | 337 | |
| 337 | − if (rp->num_keys <= hdev->stored_num_keys) | |
| 338 | − hdev->stored_num_keys -= le16_to_cpu(rp->num_keys); | |
| 338 | + num_keys = le16_to_cpu(rp->num_keys); | |
| 339 | + | |
| 340 | + if (num_keys <= hdev->stored_num_keys) | |
| 341 | + hdev->stored_num_keys -= num_keys; | |
| 339 | 342 | else |
| 340 | 343 | hdev->stored_num_keys = 0; |
| 341 | 344 | |
| @@ -44,6 +44,9 @@ static void iso_sock_kill(struct sock *sk); · 43 unmodified lines | ||
| 44 | 44 | /* ----- ISO socket info ----- */ |
| 45 | 45 | #define iso_pi(sk) ((struct iso_pinfo *)sk) |
| 46 | 46 | |
| 47 | +#define EIR_SERVICE_DATA_LENGTH 4 | |
| 48 | +#define BASE_MAX_LENGTH (HCI_MAX_PER_AD_LENGTH - EIR_SERVICE_DATA_LENGTH) | |
| 49 | + | |
| 47 | 50 | struct iso_pinfo { |
| 48 | 51 | struct bt_sock bt; |
| 49 | 52 | bdaddr_t src; |
| @@ -57,7 +60,7 @@ struct iso_pinfo { · 7 unmodified lines | ||
| 57 | 60 | __u32 flags; |
| 58 | 61 | struct bt_iso_qos qos; |
| 59 | 62 | __u8 base_len; |
| 60 | − __u8 base[HCI_MAX_PER_AD_LENGTH]; | |
| 63 | + __u8 base[BASE_MAX_LENGTH]; | |
| 61 | 64 | struct iso_conn *conn; |
| 62 | 65 | }; |
| 63 | 66 | |
| @@ -370,15 +373,24 @@ done: · 306 unmodified lines | ||
| 370 | 373 | return err; |
| 371 | 374 | } |
| 372 | 375 | |
| 376 | +static struct bt_iso_qos *iso_sock_get_qos(struct sock *sk) | |
| 377 | +{ | |
| 378 | + if (sk->sk_state == BT_CONNECTED || sk->sk_state == BT_CONNECT2) | |
| 379 | + return &iso_pi(sk)->conn->hcon->iso_qos; | |
| 380 | + | |
| 381 | + return &iso_pi(sk)->qos; | |
| 382 | +} | |
| 383 | + | |
| 373 | 384 | static int iso_send_frame(struct sock *sk, struct sk_buff *skb) |
| 374 | 385 | { |
| 375 | 386 | struct iso_conn *conn = iso_pi(sk)->conn; |
| 387 | + struct bt_iso_qos *qos = iso_sock_get_qos(sk); | |
| 376 | 388 | struct hci_iso_data_hdr *hdr; |
| 377 | 389 | int len = 0; |
| 378 | 390 | |
| 379 | 391 | BT_DBG("sk %p len %d", sk, skb->len); |
| 380 | 392 | |
| 381 | − if (skb->len > iso_pi(sk)->qos.out.sdu) | |
| 393 | + if (skb->len > qos->out.sdu) | |
| 382 | 394 | return -EMSGSIZE; |
| 383 | 395 | |
| 384 | 396 | len = skb->len; |
| @@ -1177,8 +1189,10 @@ static int iso_sock_setsockopt(struct socket *sock, int level, int optname, · 792 unmodified lines | ||
| 1177 | 1189 | } |
| 1178 | 1190 | |
| 1179 | 1191 | len = min_t(unsigned int, sizeof(qos), optlen); |
| 1180 | − if (len != sizeof(qos)) | |
| 1181 | − return -EINVAL; | |
| 1192 | + if (len != sizeof(qos)) { | |
| 1193 | + err = -EINVAL; | |
| 1194 | + break; | |
| 1195 | + } | |
| 1182 | 1196 | |
| 1183 | 1197 | memset(&qos, 0, sizeof(qos)); |
| 1184 | 1198 | |
| @@ -1233,7 +1247,7 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname, · 48 unmodified lines | ||
| 1233 | 1247 | { |
| 1234 | 1248 | struct sock *sk = sock->sk; |
| 1235 | 1249 | int len, err = 0; |
| 1236 | − struct bt_iso_qos qos; | |
| 1250 | + struct bt_iso_qos *qos; | |
| 1237 | 1251 | u8 base_len; |
| 1238 | 1252 | u8 *base; |
| 1239 | 1253 | |
| @@ -1246,7 +1260,7 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname, · 6 unmodified lines | ||
| 1246 | 1260 | |
| 1247 | 1261 | switch (optname) { |
| 1248 | 1262 | case BT_DEFER_SETUP: |
| 1249 | − if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) { | |
| 1263 | + if (sk->sk_state == BT_CONNECTED) { | |
| 1250 | 1264 | err = -EINVAL; |
| 1251 | 1265 | break; |
| 1252 | 1266 | } |
| @@ -1258,13 +1272,10 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname, · 5 unmodified lines | ||
| 1258 | 1272 | break; |
| 1259 | 1273 | |
| 1260 | 1274 | case BT_ISO_QOS: |
| 1261 | − if (sk->sk_state == BT_CONNECTED || sk->sk_state == BT_CONNECT2) | |
| 1262 | − qos = iso_pi(sk)->conn->hcon->iso_qos; | |
| 1263 | − else | |
| 1264 | − qos = iso_pi(sk)->qos; | |
| 1275 | + qos = iso_sock_get_qos(sk); | |
| 1265 | 1276 | |
| 1266 | − len = min_t(unsigned int, len, sizeof(qos)); | |
| 1267 | − if (copy_to_user(optval, (char *)&qos, len)) | |
| 1277 | + len = min_t(unsigned int, len, sizeof(*qos)); | |
| 1278 | + if (copy_to_user(optval, qos, len)) | |
| 1268 | 1279 | err = -EFAULT; |
| 1269 | 1280 | |
| 1270 | 1281 | break; |
| @@ -1970,11 +1970,11 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm, · 1969 unmodified lines | ||
| 1970 | 1970 | bdaddr_t *dst, |
| 1971 | 1971 | u8 link_type) |
| 1972 | 1972 | { |
| 1973 | − struct l2cap_chan *c, *c1 = NULL; | |
| 1973 | + struct l2cap_chan *c, *tmp, *c1 = NULL; | |
| 1974 | 1974 | |
| 1975 | 1975 | read_lock(&chan_list_lock); |
| 1976 | 1976 | |
| 1977 | − list_for_each_entry(c, &chan_list, global_l) { | |
| 1977 | + list_for_each_entry_safe(c, tmp, &chan_list, global_l) { | |
| 1978 | 1978 | if (state && c->state != state) |
| 1979 | 1979 | continue; |
| 1980 | 1980 | |
| @@ -1993,11 +1993,10 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm, · 12 unmodified lines | ||
| 1993 | 1993 | dst_match = !bacmp(&c->dst, dst); |
| 1994 | 1994 | if (src_match && dst_match) { |
| 1995 | 1995 | c = l2cap_chan_hold_unless_zero(c); |
| 1996 | − if (!c) | |
| 1997 | − continue; | |
| 1998 | − | |
| 1999 | − read_unlock(&chan_list_lock); | |
| 2000 | − return c; | |
| 1996 | + if (c) { | |
| 1997 | + read_unlock(&chan_list_lock); | |
| 1998 | + return c; | |
| 1999 | + } | |
| 2001 | 2000 | } |
| 2002 | 2001 | |
| 2003 | 2002 | /* Closest match */ |
| @@ -3819,7 +3819,7 @@ static int set_blocked_keys(struct sock *sk, struct hci_dev *hdev, void *data, · 3818 unmodified lines | ||
| 3819 | 3819 | |
| 3820 | 3820 | hci_blocked_keys_clear(hdev); |
| 3821 | 3821 | |
| 3822 | − for (i = 0; i < keys->key_count; ++i) { | |
| 3822 | + for (i = 0; i < key_count; ++i) { | |
| 3823 | 3823 | struct blocked_key *b = kzalloc(sizeof(*b), GFP_KERNEL); |
| 3824 | 3824 | |
| 3825 | 3825 | if (!b) { |
| @@ -4624,8 +4624,7 @@ static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data, · 798 unmodified lines | ||
| 4624 | 4624 | u32 current_flags = __le32_to_cpu(cp->current_flags); |
| 4625 | 4625 | |
| 4626 | 4626 | bt_dev_dbg(hdev, "Set device flags %pMR (type 0x%x) = 0x%x", |
| 4627 | − &cp->addr.bdaddr, cp->addr.type, | |
| 4628 | − __le32_to_cpu(current_flags)); | |
| 4627 | + &cp->addr.bdaddr, cp->addr.type, current_flags); | |
| 4629 | 4628 | |
| 4630 | 4629 | // We should take hci_dev_lock() early, I think.. conn_flags can change |
| 4631 | 4630 | supported_flags = hdev->conn_flags; |
| @@ -8936,6 +8935,8 @@ void mgmt_index_removed(struct hci_dev *hdev) · 4304 unmodified lines | ||
| 8936 | 8935 | HCI_MGMT_EXT_INDEX_EVENTS); |
| 8937 | 8936 | |
| 8938 | 8937 | /* Cancel any remaining timed work */ |
| 8938 | + if (!hci_dev_test_flag(hdev, HCI_MGMT)) | |
| 8939 | + return; | |
| 8939 | 8940 | cancel_delayed_work_sync(&hdev->discov_off); |
| 8940 | 8941 | cancel_delayed_work_sync(&hdev->service_cache); |
| 8941 | 8942 | cancel_delayed_work_sync(&hdev->rpa_expired); |
| @@ -120,7 +120,10 @@ static bool read_supported_features(struct hci_dev *hdev, · 119 unmodified lines | ||
| 120 | 120 | |
| 121 | 121 | skb = __hci_cmd_sync(hdev, hdev->msft_opcode, sizeof(cp), &cp, |
| 122 | 122 | HCI_CMD_TIMEOUT); |
| 123 | − if (IS_ERR(skb)) { | |
| 123 | + if (IS_ERR_OR_NULL(skb)) { | |
| 124 | + if (!skb) | |
| 125 | + skb = ERR_PTR(-EIO); | |
| 126 | + | |
| 124 | 127 | bt_dev_err(hdev, "Failed to read MSFT supported features (%ld)", |
| 125 | 128 | PTR_ERR(skb)); |
| 126 | 129 | return false; |
| @@ -319,8 +322,11 @@ static int msft_remove_monitor_sync(struct hci_dev *hdev, · 192 unmodified lines | ||
| 319 | 322 | |
| 320 | 323 | skb = __hci_cmd_sync(hdev, hdev->msft_opcode, sizeof(cp), &cp, |
| 321 | 324 | HCI_CMD_TIMEOUT); |
| 322 | − if (IS_ERR(skb)) | |
| 325 | + if (IS_ERR_OR_NULL(skb)) { | |
| 326 | + if (!skb) | |
| 327 | + return -EIO; | |
| 323 | 328 | return PTR_ERR(skb); |
| 329 | + } | |
| 324 | 330 | |
| 325 | 331 | return msft_le_cancel_monitor_advertisement_cb(hdev, hdev->msft_opcode, |
| 326 | 332 | monitor, skb); |
| @@ -432,8 +438,11 @@ static int msft_add_monitor_sync(struct hci_dev *hdev, · 105 unmodified lines | ||
| 432 | 438 | HCI_CMD_TIMEOUT); |
| 433 | 439 | kfree(cp); |
| 434 | 440 | |
| 435 | − if (IS_ERR(skb)) | |
| 441 | + if (IS_ERR_OR_NULL(skb)) { | |
| 442 | + if (!skb) | |
| 443 | + return -EIO; | |
| 436 | 444 | return PTR_ERR(skb); |
| 445 | + } | |
| 437 | 446 | |
| 438 | 447 | return msft_le_monitor_advertisement_cb(hdev, hdev->msft_opcode, |
| 439 | 448 | monitor, skb); |