Skip to content

[vminitd]: update filesystem operation to run in new namespace - #838

Open
saehejkang wants to merge 1 commit into
apple:mainfrom
saehejkang:update-filesystem-operation-to-run-in-new-namespace
Open

[vminitd]: update filesystem operation to run in new namespace#838
saehejkang wants to merge 1 commit into
apple:mainfrom
saehejkang:update-filesystem-operation-to-run-in-new-namespace

Conversation

@saehejkang

Copy link
Copy Markdown
Contributor

Changes Made

  • Updated the filesystem operation to take in the containerID, so that operations are always executed against the intended container’s mount namespace.

Co-authored-by: John Logan <john_logan@apple.com>
let containerMountFd = open("/proc/\(containerPid)/ns/mount", O_RDONLY | O_DIRECTORY | O_CLOEXEC)
if containerMountFd < 0 {
let error = swiftErrno("open")
throw RPCError(code: .internalError, message: "failed to open container mount namespace", cause: error)

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.

selfMountFd leaks here, doesn't it?

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.

Those leaks should be resolved in the defer block, no? I think we can remove the defer and then explicitly close both fds

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.

Using defer is fine.

If 757 throws, the defer block at 760 is never reached and selfMountFd doesn't get closed. You need a defer for selfMountFd before 754, and the block at 760 need only contain containerMountFd.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants