Re: [LTP] [PATCH v3 1/1] setgroups03: Fix running more iterations (-i 2)

zhaogongyi
2021-10-14T06:47:42Z

Hi,

For the testcase setgroup3:

* Test Description:
* Verify that,
* 1. setgroups() fails with -1 and sets errno to EINVAL if the size
* argument value is > NGROUPS
* 2. setgroups() fails with -1 and sets errno to EPERM if the
* calling process is not super-user.

At the first checkpoint, set errno to EINVAL if the size argument value is > NGROUPS, it seems that has no matter with the value of group_list. Meticulously, we can set group_list to a normal value.

Same situation for the second checkpoint.

So can we reserve the group_list that set a normal value?

Thanks so much!

>
> Hi,
>
> Yes, in this testcase, the groups_list is redundant and can be removed.
>
> I am sorry for my late reply.
>
> Thanks so much!
>
>
>
> > > > -int setup1(void)
> > > > +void setup1(const char *uid, uid_t euid)
> > > > {
> > > > - struct passwd *user_info; /* struct. to hold test user info */
> > > > -
> > > > -/* Switch to nobody user for correct error code collection */
> > > > - ltpuser = getpwnam(nobody_uid);
> > > > - if (seteuid(ltpuser->pw_uid) == -1) {
> > > > - tst_resm(TINFO, "setreuid failed to "
> > > > - "to set the effective uid to %d", ltpuser->pw_uid);
> > > > - perror("setreuid");
> > > > - }
> > > > + struct passwd *user_info;
> >
> > > > - if ((user_info = getpwnam(TESTUSER)) == NULL) {
> > > > - tst_brkm(TFAIL, cleanup, "getpwnam(2) of %s Failed",
> > TESTUSER);
> > > > - }
> > > > + SAFE_SETEUID(cleanup, euid);
> > > > +
> > > > + user_info = SAFE_GETPWNAM(cleanup, uid);
> >
> > > I still do not get why we call SAFE_GETPWNAM() here. We should do
> > > that in the setup and prepare two different group_list[] lists, if
> > > that is really needed.
> >
> > > But I guess that all we need in this test is:
> >
> > > * Run the EINVAL test as a root
> >
> > > * Run the EPERM test as a nobody
> >
> > > The content of the list should not matter, as a matter of a fact we
> > > pass unitialized data in the EINVAL case. What matters is the size
> > > argument, it should be 1 for the EPERM test and max+1 for the EINVAL
> > case.
> >
> > Good point, thank you!
> >
> > @Zhao feel free to let me know you're doing to implement it.
> > Otherwise I'll have look on Monday.
> >
> > Kind regards,
> > Petr
> >
> > > > if (!GID_SIZE_CHECK(user_info->pw_gid)) {
> > > > tst_brkm(TBROK,
> > > > cleanup,
> > > > "gid returned from getpwnam is too large for testing
> > setgroups16");
> > > > }
> > > > +
> > > > groups_list[0] = user_info->pw_gid;
> > > > - return 0;
> > > > }
> >
> > > > /*
> > > > --
> > > > 2.33.0