Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

daemon: preserve CDI additional GIDs#52579

Merged
thaJeztah merged 1 commit into
moby:mastermoby/moby:masterfrom
elezar:fix-cdi-additional-gidselezar/moby:fix-cdi-additional-gidsCopy head branch name to clipboard
May 12, 2026
Merged

daemon: preserve CDI additional GIDs#52579
thaJeztah merged 1 commit into
moby:mastermoby/moby:masterfrom
elezar:fix-cdi-additional-gidselezar/moby:fix-cdi-additional-gidsCopy head branch name to clipboard

Conversation

@elezar

@elezar elezar commented May 8, 2026

Copy link
Copy Markdown
Contributor

- What I did

Ensured that the addition of the user to the OCI spec runs before CDI injection. This is required to allow additional (non-root) group IDs to be added to the OCI spec if required for device nodes, for example. One case where this is applicable is on certain NVIDIA systems where device nodes have 660 permisions and are owned by root:video:

$ ls -al /dev/nvmap
crw-rw---- 1 root video 10, 124 Aug 26  2025 /dev/nvmap

This injection is equivalent with running docker with --group-add.

- How I did it

Reordered the functional modifers of the OCI spec so that WithUsers is called before WithDevices (which triggers CDI device injection).

- How to verify it

Create the following CDI spec at /var/run/cdi/additional-gid-test.yaml:

cdiVersion: "0.7.0"
kind: "example.com/device"
devices:
- name: foo
  containerEdits:
    additionalGids:
    - 1234

(also specified in the tests).

Running id in a container requesting the device should show the additional group:

$ docker run --rm -ti --device=example.com/device=foo alpine id
uid=0(root) gid=0(root) groups=0(root),0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel),11(floppy),20(dialout),26(tape),27(video),1234

(this is equivalent to running --group-add:

$ docker run --rm -ti --group-add 1234 alpine id
uid=0(root) gid=0(root) groups=0(root),0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel),11(floppy),20(dialout),26(tape),27(video),1234

(remove the created file).

- Human readable description for the release notes

Fix support for CDI specifications that request additional group IDs.

- A picture of a cute animal (not mandatory but encouraged)

@github-actions github-actions Bot added the area/daemon Core Engine label May 8, 2026
Comment thread daemon/oci_linux.go
WithNamespaces(daemon, c),
WithCapabilities(c),
WithSeccomp(daemon, c),
withMounts(daemon, daemonCfg, c, mounts),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just noted that withMounts is called after WithDevices which could also inject mounts over CDI. We may need to investigate whethere there are edge cases here that may lead to unexpected behaviour, but I suppost having command line mounts preferred over CDI mounts is fine.

@thaJeztah thaJeztah added the dco/no Automatically set by a bot when one of the commits lacks proper signature label May 8, 2026
@thaJeztah thaJeztah added this to the 29.5.0 milestone May 8, 2026
@vvoland vvoland added the kind/bugfix PR's that fix bugs label May 8, 2026
CDI injection runs from WithDevices and may append supplementary groups to process.user.additionalGids. Docker's WithUser option previously ran afterwards and replaced Process.User, dropping any CDI-provided groups.

Resolve the final container user before device injection so CDI edits append to that user, and add regression coverage for a CDI v0.7 spec that injects additionalGids.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar
elezar force-pushed the fix-cdi-additional-gids branch from 075cf45 to 704210f Compare May 8, 2026 13:24
@thaJeztah thaJeztah added status/2-code-review and removed dco/no Automatically set by a bot when one of the commits lacks proper signature labels May 11, 2026

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
thaJeztah merged commit 90deaa8 into moby:master May 12, 2026
352 of 368 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.