Skip to content

Commit

Permalink
Merge tag 'for-net-2022-10-02' of git://git.kernel.org/pub/scm/linux/…
Browse files Browse the repository at this point in the history 
…kernel/git/bluetooth/bluetooth

Luiz Augusto von Dentz says:

====================
bluetooth 2022-11-02

 - Fix memory leak in hci_vhci driver
 - Fix handling of skb on virtio_bt driver
 - Fix accepting connection for invalid L2CAP PSM
 - Fix attemting to access uninitialized memory
 - Fix use-after-free in l2cap_reassemble_sdu
 - Fix use-after-free in l2cap_conn_del
 - Fix handling of destination address type for CIS
 - Fix not restoring ISO buffer count on disconnect

* tag 'for-net-2022-10-02' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth:
  Bluetooth: L2CAP: Fix attempting to access uninitialized memory
  Bluetooth: L2CAP: Fix l2cap_global_chan_by_psm
  Bluetooth: L2CAP: Fix accepting connection request for invalid SPSM
  Bluetooth: hci_conn: Fix not restoring ISO buffer count on disconnect
  Bluetooth: L2CAP: Fix memory leak in vhci_write
  Bluetooth: L2CAP: fix use-after-free in l2cap_conn_del()
  Bluetooth: virtio_bt: Use skb_put to set length
  Bluetooth: hci_conn: Fix CIS connection dst_type handling
  Bluetooth: L2CAP: Fix use-after-free caused by l2cap_reassemble_sdu
====================

Link: https://lore.kernel.org/r/20221102235927.3324891-1-luiz.dentz@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Nov 3, 2022
2 parents ba9169f + b1a2cd5 commit ef1fdc9
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 22 deletions.
2 changes: 1 addition & 1 deletion drivers/bluetooth/virtio_bt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ static void virtbt_rx_work(struct work_struct *work)
if (!skb)
return;

skb->len = len;
skb_put(skb, len);
virtbt_rx_handle(vbt, skb);

if (virtbt_add_inbuf(vbt) < 0)
Expand Down
18 changes: 12 additions & 6 deletions net/bluetooth/hci_conn.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1067,10 +1067,21 @@ int hci_conn_del(struct hci_conn *conn)
hdev->acl_cnt += conn->sent;
} else {
struct hci_conn *acl = conn->link;

if (acl) {
acl->link = NULL;
hci_conn_drop(acl);
}

/* Unacked ISO frames */
if (conn->type == ISO_LINK) {
if (hdev->iso_pkts)
hdev->iso_cnt += conn->sent;
else if (hdev->le_pkts)
hdev->le_cnt += conn->sent;
else
hdev->acl_cnt += conn->sent;
}
}

if (conn->amp_mgr)
Expand Down Expand Up @@ -1761,6 +1772,7 @@ struct hci_conn *hci_bind_cis(struct hci_dev *hdev, bdaddr_t *dst,
if (!cis)
return ERR_PTR(-ENOMEM);
cis->cleanup = cis_cleanup;
cis->dst_type = dst_type;
}

