Skip to content

Commit d0be834

Browse files
VudentzVudentz
committed
Bluetooth: L2CAP: Fix use-after-free caused by l2cap_chan_put
This fixes the following trace which is caused by hci_rx_work starting up *after* the final channel reference has been put() during sock_close() but *before* the references to the channel have been destroyed, so instead the code now rely on kref_get_unless_zero/l2cap_chan_hold_unless_zero to prevent referencing a channel that is about to be destroyed. refcount_t: increment on 0; use-after-free. BUG: KASAN: use-after-free in refcount_dec_and_test+0x20/0xd0 Read of size 4 at addr ffffffc114f5bf18 by task kworker/u17:14/705 CPU: 4 PID: 705 Comm: kworker/u17:14 Tainted: G S W 4.14.234-00003-g1fb6d0bd49a4-dirty raspberrypi#28 Hardware name: Qualcomm Technologies, Inc. SM8150 V2 PM8150 Google Inc. MSM sm8150 Flame DVT (DT) Workqueue: hci0 hci_rx_work Call trace: dump_backtrace+0x0/0x378 show_stack+0x20/0x2c dump_stack+0x124/0x148 print_address_description+0x80/0x2e8 __kasan_report+0x168/0x188 kasan_report+0x10/0x18 __asan_load4+0x84/0x8c refcount_dec_and_test+0x20/0xd0 l2cap_chan_put+0x48/0x12c l2cap_recv_frame+0x4770/0x6550 l2cap_recv_acldata+0x44c/0x7a4 hci_acldata_packet+0x100/0x188 hci_rx_work+0x178/0x23c process_one_work+0x35c/0x95c worker_thread+0x4cc/0x960 kthread+0x1a8/0x1c4 ret_from_fork+0x10/0x18 Cc: [email protected] Reported-by: Lee Jones <[email protected]> Signed-off-by: Luiz Augusto von Dentz <[email protected]> Tested-by: Lee Jones <[email protected]> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
1 parent ef61b6e commit d0be834

File tree

2 files changed

+49
-13
lines changed

2 files changed

+49
-13
lines changed

‎include/net/bluetooth/l2cap.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -847,6 +847,7 @@ enum {
847847
};
848848

849849
void l2cap_chan_hold(struct l2cap_chan *c);
850+
struct l2cap_chan *l2cap_chan_hold_unless_zero(struct l2cap_chan *c);
850851
void l2cap_chan_put(struct l2cap_chan *c);
851852

852853
static inline void l2cap_chan_lock(struct l2cap_chan *chan)

‎net/bluetooth/l2cap_core.c

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -111,23 +111,28 @@ static struct l2cap_chan *__l2cap_get_chan_by_scid(struct l2cap_conn *conn,
111111
}
112112

113113
/* Find channel with given SCID.
114-
* Returns locked channel. */
114+
* Returns a reference locked channel.
115+
*/
115116
static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn,
116117
u16 cid)
117118
{
118119
struct l2cap_chan *c;
119120

120121
mutex_lock(&conn->chan_lock);
121122
c = __l2cap_get_chan_by_scid(conn, cid);
122-
if (c)
123-
l2cap_chan_lock(c);
123+
if (c) {
124+
/* Only lock if chan reference is not 0 */
125+
c = l2cap_chan_hold_unless_zero(c);
126+
if (c)
127+
l2cap_chan_lock(c);
128+
}
124129
mutex_unlock(&conn->chan_lock);
125130

126131
return c;
127132
}
128133

129134
/* Find channel with given DCID.
130-
* Returns locked channel.
135+
* Returns a reference locked channel.
131136
*/
132137
static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
133138
u16 cid)
@@ -136,8 +141,12 @@ static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
136141

137142
mutex_lock(&conn->chan_lock);
138143
c = __l2cap_get_chan_by_dcid(conn, cid);
139-
if (c)
140-
l2cap_chan_lock(c);
144+
if (c) {
145+
/* Only lock if chan reference is not 0 */
146+
c = l2cap_chan_hold_unless_zero(c);
147+
if (c)
148+
l2cap_chan_lock(c);
149+
}
141150
mutex_unlock(&conn->chan_lock);
142151

143152
return c;
@@ -162,8 +171,12 @@ static struct l2cap_chan *l2cap_get_chan_by_ident(struct l2cap_conn *conn,
162171

163172
mutex_lock(&conn->chan_lock);
164173
c = __l2cap_get_chan_by_ident(conn, ident);
165-
if (c)
166-
l2cap_chan_lock(c);
174+
if (c) {
175+
/* Only lock if chan reference is not 0 */
176+
c = l2cap_chan_hold_unless_zero(c);
177+
if (c)
178+
l2cap_chan_lock(c);
179+
}
167180
mutex_unlock(&conn->chan_lock);
168181

169182
return c;
@@ -497,6 +510,16 @@ void l2cap_chan_hold(struct l2cap_chan *c)
497510
kref_get(&c->kref);
498511
}
499512

