1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
|
From 1cd644db8c23a2f530ddb93cebed7dacc5f5721a Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Wed, 15 Jul 2020 18:25:37 -0700
Subject: [PATCH] shared/att: Fix possible crash on disconnect
If there are pending request while disconnecting they would be notified
but clients may endup being freed in the proccess which will then be
calling bt_att_cancel to cancal its requests causing the following
trace:
Invalid read of size 4
at 0x1D894C: enable_ccc_callback (gatt-client.c:1627)
by 0x1D247B: disc_att_send_op (att.c:417)
by 0x1CCC17: queue_remove_all (queue.c:354)
by 0x1D47B7: disconnect_cb (att.c:635)
by 0x1E0707: watch_callback (io-glib.c:170)
by 0x48E963B: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.6400.4)
by 0x48E9AC7: ??? (in /usr/lib/libglib-2.0.so.0.6400.4)
by 0x48E9ECF: g_main_loop_run (in /usr/lib/libglib-2.0.so.0.6400.4)
by 0x1E0E97: mainloop_run (mainloop-glib.c:79)
by 0x1E13B3: mainloop_run_with_signal (mainloop-notify.c:201)
by 0x12BC3B: main (main.c:770)
Address 0x7d40a28 is 24 bytes inside a block of size 32 free'd
at 0x484A2E0: free (vg_replace_malloc.c:540)
by 0x1CCC17: queue_remove_all (queue.c:354)
by 0x1CCC83: queue_destroy (queue.c:73)
by 0x1D7DD7: bt_gatt_client_free (gatt-client.c:2209)
by 0x16497B: batt_free (battery.c:77)
by 0x16497B: batt_remove (battery.c:286)
by 0x1A0013: service_remove (service.c:176)
by 0x1A9B7B: device_remove_gatt_service (device.c:3691)
by 0x1A9B7B: gatt_service_removed (device.c:3805)
by 0x1CC90B: queue_foreach (queue.c:220)
by 0x1DE27B: notify_service_changed.isra.0.part.0 (gatt-db.c:369)
by 0x1DE387: notify_service_changed (gatt-db.c:361)
by 0x1DE387: gatt_db_service_destroy (gatt-db.c:385)
by 0x1DE3EF: gatt_db_remove_service (gatt-db.c:519)
by 0x1D674F: discovery_op_complete (gatt-client.c:388)
by 0x1D6877: discover_primary_cb (gatt-client.c:1260)
by 0x1E220B: discovery_op_complete (gatt-helpers.c:628)
by 0x1E249B: read_by_grp_type_cb (gatt-helpers.c:730)
by 0x1D247B: disc_att_send_op (att.c:417)
by 0x1CCC17: queue_remove_all (queue.c:354)
by 0x1D47B7: disconnect_cb (att.c:635)
Upstream-Status: Backport
[https://github.com/bluez/bluez/commit/1cd644db8c23a2f530ddb93cebed7dacc5f5721a]
CVE: CVE-2020-27153
Signed-off-by: Chee Yang Lee <chee.yang.lee@intel.com>
---
src/shared/att.c | 46 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 40 insertions(+), 6 deletions(-)
diff --git a/src/shared/att.c b/src/shared/att.c
index ed3af2920..58f23dfcb 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -84,6 +84,7 @@ struct bt_att {
struct queue *req_queue; /* Queued ATT protocol requests */
struct queue *ind_queue; /* Queued ATT protocol indications */
struct queue *write_queue; /* Queue of PDUs ready to send */
+ bool in_disc; /* Cleanup queues on disconnect_cb */
bt_att_timeout_func_t timeout_callback;
bt_att_destroy_func_t timeout_destroy;
@@ -222,8 +223,10 @@ static void destroy_att_send_op(void *data)
free(op);
}
-static void cancel_att_send_op(struct att_send_op *op)
+static void cancel_att_send_op(void *data)
{
+ struct att_send_op *op = data;
+
if (op->destroy)
op->destroy(op->user_data);
@@ -631,11 +634,6 @@ static bool disconnect_cb(struct io *io, void *user_data)
/* Dettach channel */
queue_remove(att->chans, chan);
- /* Notify request callbacks */
- queue_remove_all(att->req_queue, NULL, NULL, disc_att_send_op);
- queue_remove_all(att->ind_queue, NULL, NULL, disc_att_send_op);
- queue_remove_all(att->write_queue, NULL, NULL, disc_att_send_op);
-
if (chan->pending_req) {
disc_att_send_op(chan->pending_req);
chan->pending_req = NULL;
@@ -654,6 +652,15 @@ static bool disconnect_cb(struct io *io, void *user_data)
bt_att_ref(att);
+ att->in_disc = true;
+
+ /* Notify request callbacks */
+ queue_remove_all(att->req_queue, NULL, NULL, disc_att_send_op);
+ queue_remove_all(att->ind_queue, NULL, NULL, disc_att_send_op);
+ queue_remove_all(att->write_queue, NULL, NULL, disc_att_send_op);
+
+ att->in_disc = false;
+
queue_foreach(att->disconn_list, disconn_handler, INT_TO_PTR(err));
bt_att_unregister_all(att);
@@ -1574,6 +1581,30 @@ bool bt_att_chan_cancel(struct bt_att_chan *chan, unsigned int id)
return true;
}
+static bool bt_att_disc_cancel(struct bt_att *att, unsigned int id)
+{
+ struct att_send_op *op;
+
+ op = queue_find(att->req_queue, match_op_id, UINT_TO_PTR(id));
+ if (op)
+ goto done;
+
+ op = queue_find(att->ind_queue, match_op_id, UINT_TO_PTR(id));
+ if (op)
+ goto done;
+
+ op = queue_find(att->write_queue, match_op_id, UINT_TO_PTR(id));
+
+done:
+ if (!op)
+ return false;
+
+ /* Just cancel since disconnect_cb will be cleaning up */
+ cancel_att_send_op(op);
+
+ return true;
+}
+
bool bt_att_cancel(struct bt_att *att, unsigned int id)
{
const struct queue_entry *entry;
@@ -1591,6 +1622,9 @@ bool bt_att_cancel(struct bt_att *att, unsigned int id)
return true;
}
+ if (att->in_disc)
+ return bt_att_disc_cancel(att, id);
+
op = queue_remove_if(att->req_queue, match_op_id, UINT_TO_PTR(id));
if (op)
goto done;
|
