Hello Frédéric Danis,
Commit ad5747d4eed1 ("Bluetooth: l2cap: Process valid commands in too
long frame") from Apr 14, 2025 (linux-next), leads to the following
Smatch static checker warning:
net/bluetooth/l2cap_core.c:7613 l2cap_recv_acldata()
error: double free of 'skb' (line 7557)
net/bluetooth/l2cap_core.c
7484 void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
7485 {
7486 struct l2cap_conn *conn;
7487 int len;
7488
7489 /* Lock hdev to access l2cap_data to avoid race with l2cap_conn_del */
7490 hci_dev_lock(hcon->hdev);
7491
7492 conn = hcon->l2cap_data;
7493
7494 if (!conn)
7495 conn = l2cap_conn_add(hcon);
7496
7497 conn = l2cap_conn_hold_unless_zero(conn);
7498
7499 hci_dev_unlock(hcon->hdev);
7500
7501 if (!conn) {
7502 kfree_skb(skb);
7503 return;
7504 }
7505
7506 BT_DBG("conn %p len %u flags 0x%x", conn, skb->len, flags);
7507
7508 mutex_lock(&conn->lock);
7509
7510 switch (flags) {
7511 case ACL_START:
7512 case ACL_START_NO_FLUSH:
7513 case ACL_COMPLETE:
7514 if (conn->rx_skb) {
7515 BT_ERR("Unexpected start frame (len %d)", skb->len);
7516 l2cap_recv_reset(conn);
7517 l2cap_conn_unreliable(conn, ECOMM);
7518 }
7519
7520 /* Start fragment may not contain the L2CAP length so just
7521 * copy the initial byte when that happens and use conn->mtu as
7522 * expected length.
7523 */
7524 if (skb->len < L2CAP_LEN_SIZE) {
7525 l2cap_recv_frag(conn, skb, conn->mtu);
7526 break;
7527 }
7528
7529 len = get_unaligned_le16(skb->data) + L2CAP_HDR_SIZE;
7530
7531 if (len == skb->len) {
7532 /* Complete frame received */
7533 l2cap_recv_frame(conn, skb);
7534 goto unlock;
7535 }
7536
7537 BT_DBG("Start: total len %d, frag len %u", len, skb->len);
7538
7539 if (skb->len > len) {
7540 BT_ERR("Frame is too long (len %u, expected len %d)",
7541 skb->len, len);
7542 /* PTS test cases L2CAP/COS/CED/BI-14-C and BI-15-C
7543 * (Multiple Signaling Command in one PDU, Data
7544 * Truncated, BR/EDR) send a C-frame to the IUT with
7545 * PDU Length set to 8 and Channel ID set to the
7546 * correct signaling channel for the logical link.
7547 * The Information payload contains one L2CAP_ECHO_REQ
7548 * packet with Data Length set to 0 with 0 octets of
7549 * echo data and one invalid command packet due to
7550 * data truncated in PDU but present in HCI packet.
7551 *
7552 * Shorter the socket buffer to the PDU length to
7553 * allow to process valid commands from the PDU before
7554 * setting the socket unreliable.
7555 */
7556 skb->len = len;
7557 l2cap_recv_frame(conn, skb);
^^^
Freed
7558 l2cap_conn_unreliable(conn, ECOMM);
7559 goto drop;
^^^^^^^^^
7560 }
7561
7562 /* Append fragment into frame (with header) */
7563 if (l2cap_recv_frag(conn, skb, len) < 0)
7564 goto drop;
7565
7566 break;
7567
7568 case ACL_CONT:
7569 BT_DBG("Cont: frag len %u (expecting %u)", skb->len, conn->rx_len);
7570
7571 if (!conn->rx_skb) {
7572 BT_ERR("Unexpected continuation frame (len %d)", skb->len);
7573 l2cap_conn_unreliable(conn, ECOMM);
7574 goto drop;
7575 }
7576
7577 /* Complete the L2CAP length if it has not been read */
7578 if (conn->rx_skb->len < L2CAP_LEN_SIZE) {
7579 if (l2cap_recv_len(conn, skb) < 0) {
7580 l2cap_conn_unreliable(conn, ECOMM);
7581 goto drop;
7582 }
7583
7584 /* Header still could not be read just continue */
7585 if (conn->rx_skb->len < L2CAP_LEN_SIZE)
7586 break;
7587 }
7588
7589 if (skb->len > conn->rx_len) {
7590 BT_ERR("Fragment is too long (len %u, expected %u)",
7591 skb->len, conn->rx_len);
7592 l2cap_recv_reset(conn);
7593 l2cap_conn_unreliable(conn, ECOMM);
7594 goto drop;
7595 }
7596
7597 /* Append fragment into frame (with header) */
7598 l2cap_recv_frag(conn, skb, skb->len);
7599
7600 if (!conn->rx_len) {
7601 /* Complete frame received. l2cap_recv_frame
7602 * takes ownership of the skb so set the global
7603 * rx_skb pointer to NULL first.
7604 */
7605 struct sk_buff *rx_skb = conn->rx_skb;
7606 conn->rx_skb = NULL;
7607 l2cap_recv_frame(conn, rx_skb);
7608 }
7609 break;
7610 }
7611
7612 drop:
--> 7613 kfree_skb(skb);
double freed.
7614 unlock:
7615 mutex_unlock(&conn->lock);
7616 l2cap_conn_put(conn);
7617 }
regards,
dan carpenter