513+
struct l2cap_chan *l2cap_chan_hold_unless_zero(struct l2cap_chan *c)
514+
{
515+
BT_DBG("chan %p orig refcnt %u", c, kref_read(&c->kref));
516+
517+
if (!kref_get_unless_zero(&c->kref))
518+
return NULL;
519+
520+
return c;
521+
}
522+
500523
void l2cap_chan_put(struct l2cap_chan *c)
501524
{
502525
BT_DBG("chan %p orig refcnt %u", c, kref_read(&c->kref));
@@ -1968,7 +1991,10 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
19681991
src_match = !bacmp(&c->src, src);
19691992
dst_match = !bacmp(&c->dst, dst);
19701993
if (src_match && dst_match) {
1971-
l2cap_chan_hold(c);
1994+
c = l2cap_chan_hold_unless_zero(c);
1995+
if (!c)
1996+
continue;
1997+
19721998
read_unlock(&chan_list_lock);
19731999
return c;
19742000
}
@@ -1983,7 +2009,7 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
19832009
}
19842010

19852011
if (c1)
1986-
l2cap_chan_hold(c1);
2012+
c1 = l2cap_chan_hold_unless_zero(c1);
19872013

19882014
read_unlock(&chan_list_lock);
19892015

@@ -4463,6 +4489,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
44634489

44644490
unlock:
44654491
l2cap_chan_unlock(chan);
4492+
l2cap_chan_put(chan);
44664493
return err;
44674494
}
44684495

@@ -4577,6 +4604,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
45774604

45784605
done:
45794606
l2cap_chan_unlock(chan);
4607+
l2cap_chan_put(chan);
45804608
return err;
45814609
}
45824610

@@ -5304,6 +5332,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
53045332
l2cap_send_move_chan_rsp(chan, result);
53055333

53065334
l2cap_chan_unlock(chan);
5335+
l2cap_chan_put(chan);
53075336

53085337
return 0;
53095338
}
@@ -5396,6 +5425,7 @@ static void l2cap_move_continue(struct l2cap_conn *conn, u16 icid, u16 result)
53965425
}
53975426

53985427
l2cap_chan_unlock(chan);
5428+
l2cap_chan_put(chan);
53995429
}
54005430

54015431
static void l2cap_move_fail(struct l2cap_conn *conn, u8 ident, u16 icid,
@@ -5425,6 +5455,7 @@ static void l2cap_move_fail(struct l2cap_conn *conn, u8 ident, u16 icid,
54255455
l2cap_send_move_chan_cfm(chan, L2CAP_MC_UNCONFIRMED);
54265456

54275457
l2cap_chan_unlock(chan);
5458+
l2cap_chan_put(chan);
54285459
}
54295460

54305461
static int l2cap_move_channel_rsp(struct l2cap_conn *conn,
@@ -5488,6 +5519,7 @@ static int l2cap_move_channel_confirm(struct l2cap_conn *conn,
54885519
l2cap_send_move_chan_cfm_rsp(conn, cmd->ident, icid);
54895520

54905521
l2cap_chan_unlock(chan);
5522+
l2cap_chan_put(chan);
54915523

54925524
return 0;
54935525
}
@@ -5523,6 +5555,7 @@ static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
55235555
}
55245556

55255557
l2cap_chan_unlock(chan);
5558+
l2cap_chan_put(chan);
55265559

55275560
return 0;
55285561
}
@@ -5895,12 +5928,11 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn,
58955928
if (credits > max_credits) {
58965929
BT_ERR("LE credits overflow");
58975930
l2cap_send_disconn_req(chan, ECONNRESET);
5898-
l2cap_chan_unlock(chan);
58995931

59005932
/* Return 0 so that we don't trigger an unnecessary
59015933
* command reject packet.
59025934
*/
5903-
return 0;
5935+
goto unlock;
59045936
}
59055937

59065938
chan->tx_credits += credits;
@@ -5911,7 +5943,9 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn,
59115943
if (chan->tx_credits)
59125944
chan->ops->resume(chan);
59135945

5946+
unlock:
59145947
l2cap_chan_unlock(chan);
5948+
l2cap_chan_put(chan);
59155949

59165950
return 0;
59175951
}
@@ -7597,6 +7631,7 @@ static void l2cap_data_channel(struct l2cap_conn *conn, u16 cid,
75977631

75987632
done:
75997633
l2cap_chan_unlock(chan);
7634+
l2cap_chan_put(chan);
76007635
}
76017636

76027637
static void l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm,
@@ -8085,7 +8120,7 @@ static struct l2cap_chan *l2cap_global_fixed_chan(struct l2cap_chan *c,
80858120
if (src_type != c->src_type)
80868121
continue;
80878122

8088-
l2cap_chan_hold(c);
8123+
c = l2cap_chan_hold_unless_zero(c);
80898124
read_unlock(&chan_list_lock);
80908125
return c;
80918126
}

0 commit comments

Comments
 (0)