[dshett549]

Implement per-process Unix datagram socket replacing UDP broadcast

[Copilot]

🟡 Changes recommended

Unresolved socket permission and lifecycle defects, process-name lookup failures, and blocking sends affect security and reliability.

6 open findings

• Missing sticky-bit requirement permits socket pathname tampering · New
• Shared directory permissions depend incorrectly on application umask · New
• Process-wide umask changes race with application threads · New
• Forked child can unlink the parent’s live control socket · New
• Short /proc read rejects processes with long executable paths · New
• Blocking datagram sends can hang when the application is idle · New

What changed in this PR

Replaces UDP broadcast runtime log control with per-process Unix datagram sockets to harden RDK Logger.

Changes:

• Adds socket permission checks and process-specific lifecycle handling.
• Changes rdklogctrl to target PIDs.
• Updates tests and usage documentation.

File	Description
utils/​rdklogctrl.c	Resolves PIDs and sends Unix datagrams.
unittests/​rdkLoggerDynamic.cpp	Updates PID targeting and strengthens assertions.
src/​rdk_dynamic_logger.c	Implements per-process sockets and cleanup.
src/​include/​rdk_dynamic_logger.h	Removes the unused message-length field.
docs/​TROUBLESHOOTING.md	Updates commands and socket diagnostics.
docs/​QUICK_REFERENCE.md	Documents PID-based control.
docs/​API_DOCUMENTATION.md	Updates control syntax and transport details.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Socket permission timing permits unauthorized requests, and shared-directory ownership can prevent control sockets from being created.

4 open findings

• Socket permissions allow unauthorized control packets · New
• Shared logger directory ownership blocks other UIDs · New
• Blocking datagram sends can hang when the application is idle
• Short /proc read rejects processes with long executable paths

4 resolved since last review

• Missing sticky-bit requirement permits socket pathname tampering
• Forked child can unlink the parent’s live control socket
• Process-wide umask changes race with application threads
• Shared directory permissions depend incorrectly on application umask

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

The removed license-comment opener prevents the logger source from compiling.

2 open findings

• Restore missing opening license comment delimiter · New
• Shared logger directory ownership blocks other UIDs

3 resolved since last review

• Socket permissions allow unauthorized control packets
• Blocking datagram sends can hang when the application is idle
• Short /proc read rejects processes with long executable paths

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Socket naming rejects supported process names, and initialized forked children remain unreachable until they log.

0 open findings

2 resolved since last review

• Restore missing opening license comment delimiter
• Shared logger directory ownership blocks other UIDs

Previously missed (3)

In code that hasn't changed since last review

Prevent long process names from exceeding Unix socket path limits

src/​rdk_dynamic_logger.c:72

A 100-character process name with module LOG.RDK.TEST fits the existing 128-byte packet limit (120 bytes total), but /tmp/.logger/<name>.<pid> exceeds Linux's 108-byte sun_path even with a one-digit PID. The receiver therefore cannot initialize, and the matching construction in utils/rdklogctrl.c rejects the target too. Use a bounded filename, such as PID-only, consistently in the receiver, controller, and documentation so previously supported names remain controllable.

Initialize child control socket before its first log call

src/​rdk_dynamic_logger.c:137

After an initialized application forks or daemonizes, the child has no PID-specific socket until its first log call reaches this branch. Calling rdk_logger_init() in the child does not help: src/rdk_logger_init.c:61 skips initialization because isLogInited is inherited. Thus rdklogctrl targeting an initialized but idle child fails with ENOENT rather than queuing a level change. Establish the child's endpoint through PID-aware post-fork initialization or an explicit lifecycle hook, and test sending a request before the child's first log call.

Restore 64-byte component-name boundary regression test

unittests/​rdkLoggerDynamic.cpp:367

This removes the positive 64-byte component-name regression test while retaining oversized-name rejection tests. An off-by-one change that rejects the maximum valid name would now go undetected. Restore a PID-based boundary test that sends a 64-byte name and verifies that the receiver processes the update.

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Child socket initialization and inconsistent log-level lookup leave runtime control and getter behavior incorrect.

2 open issues

• Consistently resolve inherited priorities for child categories · New
• Handle both NONE priority representations in config parsing · New

1 previously missed issue

In code that hasn't changed since last review

Publish child control sockets after fork without requiring logging

src/​rdk_dynamic_logger.c:137

After logger initialization followed by fork/daemonization, the child has no PID-specific endpoint until a log call reaches this branch. During that interval, rdklogctrl <child-pid> ... fails with ENOENT rather than queuing a change. Calling rdk_logger_init() again does not create the endpoint because the child inherits isLogInited == true. Provide a post-fork initialization path that publishes the child's socket without requiring a log message first.

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Forked children remain unreachable by runtime log control until their first logging call.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Publish child control endpoint immediately after fork

