run_command: Close non-std fds when execing slave processes #30756

pull luke-jr wants to merge 3 commits into bitcoin:master from luke-jr:subproc_closefds changing 2 files +39 −1
  1. luke-jr commented at 8:11 pm on August 29, 2024: member

    Currently, RunCommandParseJSON runs its target with whatever fds happen to be open inherited on POSIX platforms. I don’t think there’s any practical scenario where this is a problem right now, but there’s a lot of potential for weird problems (eg, if a process manages to outlive bitcoind - perhaps it’s hanging - the listening port(s) won’t get released and starting bitcoind again will fail). It’s also a potential security issue if a child process is intended to be sandboxed at some point. Not to mention plain ugly :)

    cpp-subprocess has a feature to address this called close_fds. Not sure why it was removed in #29961 rather than fixing this during the migration, but this PR restores it, enables it for RunCommandParseJSON, and optimises it by iterating over /proc/self/fd/ like most other libraries do these days (eg, glib) since iterating all possible fd numbers has been found to be problematic.

    (Equivalent to #22417 was for boost::process)

  2. DrahtBot commented at 8:11 pm on August 29, 2024: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    Conflicts

    No conflicts as of last run.

  3. luke-jr force-pushed on Aug 29, 2024
  4. DrahtBot added the label CI failed on Aug 29, 2024
  5. luke-jr force-pushed on Aug 29, 2024
  6. Revert "remove unneeded close_fds option from cpp-subprocess"
    This reverts commit 79c30363733503a1fb7d4c98aa0d56ced0be6e32.
    790e2f948a
  7. cpp-subprocess: Iterate through /proc/self/fd for close_fds option 18adcaaae6
  8. run_command: Enable close_fds option to avoid lingering fds 28f6d4fa7e
  9. luke-jr force-pushed on Aug 30, 2024
  10. luke-jr commented at 3:30 am on August 30, 2024: member
    (Tangent: CI passed with 1e6b0f3ec3a878efb96019f40185809ed2b0699e, but it appears there’s a CI bug making it fail with the equivalent 28f6d4fa7eb1b0da16ee940922c5c466da2ec0ec)
  11. DrahtBot removed the label CI failed on Aug 30, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-20 01:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me