Skip to content

core/mount: copy options before mutating in prepareIDMappedOverlay - #14204

Open
zanarellidev wants to merge 3 commits into
containerd:mainfrom
zanarellidev:bugsweep-mount-copy
Open

zanarellidev wants to merge 3 commits into
containerd:mainfrom
zanarellidev:bugsweep-mount-copy

Conversation

@zanarellidev

@zanarellidev zanarellidev commented Sep 22, 2026 •

Copy link
Copy Markdown

What this PR does / why we need it

prepareIDMappedOverlay removed the lowerdir= entry and appended its replacement directly on the slice it received, mutating the caller's backing array.

Mount.mount passes m.Options straight into prepareIDMappedOverlay without cloning, so any caller retaining a reference to the original Mount (for example through mount.All) observed its Options silently rewritten to the temporary idmapped lowerdir path, which is subsequently removed once the idmap cleanup function runs.

compactLowerdirOption in the same file already avoids this by copying before mutating via copyOptions(opts); this PR applies the same defensive copy guard here.

Includes regression test TestPrepareIDMappedOverlayAliasesCallerOptions asserting that caller options are not mutated.

Copilot AI lite review requested due to automatic review settings September 22, 2026 00:47
@github-project-automation github-project-automation Bot moved this to Needs Triage in Pull Request Review Sep 22, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings September 27, 2026 13:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

prepareIDMappedOverlay removed the lowerdir= entry and appended its
replacement directly on the slice it received, mutating the caller's
backing array. Mount.mount passes m.Options straight in without
cloning, so any caller retaining a reference to the original Mount
(e.g. through mount.All) observed its Options silently rewritten to
the temporary idmapped lowerdir path, which is removed again once the
idmap cleanup function runs.

compactLowerdirOption in the same file already avoids this by copying
before mutating; apply the same guard here.

Signed-off-by: zanarelli <zanarelli.dev@gmail.com>
Copilot AI lite review requested due to automatic review settings October 1, 2026 21:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

Only a minor test-comment nit remains; the implementation and regression test address the stated bug.

Review effort: Lite
Findings: 1 Low severity

Open (1)

Comment thread core/mount/mount_linux_test.go Outdated
Signed-off-by: Raphael Zanarelli <zanarelli.dev@gmail.com>
Copilot AI lite review requested due to automatic review settings October 1, 2026 21:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved issues were identified.

Review effort: Lite
Findings: None

Resolved since last review (1)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Needs Triage

Development

Successfully merging this pull request may close these issues.

2 participants