UCT/IB/MLX5: harden CoCo DEVX validators by nbellalou · Pull Request #11587 · openucx/ucx

nbellalou · 2026-06-25T14:53:28Z

What?

Draft PR. This should be merged after PR #11586.

Harden CoCo RC mlx5 policy gating.
Harden CoCo DEVX memory key validation.
Validate DEVX object IDs returned by the untrusted device.

Why?

After the CoCo control-object path is available, UCX needs to fail closed when DEVX-visible metadata or object identifiers do not match the private UCX state.

How?

Restrict the CoCo profile to the audited RC mlx5 path.
Validate MKey bounds and permissions against private metadata.
Validate DEVX object outputs before UCX trusts them.

Add configure checks for the rdma-core and kernel APIs used by the CoCo control-object path. The checks cover CC_DMA_BOUNCE detection, parent-domain attributes, CQ parent-domain support, dma-heap headers and DEVX dmabuf UMEM registration so the new code is compiled only when the headers provide the required interfaces.

Detect devices that report IBV_DEVICE_CC_DMA_BOUNCE during MD open and allocate a CoCo parent domain for UCX-owned RDMA control objects. Keep the existing PD as the default path and expose helpers that select the control PD only when CoCo bounce DMA is active.

Create verbs QPs, CQs and SRQs through the CoCo control PD when the device requires bounce DMA. Use the extended CQ creation path with parent-domain attributes in CoCo mode and make the RC verbs capability probe follow the same allocation rules.

Pass CoCo allocation attributes to mlx5 parent domains and allocate DEVX UMEM-backed control buffers from system_cc_shared when CC_DMA_BOUNCE is reported. Route mlx5 tag-matching control QPs through the control PD and reject mlx5 transports or modes that are not made CoCo control-object safe in this milestone.

Keep DEVX doorbell record bookkeeping outside memory exposed to an untrusted CoCo device. Register only the doorbell record page with DEVX, while keeping UCX metadata in private memory.

nbellalou · 2026-06-25T15:01:48Z

@svc-nvidia-pr-review

svc-nvidia-pr-review · 2026-06-25T15:01:52Z

🤖 Starting review — findings will be posted here when done.

svc-nvidia-pr-review

Code Review

17 findings

16 findings posted as inline comments.

General Findings

blocker: PR size and scope. The PR adds ~3270 lines across 17 files, well over the 500-line hard limit in REVIEW.md. It also bundles at least three independently reviewable changes that should be separate PRs:

Base IB CoCo plumbing (cc_dma_bounce detection, coco_pd, parent-domain CQ path).
mlx5 CoCo control-object validation/registry (ib_mlx5_coco.[ch], DEVX in/out validation, shared-alloc backend, rc_mlx5 policy gating).
Unrelated dbrec layout refactor (uct_ib_mlx5_dbrec.db from inline [2] to *db, new uct_ib_mlx5_add_page/init_dbrec/free_page shape). This applies to all non-CoCo paths and is not needed by anything in the diff (CoCo paths already get their own umem via the shared-alloc backend). Per REVIEW.md, feature PRs must not refactor unrelated code. Please pull this out into its own PR with a stated motivation.

The new code also has no CI job exercising the CoCo-enabled build (no buildlib/tools/builds.sh or contrib/test_jenkins.sh changes), and the gtest file only covers helpers via mocks. Per REVIEW.md, new configure flags should add an explicit enable/disable CI path.

svc-nvidia-pr-review · 2026-06-25T15:08:44Z

+
+static UCS_F_ALWAYS_INLINE int uct_ib_md_is_cc_dma_bounce(const uct_ib_md_t *md)
+{
+    return md->cc_dma_bounce;


Suggested change

return md->cc_dma_bounce;

static UCS_F_ALWAYS_INLINE int

uct_ib_md_is_coco_hardened(const uct_ib_md_t *md)

{

return md->cc_dma_bounce;

}

this function is identical to uct_ib_md_is_cc_dma_bounce: it reads md->cc_dma_bounce, asserts !enabled || md->cc_dma_bounce (tautology), and returns it. either drop one of the two helpers, or actually compute a different predicate (e.g. cc_dma_bounce && coco_pd != NULL). the docs/hardening/coco-security-properties.md text claims these are separate concepts but the code does not implement that.

svc-nvidia-pr-review · 2026-06-25T15:08:44Z

+
+static UCS_F_ALWAYS_INLINE struct ibv_pd*
+uct_ib_md_control_pd(const uct_ib_md_t *md)
+{


hardcoding the "rc_mlx5" policy in a base IB header (and pulling <string.h> into a widely-included header) inverts the layering — base shouldn't know which mlx5 TL is the allowed one. can we move this to the mlx5 side, or gate via a per-TL flag set at TL registration?

svc-nvidia-pr-review · 2026-06-25T15:08:44Z

+
+    ucs_assert(!enabled || md->cc_dma_bounce);
+    return enabled;
+}


ucs_assert(!md->cc_dma_bounce || (md->coco_pd != NULL)) followed by (md->cc_dma_bounce && (md->coco_pd != NULL)) ? coco_pd : pd makes the runtime check redundant with the assert (which is debug-only). if the invariant truly holds, drop the runtime guard; if it doesn't, the assert is wrong. having both is misleading.

svc-nvidia-pr-review · 2026-06-25T15:08:44Z

+    struct ibv_cq *cq;
+    int saved_errno;
+
+    cq_ex = ibv_create_cq_ex(md->dev.ibv_context, &cq_attr);


if uct_ib_md_create_coco_pd succeeds but uct_ib_md_probe_coco_control_cq fails, you jump to err_cleanup_device and leak md->coco_pd until uct_ib_md_free. that happens to clean it up because uct_ib_md_free checks coco_pd != NULL, but only if the caller reaches that path. pls add an err_dealloc_coco_pd: label and unwind in reverse order per docs/CodeStyle.md.

svc-nvidia-pr-review · 2026-06-25T15:08:44Z

+
    return UCS_OK;
 }



this dbrec layout split (separate ucs_calloc private page + dedicated db umem) is unrelated to CoCo and applies to all paths. it doubles the per-page allocations and changes umem semantics for every dbrec consumer. pls move to a separate PR with its own justification; the CoCo path goes through uct_ib_mlx5_coco_md_buf_alloc_shared and does not need this.

svc-nvidia-pr-review · 2026-06-25T15:08:45Z

    ucs_trace_func("");

