Skip to content

Commit 9d32011

Browse files
committed
Add some protection codes to the curl module to avoid possible use after free problems
1 parent 7e1c355 commit 9d32011

File tree

2 files changed

+41
-17
lines changed

2 files changed

+41
-17
lines changed

ext-src/php_swoole_curl.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ struct Socket {
5252
int bitmask;
5353
int sockfd;
5454
int action;
55+
bool deleted;
5556
};
5657

5758
struct Handle {
@@ -77,7 +78,9 @@ void destroy_handle(CURL *ch);
7778

7879
struct Selector {
7980
bool timer_callback = false;
81+
bool executing = false;
8082
std::unordered_set<Socket *> active_sockets;
83+
std::unordered_set<Socket *> release_sockets;
8184
};
8285

8386
class Multi {

ext-src/swoole_curl.cc

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,13 @@ int Multi::del_event(void *socket_ptr, curl_socket_t sockfd) {
109109

110110
curl_socket->socket->fd = -1;
111111
curl_socket->socket->free();
112-
delete curl_socket;
112+
113+
if (selector.executing) {
114+
curl_socket->deleted = true;
115+
selector.release_sockets.insert(curl_socket);
116+
} else {
117+
delete curl_socket;
118+
}
113119

114120
return SW_OK;
115121
}
@@ -295,29 +301,44 @@ int Multi::handle_timeout(CURLM *mh, long timeout_ms, void *userp) {
295301
void Multi::selector_finish() {
296302
del_timer();
297303

304+
selector.executing = true;
305+
298306
if (selector.timer_callback) {
299307
selector.timer_callback = false;
300308
curl_multi_socket_action(multi_handle_, CURL_SOCKET_TIMEOUT, 0, &running_handles_);
301309
swoole_trace_log(SW_TRACE_CO_CURL, "socket_action[timer], running_handles=%d", running_handles_);
302310
}
303311

304-
for (auto curl_socket : selector.active_sockets) {
305-
/**
306-
* In `curl_multi_socket_action`, `Handle::destroy_socket()` may be invoked,
307-
* which will remove entries from the `unordered_map`.
308-
* In C++, removing elements during iteration can render the iterator invalid; hence,
309-
* it's necessary to copy `handle->sockets` into a new `unordered_map`.
310-
*/
311-
swoole_trace_log(SW_TRACE_CO_CURL,
312-
"curl_multi_socket_action(): sockfd=%d, bitmask=%d, running_handles_=%d",
313-
curl_socket->sockfd,
314-
curl_socket->bitmask,
315-
running_handles_);
316-
int bitmask = curl_socket->bitmask;
317-
curl_socket->bitmask = 0;
318-
curl_multi_socket_action(multi_handle_, curl_socket->sockfd, bitmask, &running_handles_);
312+
while (!selector.active_sockets.empty()) {
313+
auto active_sockets = selector.active_sockets;
314+
selector.active_sockets.clear();
315+
316+
for (auto curl_socket : active_sockets) {
317+
/**
318+
* In `curl_multi_socket_action`, `Handle::destroy_socket()` may be invoked,
319+
* which will remove entries from the `unordered_map`.
320+
* In C++, removing elements during iteration can render the iterator invalid; hence,
321+
* it's necessary to copy `handle->sockets` into a new `unordered_map`.
322+
*/
323+
swoole_trace_log(SW_TRACE_CO_CURL,
324+
"curl_multi_socket_action(): sockfd=%d, bitmask=%d, running_handles_=%d",
325+
curl_socket->sockfd,
326+
curl_socket->bitmask,
327+
running_handles_);
328+
329+
if (!curl_socket->deleted) {
330+
int bitmask = curl_socket->bitmask;
331+
curl_socket->bitmask = 0;
332+
curl_multi_socket_action(multi_handle_, curl_socket->sockfd, bitmask, &running_handles_);
333+
}
334+
}
335+
}
336+
337+
selector.executing = false;
338+
for (auto curl_socket : selector.release_sockets) {
339+
delete curl_socket;
319340
}
320-
selector.active_sockets.clear();
341+
selector.release_sockets.clear();
321342
}
322343

323344
long Multi::select(php_curlm *mh, double timeout) {

0 commit comments

Comments
 (0)