#35037 ipc: support per-address max-connections options on -ipcbind
https://github.com/bitcoin/bitcoin/pull/35037 · · +406/-17 in 12 files, 4 commits · labels: IPC
Goal
- Add per-listener connection limits and file descriptor reservations for IPC sockets
- Prevent runaway IPC connections from exhausting node file descriptors or starving P2P and RPC sockets
This pull request adds per-address connection limits to `-ipcbind` using inline socket options (e.g. `-ipcbind=unix:/path.sock,max-connections=8`). It integrates IPC connection limits into node initialization file descriptor accounting, enforcing a default limit of 16 connections per listener and a maximum limit of 256.
Problem: Previously, IPC listening sockets lacked connection limits and were omitted from initialization file descriptor reservations, creating a potential denial-of-service or crash risk via file descriptor exhaustion. Furthermore, a global connection limit would not accommodate distinct connection limits across different IPC listeners serving different roles.
Category: IPC / multiprocess (#4 of 20)
P2 · stability
- P2 because uncontrolled IPC connections could exhaust node file descriptors or trigger startup crashes
- Protects node resource allocation and establishes a pattern for upcoming IPC listener options
P2 because it closes an unconstrained file descriptor allocation hazard on IPC listening sockets and provides necessary capacity management for multiprocess listeners. As ryanofsky noted, 'This fixes a gap in descriptor accounting, and adds a helpful connection limiting feature' that allows independent limits for distinct consumers such as mining clients and GUI processes.
Membership: Changes IPC listening interfaces, protocol connection limits, and IPC-specific socket argument parsing.
Factors: security/stability 2, bug 1, performance 0, user value 2, leverage 1
Reviewability: Ready
- Ready to review: all previous feedback has been addressed with no open issues or blockers
Clean merge state, passing CI, and all reviewer concerns have been addressed with corresponding tests.
Author status: Active, promptly addressed reviewer feedback and updated commits.
Resolved concerns:
- Potential integer overflow and size_t underflow in startup file descriptor accounting calculations
- Option parsing ambiguities with Windows drive-letter paths and unix socket paths containing commas
- Overly permissive connection upper bounds that could abort during startup
- Loss of resolved socket paths in startup log messages
Agreement: Strong
- Strong consensus on attaching connection limits and file descriptor reservations to listeners
- Fixes a gap in descriptor accounting and adds helpful socket limits (ryanofsky)
- Verified connection limits locally using multiple client sockets (xyzconstant)
- Code approved without objections (Eunovo, ViniciusCestarii)
Strong consensus with multiple ACKs; all review suggestions on bounds, parsing, and arithmetic have been resolved.
The PR has received thorough review and approval from four contributors and members including IPC maintainer ryanofsky. Reviewers unanimously prefer per-listener configuration over a global option, and all raised edge cases and bug reports have been addressed with test coverage.
- ryanofsky: 'This fixes a gap in descriptor accounting, and adds a helpful connection limiting feature, and is nicely implemented.'
- Eunovo: 'reACK 78a1251365ab0dbfa9267c30760566d3b04bcd08'
- xyzconstant: 're-ACK 39b1c5a22c845857709d4e95d4205e0901e85b09 Nice changes'
- ViniciusCestarii: 'Re-ACK 39b1c5a22c845857709d4e95d4205e0901e85b09'
Objections:
| Reviewer | Kind | Harm | Status | Blocking | Author replied | Quote |
|---|---|---|---|---|---|---|
| xyzconstant | correctness | Underflow and overflow in file descriptor accounting arithmetic could cause node to bypass checks or assert-fail on startup. | resolved | yes | yes | 2026-07-22: 'size_t underflow lets the aggregate check pass... The node starts up with min_required_fds reduced by 2 instead of failing' Settled: 2026-09-12: 'Tested ACK 6f8f23866a92f7c74662429e29688dc060f83bd5... Overall, the code looks good to me.' |
| ryanofsky | interface | Using colons for socket options conflicted with Windows filesystem paths containing drive letters. | resolved | yes | yes | 2026-07-15: 'I do really think the ":" options separator should be replaced by "," to avoid problems with windows paths' Settled: 2026-07-28: 'Still I think it's nice that you switched to commas to match socat syntax more' |
| ViniciusCestarii | correctness | Absence of an upper bound check on connection limits could cause startup crashes, and resolved paths were lost in the listening log message. | resolved | yes | yes | 2026-06-26: 'I believe it would be worth to add an upper bound check too because large values flow into the int FD reservation and overflow it' Settled: 2026-09-15: 'Re-ACK 39b1c5a22c845857709d4e95d4205e0901e85b09' |
| Eunovo | correctness | Socket paths containing commas could be mistakenly parsed as invalid socket options. | resolved | no | yes | 2026-08-11: 'The current implementation will accept unix:path,backup.sock=1 as valid when it should fail with Unknown socket option' Settled: 2026-09-17: 'reACK 78a1251365ab0dbfa9267c30760566d3b04bcd08' |
| ViniciusCestarii | correctness | resolved socket path is lost in listener logging, leaving only bare argument in logs and InitError | resolved | no | yes | 2026-08-28: 'This regresses the listener log line: before: Listening for IPC requests on address unix:/home/vinicius/.bitcoin/regtest/node.sock after: Listening for IPC requests on address unix' Settled: 2026-09-01: 'ACK 7a839565cae797e87c43dc483066d47c2dffc568' |
| Sjors | scope | premature libmultiprocess subtree update included ahead of upstream merge causing CI lint failure | resolved | yes | yes | 2026-06-30: 'Also it looks like the subtree is based on https://github.com/bitcoin-core/libmultiprocess/pull/269, which is still open. So this PR should be draft.' Settled: 2026-07-11: 'Subtree PR adding the max connections PR has been merged and added to core https://github.com/bitcoin/bitcoin/pull/35684' |
Support:
- ryanofsky: Enables per-listener limits (e.g. distinguishing GUI vs mining clients) and fixes descriptor accounting.
- Eunovo: Thoroughly tested and reviewed socket limiting behavior and parser edge cases.
- xyzconstant: Exercised socket limiting and FD reservation edge cases on multiple platforms.
- ViniciusCestarii: Verified connection limiting behavior and resolved logging paths.
- Sjors: Approved approach for extensibility (e.g. future per-endpoint permissioning).
- kevkevinpal: Agreed inline options are cleaner than introducing global -ipcmaxconnections.
Participants: ryanofsky (objection), ViniciusCestarii (objection), kevkevinpal (support), fanquake (neutral), Sjors (objection), xyzconstant (objection), Eunovo (objection)
State derived from the lists: substantive support, no open objection (ryanofsky, Eunovo, xyzconstant, ViniciusCestarii, Sjors, kevkevinpal)
Review verdicts (DrahtBot): 1 (+3)
- ACK: Eunovo
- Stale ACK: ryanofsky, ViniciusCestarii, xyzconstant
- Concept ACK: kevkevinpal, Sjors
Files
123 lines under test/bench/ci.
- test/functional/interface_ipc_init.py +122/-0
- src/ipc/listen.cpp +91/-0
- src/ipc/test/ipc_tests.cpp +80/-0
- src/init.cpp +44/-8
- src/ipc/types.h +40/-0
- src/interfaces/ipc.h +11/-3
- doc/release-notes-35037.md +7/-0
- src/ipc/interfaces.cpp +4/-3
- src/ipc/capnp/protocol.cpp +2/-2
- src/ipc/protocol.h +3/-1
- src/CMakeLists.txt +1/-0
- test/functional/test_runner.py +1/-0
Card
PR #35037 allows setting per-listener connection limits on -ipcbind using comma-delimited options like max-connections=N, defaulting to 16 and capped at 256. It incorporates IPC listeners and client connection slots into startup file descriptor reservation checks, preventing unconstrained IPC connections from starving the node of descriptors. The changes enjoy broad review and strong support from ryanofsky, Eunovo, xyzconstant, and ViniciusCestarii, with all raised bounds checks and parser bugs resolved.