+    if (uct_ib_md_is_coco_hardened(&md->super)) {
+        ucs_error("%s: %s is not CoCo control-object safe in this milestone",


the message says "in this milestone" but UCX has no concept of milestones in error output. pls phrase as a stable runtime reason (e.g. "%s does not support CoCo hardening").

svc-nvidia-pr-review · 2026-06-25T15:08:45Z


+    if (uct_ib_md_is_coco_hardened(&md->super)) {
+        ucs_error("%s: %s is not CoCo control-object safe in this milestone",
+                  uct_ib_device_name(&md->super.dev), "ud_mlx5");


same here — "in this milestone" is not a stable user-facing phrasing. pls rephrase as a capability statement (e.g. "%s does not support CoCo hardening").

svc-nvidia-pr-review · 2026-06-25T15:08:45Z

+
+When `uct_ib_md_is_coco_hardened()` is true, CoCo hardening is intended to
+protect local memory safety for UCX-owned control paths. The hardened path must
+preserve object lifetime, descriptor ownership, queue bounds, slot bounds, and


sentence-initial lowercase in "private UCX request state is...", "capability queries are availability hints..." reads as typos. pls capitalize.

svc-nvidia-pr-review · 2026-06-25T15:08:45Z

@@ -0,0 +1,923 @@
+/**


these tests exercise helpers (mkey/umem/obj registry, capability masking) by constructing uct_ib_mlx5_md_t on the stack with cc_dma_bounce = 1. there is no test that actually drives an RC mlx5 iface through the CoCo path on the mock or real HW, so the integration is uncovered. can we add a parametrized fixture in test_rc.cc/test_devx.cc that runs once with cc_dma_bounce forced on (e.g. via coco_set_shared_alloc_ops + a test-only md flag)?

svc-nvidia-pr-review · 2026-06-25T15:08:45Z


+       AC_CHECK_HEADERS([linux/dma-heap.h])
+
+       AC_CHECK_DECLS([IBV_DEVICE_CC_DMA_BOUNCE,


mlx5dv_devx_umem_reg_ex is a function, not an enum; AC_CHECK_DECLS works but AC_CHECK_FUNCS is the more idiomatic check used elsewhere in this file. minor consistency.

Fix CoCo MD error cleanup, quiet expected unsupported probes, share the cc-dma-bounce transport gate, and add behavioral DEVX dbrec coverage. Also apply fork handling to the dmabuf-backed shared control-object mapping path.

Gate CoCo-capable memory domains to the rc_mlx5 profile and keep unsupported IB transports hidden when cc_dma_bounce is active. Fail closed on unsafe rc_mlx5 configuration paths, force safe effective defaults, and mask RMA/atomic capabilities until the later MR and mkey milestones harden them. Document the current CoCo security properties and add gtest coverage for the policy/query/config guards.

Add CoCo-only shared allocation bookkeeping, UMEM authority tracking, and MR/MKey bound validation for mlx5 DEVX memory paths. Keep device-visible allocation metadata private and fail closed for CoCo mkey import/export until bounded metadata is available. Extend test_coco_hardening gtest coverage for shared allocation zeroing/scrubbing, UMEM duplicate and widening checks, MR/MKey bounds, permission narrowing, and the non-CoCo path.

Add CoCo CQ/QP/RMP request records and private object registries for DEVX lifecycle validation. Validate DEVX output length, status, and ID uniqueness before publishing CQN/QPN/RMPN into UCX state. Reject unsupported CoCo object profiles before creation. Add gtest coverage for guarded short outputs, duplicate IDs, invalid object profiles, and wrapper publish-order checks.

nbellalou added 5 commits June 25, 2026 17:24

UCT/IB/MLX5: keep DEVX dbrec metadata private

354bd9b

Keep DEVX doorbell record bookkeeping outside memory exposed to an untrusted CoCo device. Register only the doorbell record page with DEVX, while keeping UCX metadata in private memory.

nbellalou mentioned this pull request Jun 25, 2026

UCT/IB/MLX5: harden CoCo RC runtime checks #11588

Draft

svc-nvidia-pr-review reviewed Jun 25, 2026

View reviewed changes

nbellalou added 5 commits June 28, 2026 11:42

UCT/IB/MLX5: PR fixes

5888481

Fix CoCo MD error cleanup, quiet expected unsupported probes, share the cc-dma-bounce transport gate, and add behavioral DEVX dbrec coverage. Also apply fork handling to the dmabuf-backed shared control-object mapping path.

UCT/IB/MLX5: PR fixes

42cfd68

nbellalou force-pushed the coco/devx-validators branch from 4e507c2 to 42cfd68 Compare July 1, 2026 09:58


		AC_CHECK_HEADERS([linux/dma-heap.h])

		AC_CHECK_DECLS([IBV_DEVICE_CC_DMA_BOUNCE,

Uh oh!

Conversation

nbellalou commented Jun 25, 2026

What?

Why?

How?

Uh oh!

nbellalou commented Jun 25, 2026

Uh oh!

svc-nvidia-pr-review commented Jun 25, 2026

Uh oh!

svc-nvidia-pr-review left a comment

Choose a reason for hiding this comment

Code Review

General Findings

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants