Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

curl_multi_fdset does not accept NULL sets #12691

Closed
sfan5 opened this issue Jan 13, 2024 · 1 comment
Closed

curl_multi_fdset does not accept NULL sets #12691

sfan5 opened this issue Jan 13, 2024 · 1 comment
Labels
documentation libcurl API

Comments

@sfan5
Copy link

sfan5 commented Jan 13, 2024

I did this

	int max_fd;
	mres = curl_multi_fdset(m_multi, nullptr, nullptr, nullptr, &max_fd);

The context here is that I am attempting to emulate curl_multi_poll on libcurl older than 7.66.0.
The idea I had is:

  • check if libcurl has any fds it waits for
  • if yes, just call curl_multi_wait
  • if no, sleep for the wanted timeout

This actually means reporting this bug is pointless since the fallback code I am writing can never make use of a fix. Maybe you still want to update the docs 🤷

I expected the following

No crash.

The documentation for curl_multi_fdset says (emphasis mine):

If the read_fd_set argument is not a null pointer, it points to an object of type fd_set that on returns specifies the file descriptors to be checked for being ready to read.
If the write_fd_set argument is not a null pointer, it points to an object of type fd_set that on return specifies the file descriptors to be checked for being ready to write.
If the exc_fd_set argument is not a null pointer, it points to an object of type fd_set that on return specifies the file descriptors to be checked for error conditions pending.

Checking the code reveals no attempt is made to check the sets against null:

curl/lib/multi.c

Lines 1178 to 1189 in d7b6ce6

for(i = 0; i < ps.num; i++) {
if(!FDSET_SOCK(ps.sockets[i]))
/* pretend it doesn't exist */
continue;
if(ps.actions[i] & CURL_POLL_IN)
FD_SET(ps.sockets[i], read_fd_set);
if(ps.actions[i] & CURL_POLL_OUT)
FD_SET(ps.sockets[i], write_fd_set);
if((int)ps.sockets[i] > this_max_fd)
this_max_fd = (int)ps.sockets[i];
}
}

As far as I know FD_SET(..., NULL) is not specified to be safe either.

curl/libcurl version

curl 8.5.0 (x86_64-pc-linux-gnu) libcurl/8.5.0 OpenSSL/3.2.0 zlib/1.3 brotli/1.1.0 zstd/1.5.5 libidn2/2.3.4 libpsl/0.21.2 (+libidn2/2.3.4) libssh2/1.11.0 nghttp2/1.58.0

operating system

Arch Linux x86_64
@bagder
Copy link
Member

bagder commented Jan 13, 2024

It seems it has not supported that since at least 2006 so, yes, let's fix the docs.

bagder added a commit that referenced this issue Jan 13, 2024
@bagder
curl_multi_fdset.3: remove mention of null pointer support
a9f3eca 
... since this funtion has not supported null pointer fd_set arguments since
at least 2006. (That's when I stopped my git blame journey)

Fixes #12691
Reported-by: sfan5 on github
@bagder bagder mentioned this issue Jan 13, 2024
Closed
@bagder bagder closed this as completed in beb2283 Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
documentation libcurl API
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

curl_multi_fdset.3: remove mention of null pointer support curl/curl
2 participants
@bagder @sfan5