Skip to content

Commit 60f1692

Browse files
bmastbergenbmastbergen
committed
Bluetooth: L2CAP: Fix use-after-free caused by l2cap_chan_put
jira VULN-155527 cve-pre CVE-2022-50386 commit-author Luiz Augusto von Dentz <luiz.von.dentz@intel.com> commit d0be834 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: stable@kernel.org Reported-by: Lee Jones <lee.jones@linaro.org> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Tested-by: Lee Jones <lee.jones@linaro.org> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> (cherry picked from commit d0be834) Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>
1 parent ee704c4 commit 60f1692

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
@@ -807,6 +807,7 @@ enum {
807807
};
808808

809809
void l2cap_chan_hold(struct l2cap_chan *c);
810+
struct l2cap_chan *l2cap_chan_hold_unless_zero(struct l2cap_chan *c);
810811
void l2cap_chan_put(struct l2cap_chan *c);
811812

812813
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
@@ -114,23 +114,28 @@ static struct l2cap_chan *__l2cap_get_chan_by_scid(struct l2cap_conn *conn,
114114
}
115115

116116
/* Find channel with given SCID.
117-
* Returns locked channel. */
117+
* Returns a reference locked channel.
118+
*/
118119
static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn,
119120
u16 cid)
120121
{
121122
struct l2cap_chan *c;
122123

123124
mutex_lock(&conn->chan_lock);
124125
c = __l2cap_get_chan_by_scid(conn, cid);
125-
if (c)
126-
l2cap_chan_lock(c);
126+
if (c) {
127+
/* Only lock if chan reference is not 0 */
128+
c = l2cap_chan_hold_unless_zero(c);
129+
if (c)
130+
l2cap_chan_lock(c);
131+
}
127132
mutex_unlock(&conn->chan_lock);
128133

129134
return c;
130135
}
131136

132137
/* Find channel with given DCID.
133-
* Returns locked channel.
138+
* Returns a reference locked channel.
134139
*/
135140
static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
136141
u16 cid)
@@ -139,8 +144,12 @@ static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
139144

140145
mutex_lock(&conn->chan_lock);
141146
c = __l2cap_get_chan_by_dcid(conn, cid);
142-
if (c)
143-
l2cap_chan_lock(c);
147+
if (c) {
148+
/* Only lock if chan reference is not 0 */
149+
c = l2cap_chan_hold_unless_zero(c);
150+
if (c)
151+
l2cap_chan_lock(c);
152+
}
144153
mutex_unlock(&conn->chan_lock);
145154

146155
return c;
@@ -165,8 +174,12 @@ static struct l2cap_chan *l2cap_get_chan_by_ident(struct l2cap_conn *conn,
165174

166175
mutex_lock(&conn->chan_lock);
167176
c = __l2cap_get_chan_by_ident(conn, ident);
168-
if (c)
169-
l2cap_chan_lock(c);
177+
if (c) {
178+
/* Only lock if chan reference is not 0 */
179+
c = l2cap_chan_hold_unless_zero(c);
180+
if (c)
181+
l2cap_chan_lock(c);
182+
}
170183
mutex_unlock(&conn->chan_lock);
171184

172185
return c;
@@ -487,6 +500,16 @@ void l2cap_chan_hold(struct l2cap_chan *c)
487500
kref_get(&c->kref);
488501
}
489502

503+
struct l2cap_chan *l2cap_chan_hold_unless_zero(struct l2cap_chan *c)
504+
{
505+
BT_DBG("chan %p orig refcnt %u", c, kref_read(&c->kref));
506+
507+
if (!kref_get_unless_zero(&c->kref))
508+
return NULL;
509+
510+
return c;
511+
}
512+
490513
void l2cap_chan_put(struct l2cap_chan *c)
491514
{
492515
BT_DBG("chan %p orig refcnt %d", c, kref_read(&c->kref));
@@ -1800,7 +1823,10 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
18001823
src_match = !bacmp(&c->src, src);
18011824
dst_match = !bacmp(&c->dst, dst);
18021825
if (src_match && dst_match) {
1803-
l2cap_chan_hold(c);
1826+
c = l2cap_chan_hold_unless_zero(c);
1827+
if (!c)
1828+
continue;
1829+
18041830
read_unlock(&chan_list_lock);
18051831
return c;
18061832
}
@@ -1815,7 +1841,7 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
18151841
}
18161842

