Hello Luiz Augusto von Dentz,
Commit dd6367916d2d ("Bluetooth: L2CAP: Fix corrupted list in
hci_chan_del") from Feb 6, 2025 (linux-next), leads to the following
Smatch static checker warning:
net/bluetooth/l2cap_core.c:7555 l2cap_recv_acldata()
error: we previously assumed 'conn' could be null (see line 7459)
net/bluetooth/l2cap_core.c
7442 void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
7443 {
7444 struct l2cap_conn *conn;
7445 int len;
7446
7447 /* Lock hdev to access l2cap_data to avoid race with l2cap_conn_del */
7448 hci_dev_lock(hcon->hdev);
7449
7450 conn = hcon->l2cap_data;
7451
7452 if (!conn)
7453 conn = l2cap_conn_add(hcon);
7454
7455 conn = l2cap_conn_hold_unless_zero(conn);
7456
7457 hci_dev_unlock(hcon->hdev);
7458
7459 if (!conn)
7460 goto drop;
Do you want to move drop after the unlock: or do you want to make this
a direct return? Choices, choices...
if (!conn) {
kfree_skb(skb);
return;
}
7461
7462 BT_DBG("conn %p len %u flags 0x%x", conn, skb->len, flags);
7463
7464 mutex_lock(&conn->lock);
7465
7466 switch (flags) {
7467 case ACL_START:
7468 case ACL_START_NO_FLUSH:
7469 case ACL_COMPLETE:
7470 if (conn->rx_skb) {
7471 BT_ERR("Unexpected start frame (len %d)", skb->len);
7472 l2cap_recv_reset(conn);
7473 l2cap_conn_unreliable(conn, ECOMM);
7474 }
7475
7476 /* Start fragment may not contain the L2CAP length so just
7477 * copy the initial byte when that happens and use conn->mtu as
7478 * expected length.
7479 */
7480 if (skb->len < L2CAP_LEN_SIZE) {
7481 l2cap_recv_frag(conn, skb, conn->mtu);
7482 break;
7483 }
7484
7485 len = get_unaligned_le16(skb->data) + L2CAP_HDR_SIZE;
7486
7487 if (len == skb->len) {
7488 /* Complete frame received */
7489 l2cap_recv_frame(conn, skb);
7490 goto unlock;
7491 }
7492
7493 BT_DBG("Start: total len %d, frag len %u", len, skb->len);
7494
7495 if (skb->len > len) {
7496 BT_ERR("Frame is too long (len %u, expected len %d)",
7497 skb->len, len);
7498 l2cap_conn_unreliable(conn, ECOMM);
7499 goto drop;
7500 }
7501
7502 /* Append fragment into frame (with header) */
7503 if (l2cap_recv_frag(conn, skb, len) < 0)
7504 goto drop;
7505
7506 break;
7507
7508 case ACL_CONT:
7509 BT_DBG("Cont: frag len %u (expecting %u)", skb->len, conn->rx_len);
7510
7511 if (!conn->rx_skb) {
7512 BT_ERR("Unexpected continuation frame (len %d)", skb->len);
7513 l2cap_conn_unreliable(conn, ECOMM);
7514 goto drop;
7515 }
7516
7517 /* Complete the L2CAP length if it has not been read */
7518 if (conn->rx_skb->len < L2CAP_LEN_SIZE) {
7519 if (l2cap_recv_len(conn, skb) < 0) {
7520 l2cap_conn_unreliable(conn, ECOMM);
7521 goto drop;
7522 }
7523
7524 /* Header still could not be read just continue */
7525 if (conn->rx_skb->len < L2CAP_LEN_SIZE)
7526 break;
7527 }
7528
7529 if (skb->len > conn->rx_len) {
7530 BT_ERR("Fragment is too long (len %u, expected %u)",
7531 skb->len, conn->rx_len);
7532 l2cap_recv_reset(conn);
7533 l2cap_conn_unreliable(conn, ECOMM);
7534 goto drop;
7535 }
7536
7537 /* Append fragment into frame (with header) */
7538 l2cap_recv_frag(conn, skb, skb->len);
7539
7540 if (!conn->rx_len) {
7541 /* Complete frame received. l2cap_recv_frame
7542 * takes ownership of the skb so set the global
7543 * rx_skb pointer to NULL first.
7544 */
7545 struct sk_buff *rx_skb = conn->rx_skb;
7546 conn->rx_skb = NULL;
7547 l2cap_recv_frame(conn, rx_skb);
7548 }
7549 break;
7550 }
7551
7552 drop:
7553 kfree_skb(skb);
7554 unlock:
--> 7555 mutex_unlock(&conn->lock);
7556 l2cap_conn_put(conn);
7557 }
regards,
dan carpenter
