Skip to content
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

Add setgroups and fix getgroups #1006

Merged
merged 1 commit into from Mar 19, 2024

Conversation

yujincheng08
Copy link
Contributor

No description provided.

@sunfishcode
Copy link
Member

The CI failure here is unrelated.

The Linux setgroups syscall only operates on the current thread, which differs from the POSIX behavior of operating on the entire process, so these functions should go in thread rather than process. And the libc backend will need to use the syscall! macro rather than libc to achieve the same behavior. See the other set_* functions in src/thread/id.rs for examples of this.

@yujincheng08
Copy link
Contributor Author

yujincheng08 commented Feb 6, 2024

@sunfishcode I also considered the same thing as I saw the implementation of other set_* of id.rs but the thing is

  • there's no setgroups in POSIX;
  • in Linux it's actually setting for the whole process instead of the current thread.

See the following codes for testing:

#include <sys/types.h>
#include <thread>
#include <grp.h>
#include <unistd.h>
#include <stdio.h>

int main() {
    std::thread t([]() {
        gid_t gids[] = {1000, 27, 109};
        setgroups(3, gids);
    });
    t.join();
    gid_t gids[10];
    int n = getgroups(10, gids);
    for (int i = 0; i < n; i++) {
        printf("%d\n", gids[i]);
    }
    return 0;
}

@sunfishcode
Copy link
Member

Ah, I was mistaken about POSIX. Still, the Linux libc function sets the groups for all threads, but it appears the syscall only sets the groups for one thread. That means that in the current PR, the linux_raw backend which uses the syscall behaves differently from the libc backend which uses the libc function. Rustix does not currently have the infrastructure for setting the ids on all threads, and in the absence of such infrastructure, so we should provide the single-thread behahvior, in the thread module.

@yujincheng08
Copy link
Contributor Author

yujincheng08 commented Feb 25, 2024

You are right. The syscall itself is done for the thread only. But glibc uses nptl to set for the whole process.

int main() {
  std::thread t([]() {
    gid_t gids[] = {1000, 27, 109};
    syscall(SYS_setgroups, 3, gids);
  });
  t.join();
  gid_t gids[10];
  int n = syscall(SYS_getgroups, 10, gids);
  for (int i = 0; i < n; i++) {
    printf("in main %d\n", gids[i]);
  }
  return 0;
}

@yujincheng08 yujincheng08 changed the title Add setgroups Add setgroups and fix getgroups Feb 25, 2024
@yujincheng08
Copy link
Contributor Author

Modified as requested.

Also, I fixed getgroups:

EINVAL size is less than the number of supplementary group IDs,
but is not zero.

If size is zero, list is not modified, but the total number of supplementary group IDs for the process is returned. This allows the caller to determine the size of a dynamically allocated list to be used in a further call to getgroups().

@yujincheng08 yujincheng08 force-pushed the setgroups branch 4 times, most recently from 69bb2b8 to b3357af Compare February 25, 2024 17:58
@yujincheng08
Copy link
Contributor Author

@sunfishcode Gentle ping.

@sunfishcode
Copy link
Member

Looks good, thanks!

@sunfishcode sunfishcode merged commit 2fc59e9 into bytecodealliance:main Mar 19, 2024
43 checks passed
@yujincheng08 yujincheng08 deleted the setgroups branch March 20, 2024 00:46
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.

None yet

2 participants