privsep: Ensure we recv for real after a successful recv MSG_PEEK

[rsmarples]

Adjust the code flow so that the same errors would be caught after the final recv. This ensures we read what is really meant for us and not something silly.

Should fix #555.

[coderabbitai[bot]]

Walkthrough

Reworked privsep message I/O to a two-phase recvmsg flow (peek sizing then mandatory consume), moved per-message buffers to optional heap allocation, removed per-process data cleanup hooks, and removed two fields from the public process struct.

Changes

Cohort / File(s)	Summary
Privsep root I/O and buffer handling src/privsep-root.c	Replaced single MSG_PEEK read with msghdr/iovec + recvmsg(MSG_PEEK
Process struct and cleanup behavior src/privsep.h, src/privsep.c	Removed psp_data and psp_freedata fields from struct ps_process; removed runtime cleanup of psp_data in ps_freeprocess, so per-process data callback/path no longer invoked during process free.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Review focus:
  • Correctness of new two-step recvmsg control flow and guarantee that peeked messages are always consumed (including on error paths).
  • Memory semantics around psr_mallocdata / psr_data allocation, ownership, and absence of the previous in-place mdata lifecycle.
  • Error mapping and logging for MSG_TRUNC, short reads, and total-length mismatches.
  • Effects of removing psp_data / psp_freedata and skipping per-process data cleanup on callers and lifecycle (ensure no leaked resources or dangling pointers).
  • Call sites to ps_root_writeerror and other modified APIs to confirm consistent zero-length payload handling.

Possibly related PRs

• NetworkConfiguration/dhcpcd#533 — Similar privsep IPC adjustments (switch to msghdr/recvmsg with MSG_WAITALL/MSG_EOR and modified iovec/msg handling); likely directly related to the recvmsg/iovec/error-handling changes in this PR.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: ensuring a real recv occurs after MSG_PEEK, which aligns with the core code modifications switching from simple MSG_PEEK to a two-phase recv approach.
Description check	✅ Passed	The description is related to the changeset, explaining that the code flow is adjusted to re-check error conditions after a real recv following MSG_PEEK, which matches the implementation details.
Linked Issues check	✅ Passed	The PR implements a two-phase recv approach (MSG_PEEK followed by real recv) that addresses the buffer handling issue reported in #555 by ensuring proper error detection on the final recv path.
Out of Scope Changes check	✅ Passed	All changes are directly related to the recv mechanism and buffer handling objectives: modifications to privsep-root.c for two-phase recv, removal of cleanup callbacks in privsep.c/h consistent with the new allocation strategy.

✨ Finishing touches

• [ ] 📝 Generate docstrings

🧪 Generate unit tests (beta)

• [ ] Create PR with unit tests
• [ ] Post copyable unit tests in a comment
• [ ] Commit unit tests in branch privsep-recvmsg

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd75b08061c21b6bdffa50aa6ad9c92c1f385490 and 16a0bfbdd6f5ba3b06a5ece1e7dd7fc95629a9f5.

📒 Files selected for processing (3)

• src/privsep-root.c (6 hunks)
• src/privsep.c (0 hunks)
• src/privsep.h (0 hunks)

💤 Files with no reviewable changes (2)

• src/privsep.h
• src/privsep.c

🚧 Files skipped from review as they are similar to previous changes (1)

• src/privsep-root.c

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)

• GitHub Check: netbsd (--disable-arp, -DSMALL)
• GitHub Check: netbsd (--disable-ipv4ll)
• GitHub Check: netbsd (--disable-ipv4ll, -DSMALL)
• GitHub Check: netbsd (--disable-dhcp6, -DSMALL)
• GitHub Check: netbsd (--disable-ipv6)
• GitHub Check: netbsd (--disable-dhcp6)
• GitHub Check: netbsd (--disable-ipv6, -DSMALL)
• GitHub Check: netbsd (--disable-arp)
• GitHub Check: netbsd (--disable-ipv4, -DSMALL)
• GitHub Check: netbsd (-DSMALL)
• GitHub Check: netbsd (--disable-ipv4)
• GitHub Check: netbsd
• GitHub Check: freebsd
• GitHub Check: openbsd

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[perkelix]

Does this one seem ready to merge too?

[rsmarples]

Does this one seem ready to merge too?

I would like some feedback on it first as I still don't know if it fixes people issues as I cannot replicate the problem.