Collecting dumps can cause process termination with "*** buffer overflow detected ***" message

[marctheisen]

When using this (great!) library I ran into a buffer overflow because my process was using more than 1024 file handles. The crash happened in signal_handler.cc, line 391/392:

FD_SET(pipe_fd[0], &read_fds);
FD_SET(timer_fd, &read_fds);

The 'fd_set' is a simple bitset supporting up to 1024 entries (i.e. file handle 0 .. file handle 1023).

The fix is quite simple. Instead of using fd_set / select() you can change the following code section to use pollfd / poll().

Just replace:

fd_set read_fds;
FD_ZERO(&read_fds);
FD_SET(pipe_fd[0], &read_fds);
FD_SET(timer_fd, &read_fds);
auto max_fd = std::max(pipe_fd[0], timer_fd) + 1;
auto ret = select(max_fd, &read_fds, nullptr, nullptr, nullptr);
if (ret == -1) {
  std::cerr << "select(...) failed, will try again" << std::endl;  // errno
} else if (ret == 0) {
  // We should never encounter this case as we use an infinite timeout in
  // the select syscall.
  std::cerr << "No file descriptors ready, will try again"
            << std::endl;  // errno
} else if (FD_ISSET(timer_fd, &read_fds)) {
  std::cerr << "Failed to get all (" << tids.size()
            << ") the stacktrace acks within timeout. Got only " << acks
            << std::endl;  // errno
  error->assign("Failed to get all (" + std::to_string(tids.size()) +
                ") stacktraces within timeout. Got only " +
                std::to_string(acks));
  return {};
} else if (FD_ISSET(pipe_fd[0], &read_fds)) {
  char ch;
  auto num_read = read(pipe_fd[0], &ch, sizeof(ch));
  if (-1 == num_read) {
    std::cerr << "Failed to read from pipe" << std::endl;
  } else if (sizeof(ch) != num_read) {
    std::cerr << "Read unexpected number of bytes. Expected: " << sizeof(ch)
              << ", got: " << num_read << std::endl;
  } else {
    ++acks;
  }
}

with

pollfd pipeAndTimerPollFd[2];
pipeAndTimerPollFd[0].fd = pipe_fd[0];
pipeAndTimerPollFd[0].events = POLLIN;
pipeAndTimerPollFd[1].fd = timer_fd;
pipeAndTimerPollFd[1].events = POLLIN;
const int noTimeout = -1;
auto ret = poll(pipeAndTimerPollFd, 2, noTimeout);
if (ret == -1)
{
  std::cerr << "poll(...) failed, will try again" << std::endl;
}
else if (ret == 0)
{
  // This should never happen as 0 means timeout but no timeout has been given!
  std::cerr << "poll() returned 0 even though no timeout was given, will try again" << std::endl;
}
else if(pipeAndTimerPollFd[1].revents == POLLIN)
{
  std::cerr << "Failed to get all (" << tids.size()
            << ") the stacktrace messages within timeout. Got only " << acks
            << std::endl;
  error->assign("Failed to get all (" + std::to_string(tids.size()) +
                ") stacktraces within timeout. Got only " +
                std::to_string(acks));
  return {};
}
else
{
  if(pipeAndTimerPollFd[0].revents != POLLIN)
  {
    std::cerr << "Error calling poll(), expected the pipe to be ready for reading!" << std::endl;
  } 
  
  char ch;
  auto num_read = read(pipe_fd[0], &ch, sizeof(ch));
  if (-1 == num_read)
  {
    std::cerr << "Failed to read from pipe" << std::endl;
  }
  else if (sizeof(ch) != num_read)
  {
    std::cerr << "Read unexpected number of bytes. Expected: " << sizeof(ch)
              << ", got: " << num_read << std::endl;
  }
  else
  {
    ++acks;
  }
}

Let me know if this project is still maintained and in this case I am happy to create a pull request.

Marc

[nipun-sehrawat]

