Skip to content

dcp: Restore UID/GID after setting with --uid/--gid #586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

bdevcich
Copy link
Contributor

@bdevcich bdevcich commented Oct 3, 2024

When --uid or --gid is used, MPI_Init() is called as the original user. The original uid/gid is not being restored prior to calling MPI_Finalize(), which leads to cleanup issues.

Resolves #585.

@bdevcich
Copy link
Contributor Author

bdevcich commented Oct 7, 2024

I'm not sure who I should ask to take a look so taking some guesses based on previous PRs: @ofaaland @gonsie

@ofaaland
Copy link
Collaborator

ofaaland commented Oct 7, 2024

I'm not sure who I should ask to take a look so taking some guesses based on previous PRs: @ofaaland @gonsie

Thanks Blake, I'll look

Copy link
Collaborator

@ofaaland ofaaland left a comment

Choose a reason for hiding this comment

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

Thanks @bdevcich for debugging this! I have just one nit.

src/dcp/dcp.c Outdated
if (setgroups(0, NULL) < 0) {
MFU_LOG(MFU_LOG_ERR, "Could not setgroups: %s", strerror(errno));
mfu_finalize();
MPI_Finalize();
return 1;
}

/* record the original groups */
getgroups(MAX_GIDS, &gids);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please check this for failure and report it (e.g. errno==EINVAL means MAX_GIDS was too small), or dynamically get the size needed and allocate the buffer for the gids.

@bdevcich
Copy link
Contributor Author

bdevcich commented Oct 8, 2024

Thanks @bdevcich for debugging this! I have just one nit.

It was my bug, so I felt obligated! 😄

@bdevcich bdevcich requested a review from ofaaland October 8, 2024 18:11
Copy link
Collaborator

@ofaaland ofaaland left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@ofaaland
Copy link
Collaborator

ofaaland commented Oct 9, 2024

Hi @bdevcich can you squash these commits and re-push? Thanks

When `--uid` or `--gid` is used, MPI_Init() is called as the original
user. The original uid/gid is not being restored prior to calling
MPI_Finalize(), which leads to cleanup issues.

Resolves hpc#585.

Signed-off-by: Blake Devcich <[email protected]>
@bdevcich
Copy link
Contributor Author

bdevcich commented Oct 9, 2024

@ofaaland done!

@bdevcich
Copy link
Contributor Author

@ofaaland can you merge? I don't have permission to do so.

@ofaaland
Copy link
Collaborator

@ofaaland can you merge? I don't have permission to do so.

Odd, I thought I already had. I'll try again.

@ofaaland ofaaland merged commit d56910b into hpc:main Oct 14, 2024
@ofaaland
Copy link
Collaborator

@bdevcich there you go. Thanks again!

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.

dcp: MPI permissions issue with --uid and --gid
2 participants