src/​rdk_dynamic_logger.c:137

When an initialized process forks or daemonizes, the child's PID-specific socket is not published until a logging call reaches this branch. Until then, rdklogctrl <child-pid> ... fails with ENOENT instead of queuing an update. Calling rdk_logger_init() again does not help because the child inherits isLogInited == true. Provide a supported post-fork initialization path that publishes the child's endpoint without requiring a log call, and test control of a child before it logs.

2 resolved since last review

• Handle both NONE priority representations in config parsing
• Consistently resolve inherited priorities for child categories

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Security-sensitive socket access and fork lifecycle behavior need additional regression coverage and human review.

3 open issues

• Publish control socket after fork without requiring a log call · New
• Add regression tests for socket ownership and access control · New
• Restore the successful 64-character component boundary test · New

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Unresolved message-validation, socket-authorization, and lifecycle defects prevent safe deployment.

10 open issues

• Reject datagrams with unterminated name fields · New
• Clean up socket and unlock mutex on path-length failure · New
• Enforce owner-only authorization for the control socket · New
• Safely remove stale pathname sockets during lifecycle · New
• Preserve Receiver module name exception · New
• Preserve accepted 64-character component and application names · New
• Restore the successful 64-character component boundary test
• Add regression tests for socket ownership and access control
• Publish control socket after fork without requiring a log call
• Consistently resolve inherited priorities for child categories

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

The security-sensitive transport migration needs human validation of message handling, access controls, and socket lifecycle before approval.

12 open issues

• Enforce owner-only authorization for the control socket
• Clean up socket and unlock mutex on path-length failure
• Reject datagrams with unterminated name fields
• Preserve accepted 64-character component and application names
• Preserve Receiver module name exception
• Safely remove stale pathname sockets during lifecycle
• Restore the successful 64-character component boundary test
• Add regression tests for socket ownership and access control
• Publish control socket after fork without requiring a log call
• Correct documented socket path and access claims · New
• Update documentation for PID-based socket transport · New
• Fix malformed socket-permission command example · New

1 resolved since last review

• Consistently resolve inherited priorities for child categories

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Predictable endpoints in world-writable storage and boundary-length name truncation can disable or misdirect runtime log control.

10 open issues

• Enforce owner-only authorization for the control socket
• Clean up socket and unlock mutex on path-length failure
• Use a trusted protected directory for control sockets · New
• Restore terminator space for boundary-length names · New
• Preserve accepted 64-character component and application names
• Preserve Receiver module name exception
• Safely remove stale pathname sockets during lifecycle
• Restore the successful 64-character component boundary test
• Add regression tests for socket ownership and access control
• Publish control socket after fork without requiring a log call

4 resolved since last review

• Reject datagrams with unterminated name fields
• Fix malformed socket-permission command example
• Update documentation for PID-based socket transport
• Correct documented socket path and access claims

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

The transport migration needs human confirmation that previously reported lifecycle, access-control, and compatibility concerns are resolved.

10 open issues

• Clean up socket and unlock mutex on path-length failure
• Restore terminator space for boundary-length names
• Use a trusted protected directory for control sockets
• Restore the successful 64-character component boundary test
• Add regression tests for socket ownership and access control
• Publish control socket after fork without requiring a log call
• Document the AF_UNIX socket endpoint correctly · New
• Fix Mermaid socket participant name · New
• Update integration table for Unix-domain sockets · New
• Correct documentation of sender authentication · New

4 resolved since last review

• Enforce owner-only authorization for the control socket
• Preserve accepted 64-character component and application names
• Preserve Receiver module name exception
• Safely remove stale pathname sockets during lifecycle

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

The review evidence identifies unresolved runtime-control compatibility and socket-lifecycle regressions that block approval.

3 open issues

• Restore terminator space for boundary-length names
• Use a trusted protected directory for control sockets
• Publish control socket after fork without requiring a log call

7 resolved since last review

• Clean up socket and unlock mutex on path-length failure
• Restore the successful 64-character component boundary test
• Add regression tests for socket ownership and access control
• Correct documentation of sender authentication
• Update integration table for Unix-domain sockets
• Fix Mermaid socket participant name
• Document the AF_UNIX socket endpoint correctly

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

The transport migration needs human validation of socket lifecycle behavior and compatibility before approval.

1 open issue

• Publish control socket after fork without requiring a log call

2 resolved since last review

• Restore terminator space for boundary-length names
• Use a trusted protected directory for control sockets

_{Review effort: Balanced}