Hi Marc, Thanks for fixing this issues. Yes, please create a PR. Thanks, Nipun

…

On Thu, Feb 15, 2024, 7:43 AM marctheisen ***@***.***> wrote: When using this (great!) library I ran into a buffer overflow because my process was using more than 1024 file handles. The crash happened in signal_handler.cc, line 391/392: FD_SET(pipe_fd[0], &read_fds); FD_SET(timer_fd, &read_fds); The 'fd_set' is a simple bitset supporting up to 1024 entries (i.e. file handle 0 .. file handle 1023). The fix is quite simple. Instead of using fd_set / select() you can change the following code section to use pollfd / poll(). Just replace: fd_set read_fds; FD_ZERO(&read_fds); FD_SET(pipe_fd[0], &read_fds); FD_SET(timer_fd, &read_fds); auto max_fd = std::max(pipe_fd[0], timer_fd) + 1; auto ret = select(max_fd, &read_fds, nullptr, nullptr, nullptr); if (ret == -1) { std::cerr << "select(...) failed, will try again" << std::endl; // errno } else if (ret == 0) { // We should never encounter this case as we use an infinite timeout in // the select syscall. std::cerr << "No file descriptors ready, will try again" << std::endl; // errno } else if (FD_ISSET(timer_fd, &read_fds)) { std::cerr << "Failed to get all (" << tids.size() << ") the stacktrace acks within timeout. Got only " << acks << std::endl; // errno error->assign("Failed to get all (" + std::to_string(tids.size()) + ") stacktraces within timeout. Got only " + std::to_string(acks)); return {}; } else if (FD_ISSET(pipe_fd[0], &read_fds)) { char ch; auto num_read = read(pipe_fd[0], &ch, sizeof(ch)); if (-1 == num_read) { std::cerr << "Failed to read from pipe" << std::endl; } else if (sizeof(ch) != num_read) { std::cerr << "Read unexpected number of bytes. Expected: " << sizeof(ch) << ", got: " << num_read << std::endl; } else { ++acks; } } with pollfd pipeAndTimerPollFd[2]; pipeAndTimerPollFd[0].fd = pipe_fd[0]; pipeAndTimerPollFd[0].events = POLLIN; pipeAndTimerPollFd[1].fd = timer_fd; pipeAndTimerPollFd[1].events = POLLIN; const int noTimeout = -1; auto ret = poll(pipeAndTimerPollFd, 2, noTimeout); if (ret == -1) { std::cerr << "poll(...) failed, will try again" << std::endl; } else if (ret == 0) { // This should never happen as 0 means timeout but no timeout has been given! std::cerr << "poll() returned 0 even though no timeout was given, will try again" << std::endl; } else if(pipeAndTimerPollFd[1].revents == POLLIN) { std::cerr << "Failed to get all (" << tids.size() << ") the stacktrace messages within timeout. Got only " << acks << std::endl; error->assign("Failed to get all (" + std::to_string(tids.size()) + ") stacktraces within timeout. Got only " + std::to_string(acks)); return {}; } else { if(pipeAndTimerPollFd[0].revents != POLLIN) { std::cerr << "Error calling poll(), expected the pipe to be ready for reading!" << std::endl; } char ch; auto num_read = read(pipe_fd[0], &ch, sizeof(ch)); if (-1 == num_read) { std::cerr << "Failed to read from pipe" << std::endl; } else if (sizeof(ch) != num_read) { std::cerr << "Read unexpected number of bytes. Expected: " << sizeof(ch) << ", got: " << num_read << std::endl; } else { ++acks; } } Let me know if this project is still maintained and in this case I am happy to create a pull request. Marc — Reply to this email directly, view it on GitHub <#5>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACD7YVUR24THEIDARMCTM3YTYUJPAVCNFSM6AAAAABDKPWE72VHI2DSMVQWIX3LMV43ASLTON2WKOZSGEZTMOBVGYZTQNA> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>