Skip to content

Commit f32d561

Browse files
Vudentzgregkh
Vudentz
authored and
gregkh
committed
Bluetooth: L2CAP: Fix use-after-free caused by l2cap_chan_put
commit d0be834 upstream. 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 #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]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 7d8048d commit f32d561

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));
@@ -1969,7 +1992,10 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
19691992
src_match = !bacmp(&c->src, src);
19701993
dst_match = !bacmp(&c->dst, dst);
19711994
if (src_match && dst_match) {
1972-
l2cap_chan_hold(c);
1995+
c = l2cap_chan_hold_unless_zero(c);
1996+
if (!c)
1997+
continue;
1998+
19731999
read_unlock(&chan_list_lock);
19742000
return c;
19752001
}
@@ -1984,7 +2010,7 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
19842010
}
19852011

19862012
if (c1)
1987-
l2cap_chan_hold(c1);
2013+
c1 = l2cap_chan_hold_unless_zero(c1);
19882014

19892015
read_unlock(&chan_list_lock);
19902016

@@ -4464,6 +4490,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
44644490

44654491
unlock:
44664492
l2cap_chan_unlock(chan);
4493+
l2cap_chan_put(chan);
44674494
return err;
44684495
}
44694496

@@ -4578,6 +4605,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
45784605

45794606
done:
45804607
l2cap_chan_unlock(chan);
4608+
l2cap_chan_put(chan);
45814609
return err;
45824610
}
45834611

@@ -5305,6 +5333,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
53055333
l2cap_send_move_chan_rsp(chan, result);
53065334

53075335
l2cap_chan_unlock(chan);
5336+
l2cap_chan_put(chan);
53085337

53095338
return 0;
53105339
}
@@ -5397,6 +5426,7 @@ static void l2cap_move_continue(struct l2cap_conn *conn, u16 icid, u16 result)
53975426
}
53985427

53995428
l2cap_chan_unlock(chan);
5429+
l2cap_chan_put(chan);
54005430
}
54015431

54025432
static void l2cap_move_fail(struct l2cap_conn *conn, u8 ident, u16 icid,
@@ -5426,6 +5456,7 @@ static void l2cap_move_fail(struct l2cap_conn *conn, u8 ident, u16 icid,
54265456
l2cap_send_move_chan_cfm(chan, L2CAP_MC_UNCONFIRMED);
54275457

54285458
l2cap_chan_unlock(chan);
5459+
l2cap_chan_put(chan);
54295460
}
54305461

54315462
static int l2cap_move_channel_rsp(struct l2cap_conn *conn,
@@ -5489,6 +5520,7 @@ static int l2cap_move_channel_confirm(struct l2cap_conn *conn,
54895520
l2cap_send_move_chan_cfm_rsp(conn, cmd->ident, icid);
54905521

54915522
l2cap_chan_unlock(chan);
5523+
l2cap_chan_put(chan);
54925524

54935525
return 0;
54945526
}
@@ -5524,6 +5556,7 @@ static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
55245556
}
55255557

55265558
l2cap_chan_unlock(chan);
5559+
l2cap_chan_put(chan);
55275560

55285561
return 0;
55295562
}
@@ -5896,12 +5929,11 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn,
58965929
if (credits > max_credits) {
58975930
BT_ERR("LE credits overflow");
58985931
l2cap_send_disconn_req(chan, ECONNRESET);
5899-
l2cap_chan_unlock(chan);
59005932

59015933
/* Return 0 so that we don't trigger an unnecessary
59025934
* command reject packet.
59035935
*/
5904-
return 0;
5936+
goto unlock;
59055937
}
59065938

59075939
chan->tx_credits += credits;
@@ -5912,7 +5944,9 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn,
59125944
if (chan->tx_credits)
59135945
chan->ops->resume(chan);
59145946

5947+
unlock:
59155948
l2cap_chan_unlock(chan);
5949+
l2cap_chan_put(chan);
59165950

59175951
return 0;
59185952
}
@@ -7598,6 +7632,7 @@ static void l2cap_data_channel(struct l2cap_conn *conn, u16 cid,
75987632

75997633
done:
76007634
l2cap_chan_unlock(chan);
7635+
l2cap_chan_put(chan);
76017636
}
76027637

76037638
static void l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm,
@@ -8086,7 +8121,7 @@ static struct l2cap_chan *l2cap_global_fixed_chan(struct l2cap_chan *c,
80868121
if (src_type != c->src_type)
80878122
continue;
80888123

8089-
l2cap_chan_hold(c);
8124+
c = l2cap_chan_hold_unless_zero(c);
80908125
read_unlock(&chan_list_lock);
80918126
return c;
80928127
}

0 commit comments

Comments
 (0)