Skip to content

Commit 56e1791

Browse files
sekharp-genesysliviuchircu
authored andcommitted
rest_client: guard fd selection against libcurl wakeup eventfd
curl_multi_fdset()'s read set can include libcurl's own wakeup eventfd alongside the real transfer socket (observed on 8.21.0, not 8.17.0). is_new_transfer() only checks fd numbers, not fd type, so it could register the eventfd instead of the socket. Add a SO_TYPE check before trusting a candidate fd. Also zero wset/eset before curl_multi_fdset(), and improve the sanity check on `connect_poll_interval` modparam. (cherry picked from commit 54edbe6)
1 parent 777caad commit 56e1791

2 files changed

Lines changed: 27 additions & 2 deletions

File tree

‎modules/rest_client/rest_client.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ static int mod_init(void)
242242
connection_timeout_ms = connection_timeout * 1000L;
243243
_async_resume_retr_timeout = curl_timeout * 1000000U;
244244

245-
if (connect_poll_interval < 0) {
245+
if (connect_poll_interval <= 0) {
246246
LM_ERR("Bad connect_poll_interval (%ldms), setting to 20ms\n",
247247
connect_poll_interval);
248248
connect_poll_interval = 20;

‎modules/rest_client/rest_methods.c‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,26 @@ static inline char del_transfer(int fd)
345345
return -1;
346346
}
347347

348+
/*
349+
* guard against registering a non-socket fd. libcurl's
350+
* internal wakeup eventfd can appear in curl_multi_fdset()'s read set
351+
* alongside the real transfer socket (observed on 8.21.0, not 8.17.0);
352+
* is_new_transfer() only checks fd numbers, not fd type.
353+
*
354+
* Note: CURLINFO_ACTIVESOCKET is not a usable alternative here - at this
355+
* point the easy handle is fresh out of curl_easy_init(), so it always
356+
* reads back CURL_SOCKET_BAD. A cleaner long-term fix would be adopting
357+
* CURLMOPT_SOCKETFUNCTION, which reports fds with an explicit type instead
358+
* of relying on fd_set scanning.
359+
*/
360+
static inline int rest_fd_is_socket(int fd)
361+
{
362+
int type;
363+
socklen_t len = sizeof(type);
364+
365+
return getsockopt(fd, SOL_SOCKET, SO_TYPE, &type, &len) == 0;
366+
}
367+
348368
static OSS_CURLM *get_multi(void)
349369
{
350370
OSS_CURLM *multi_list;
@@ -963,6 +983,8 @@ int start_async_http_req(struct sip_msg *msg, enum rest_client_method method,
963983
}
964984

965985
FD_ZERO(&rset);
986+
FD_ZERO(&wset);
987+
FD_ZERO(&eset);
966988
mrc = curl_multi_fdset(multi_handle, &rset, &wset, &eset, &max_fd);
967989
if (mrc != CURLM_OK) {
968990
LM_ERR("curl_multi_fdset: %s\n", curl_multi_strerror(mrc));
@@ -973,7 +995,8 @@ int start_async_http_req(struct sip_msg *msg, enum rest_client_method method,
973995
for (fd = 0; fd <= max_fd; fd++) {
974996
if (FD_ISSET(fd, &rset)) {
975997
LM_DBG("ongoing transfer on fd %d\n", fd);
976-
if ((connect > 0 || req_sz > 0) && is_new_transfer(fd)) {
998+
if ((connect > 0 || req_sz > 0) && is_new_transfer(fd) &&
999+
rest_fd_is_socket(fd)) {
9771000
LM_DBG(">>> add fd %d to ongoing transfers\n", fd);
9781001
add_transfer(fd);
9791002
goto success;
@@ -1113,6 +1136,8 @@ static enum async_ret_code _resume_async_http_req(int fd, struct sip_msg *msg,
11131136
}
11141137

11151138
FD_ZERO(&rset);
1139+
FD_ZERO(&wset);
1140+
FD_ZERO(&eset);
11161141
mrc = curl_multi_fdset(multi_handle, &rset, &wset, &eset, &max_fd);
11171142
if (mrc != CURLM_OK) {
11181143
LM_ERR("curl_multi_fdset: %s\n", curl_multi_strerror(mrc));

0 commit comments

Comments
 (0)