18171843
if (c1)
1818-
l2cap_chan_hold(c1);
1844+
c1 = l2cap_chan_hold_unless_zero(c1);
18191845

18201846
read_unlock(&chan_list_lock);
18211847

@@ -4190,6 +4216,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
41904216

41914217
unlock:
41924218
l2cap_chan_unlock(chan);
4219+
l2cap_chan_put(chan);
41934220
return err;
41944221
}
41954222

@@ -4302,6 +4329,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
43024329

43034330
done:
43044331
l2cap_chan_unlock(chan);
4332+
l2cap_chan_put(chan);
43054333
return err;
43064334
}
43074335

@@ -5026,6 +5054,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
50265054
l2cap_send_move_chan_rsp(chan, result);
50275055

50285056
l2cap_chan_unlock(chan);
5057+
l2cap_chan_put(chan);
50295058

50305059
return 0;
50315060
}
@@ -5118,6 +5147,7 @@ static void l2cap_move_continue(struct l2cap_conn *conn, u16 icid, u16 result)
51185147
}
51195148

51205149
l2cap_chan_unlock(chan);
5150+
l2cap_chan_put(chan);
51215151
}
51225152

51235153
static void l2cap_move_fail(struct l2cap_conn *conn, u8 ident, u16 icid,
@@ -5147,6 +5177,7 @@ static void l2cap_move_fail(struct l2cap_conn *conn, u8 ident, u16 icid,
51475177
l2cap_send_move_chan_cfm(chan, L2CAP_MC_UNCONFIRMED);
51485178

51495179
l2cap_chan_unlock(chan);
5180+
l2cap_chan_put(chan);
51505181
}
51515182

51525183
static int l2cap_move_channel_rsp(struct l2cap_conn *conn,
@@ -5210,6 +5241,7 @@ static int l2cap_move_channel_confirm(struct l2cap_conn *conn,
52105241
l2cap_send_move_chan_cfm_rsp(conn, cmd->ident, icid);
52115242

52125243
l2cap_chan_unlock(chan);
5244+
l2cap_chan_put(chan);
52135245

52145246
return 0;
52155247
}
@@ -5245,6 +5277,7 @@ static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
52455277
}
52465278

52475279
l2cap_chan_unlock(chan);
5280+
l2cap_chan_put(chan);
52485281

52495282
return 0;
52505283
}
@@ -5630,12 +5663,11 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn,
56305663
if (credits > max_credits) {
56315664
BT_ERR("LE credits overflow");
56325665
l2cap_send_disconn_req(chan, ECONNRESET);
5633-
l2cap_chan_unlock(chan);
56345666

56355667
/* Return 0 so that we don't trigger an unnecessary
56365668
* command reject packet.
56375669
*/
5638-
return 0;
5670+
goto unlock;
56395671
}
56405672

56415673
chan->tx_credits += credits;
@@ -5648,7 +5680,9 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn,
56485680
if (chan->tx_credits)
56495681
chan->ops->resume(chan);
56505682

5683+
unlock:
56515684
l2cap_chan_unlock(chan);
5685+
l2cap_chan_put(chan);
56525686

56535687
return 0;
56545688
}
@@ -6970,6 +7004,7 @@ static void l2cap_data_channel(struct l2cap_conn *conn, u16 cid,
69707004

69717005
done:
69727006
l2cap_chan_unlock(chan);
7007+
l2cap_chan_put(chan);
69737008
}
69747009

69757010
static void l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm,
@@ -7374,7 +7409,7 @@ static struct l2cap_chan *l2cap_global_fixed_chan(struct l2cap_chan *c,
73747409
if (src_type != c->src_type)
73757410
continue;
73767411

7377-
l2cap_chan_hold(c);
7412+
c = l2cap_chan_hold_unless_zero(c);
73787413
read_unlock(&chan_list_lock);
73797414
return c;
73807415
}

0 commit comments

Comments
 (0)