if (cis->state == BT_CONNECTED)
Expand Down Expand Up @@ -2140,12 +2152,6 @@ struct hci_conn *hci_connect_cis(struct hci_dev *hdev, bdaddr_t *dst,
struct hci_conn *le;
struct hci_conn *cis;

/* Convert from ISO socket address type to HCI address type */
if (dst_type == BDADDR_LE_PUBLIC)
dst_type = ADDR_LE_DEV_PUBLIC;
else
dst_type = ADDR_LE_DEV_RANDOM;

if (hci_dev_test_flag(hdev, HCI_ADVERTISING))
le = hci_connect_le(hdev, dst, dst_type, false,
BT_SECURITY_LOW,
Expand Down
14 changes: 12 additions & 2 deletions net/bluetooth/iso.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,14 @@ static int iso_chan_add(struct iso_conn *conn, struct sock *sk,
return err;
}

static inline u8 le_addr_type(u8 bdaddr_type)
{
if (bdaddr_type == BDADDR_LE_PUBLIC)
return ADDR_LE_DEV_PUBLIC;
else
return ADDR_LE_DEV_RANDOM;
}

static int iso_connect_bis(struct sock *sk)
{
struct iso_conn *conn;
Expand Down Expand Up @@ -328,14 +336,16 @@ static int iso_connect_cis(struct sock *sk)
/* Just bind if DEFER_SETUP has been set */
if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
hcon = hci_bind_cis(hdev, &iso_pi(sk)->dst,
iso_pi(sk)->dst_type, &iso_pi(sk)->qos);
le_addr_type(iso_pi(sk)->dst_type),
&iso_pi(sk)->qos);
if (IS_ERR(hcon)) {
err = PTR_ERR(hcon);
goto done;
}
} else {
hcon = hci_connect_cis(hdev, &iso_pi(sk)->dst,
iso_pi(sk)->dst_type, &iso_pi(sk)->qos);
le_addr_type(iso_pi(sk)->dst_type),
&iso_pi(sk)->qos);
if (IS_ERR(hcon)) {
err = PTR_ERR(hcon);
goto done;
Expand Down
86 changes: 73 additions & 13 deletions net/bluetooth/l2cap_core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1990,7 +1990,7 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
if (link_type == LE_LINK && c->src_type == BDADDR_BREDR)
continue;

if (c->psm == psm) {
if (c->chan_type != L2CAP_CHAN_FIXED && c->psm == psm) {
int src_match, dst_match;
int src_any, dst_any;

Expand Down Expand Up @@ -3764,7 +3764,8 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data
l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC,
sizeof(rfc), (unsigned long) &rfc, endptr - ptr);

if (test_bit(FLAG_EFS_ENABLE, &chan->flags)) {
if (remote_efs &&
test_bit(FLAG_EFS_ENABLE, &chan->flags)) {
chan->remote_id = efs.id;
chan->remote_stype = efs.stype;
chan->remote_msdu = le16_to_cpu(efs.msdu);
Expand Down Expand Up @@ -5813,6 +5814,19 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
BT_DBG("psm 0x%2.2x scid 0x%4.4x mtu %u mps %u", __le16_to_cpu(psm),
scid, mtu, mps);

/* BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 3, Part A
* page 1059:
*
* Valid range: 0x0001-0x00ff
*
* Table 4.15: L2CAP_LE_CREDIT_BASED_CONNECTION_REQ SPSM ranges
*/
if (!psm || __le16_to_cpu(psm) > L2CAP_PSM_LE_DYN_END) {
result = L2CAP_CR_LE_BAD_PSM;
chan = NULL;
goto response;
}

/* Check if we have socket listening on psm */
pchan = l2cap_global_chan_by_psm(BT_LISTEN, psm, &conn->hcon->src,
&conn->hcon->dst, LE_LINK);
Expand Down Expand Up @@ -6001,6 +6015,18 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,

psm = req->psm;

/* BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 3, Part A
* page 1059:
*
* Valid range: 0x0001-0x00ff
*
* Table 4.15: L2CAP_LE_CREDIT_BASED_CONNECTION_REQ SPSM ranges
*/
if (!psm || __le16_to_cpu(psm) > L2CAP_PSM_LE_DYN_END) {
result = L2CAP_CR_LE_BAD_PSM;
goto response;
}

BT_DBG("psm 0x%2.2x mtu %u mps %u", __le16_to_cpu(psm), mtu, mps);

memset(&pdu, 0, sizeof(pdu));
Expand Down Expand Up @@ -6885,6 +6911,7 @@ static int l2cap_rx_state_recv(struct l2cap_chan *chan,
struct l2cap_ctrl *control,
struct sk_buff *skb, u8 event)
{
struct l2cap_ctrl local_control;
int err = 0;
bool skb_in_use = false;

Expand All @@ -6909,15 +6936,32 @@ static int l2cap_rx_state_recv(struct l2cap_chan *chan,
chan->buffer_seq = chan->expected_tx_seq;
skb_in_use = true;

/* l2cap_reassemble_sdu may free skb, hence invalidate
* control, so make a copy in advance to use it after
* l2cap_reassemble_sdu returns and to avoid the race
* condition, for example:
*
* The current thread calls:
* l2cap_reassemble_sdu
* chan->ops->recv == l2cap_sock_recv_cb
* __sock_queue_rcv_skb
* Another thread calls:
* bt_sock_recvmsg
* skb_recv_datagram
* skb_free_datagram
* Then the current thread tries to access control, but
* it was freed by skb_free_datagram.
*/
local_control = *control;
err = l2cap_reassemble_sdu(chan, skb, control);
if (err)
break;

if (control->final) {
if (local_control.final) {
if (!test_and_clear_bit(CONN_REJ_ACT,
&chan->conn_state)) {
control->final = 0;
l2cap_retransmit_all(chan, control);
local_control.final = 0;
l2cap_retransmit_all(chan, &local_control);
l2cap_ertm_send(chan);
}
}
Expand Down Expand Up @@ -7297,11 +7341,27 @@ static int l2cap_rx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
static int l2cap_stream_rx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
struct sk_buff *skb)
{
/* l2cap_reassemble_sdu may free skb, hence invalidate control, so store
* the txseq field in advance to use it after l2cap_reassemble_sdu
* returns and to avoid the race condition, for example:
*
* The current thread calls:
* l2cap_reassemble_sdu
* chan->ops->recv == l2cap_sock_recv_cb
* __sock_queue_rcv_skb
* Another thread calls:
* bt_sock_recvmsg
* skb_recv_datagram
* skb_free_datagram
* Then the current thread tries to access control, but it was freed by
* skb_free_datagram.
*/
u16 txseq = control->txseq;

BT_DBG("chan %p, control %p, skb %p, state %d", chan, control, skb,
chan->rx_state);

if (l2cap_classify_txseq(chan, control->txseq) ==
L2CAP_TXSEQ_EXPECTED) {
if (l2cap_classify_txseq(chan, txseq) == L2CAP_TXSEQ_EXPECTED) {
l2cap_pass_to_tx(chan, control);

BT_DBG("buffer_seq %u->%u", chan->buffer_seq,
Expand All @@ -7324,8 +7384,8 @@ static int l2cap_stream_rx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
}
}

chan->last_acked_seq = control->txseq;
chan->expected_tx_seq = __next_seq(chan, control->txseq);
chan->last_acked_seq = txseq;
chan->expected_tx_seq = __next_seq(chan, txseq);

return 0;
}
Expand Down Expand Up @@ -7581,6 +7641,7 @@ static void l2cap_data_channel(struct l2cap_conn *conn, u16 cid,
return;
}

l2cap_chan_hold(chan);
l2cap_chan_lock(chan);
} else {
BT_DBG("unknown cid 0x%4.4x", cid);
Expand Down Expand Up @@ -8426,9 +8487,8 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
* expected length.
*/
if (skb->len < L2CAP_LEN_SIZE) {
if (l2cap_recv_frag(conn, skb, conn->mtu) < 0)
goto drop;
return;
l2cap_recv_frag(conn, skb, conn->mtu);
break;
}

len = get_unaligned_le16(skb->data) + L2CAP_HDR_SIZE;
Expand Down Expand Up @@ -8472,7 +8532,7 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)

/* Header still could not be read just continue */
if (conn->rx_skb->len < L2CAP_LEN_SIZE)
return;
break;
}

if (skb->len > conn->rx_len) {
Expand Down

0 comments on commit ef1fdc9

Please sign in to comment.