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

Petr Vorel
2021-10-08T11:12:30Z

Hi Cyril, Zhao,

> Hi!
> > -int setup1(); /* setup function to test error EPERM */
> > +void setup1(const char *uid, uid_t euid); /* setup function to test error EPERM */
> > void setup(); /* setup function for the test */
> > void cleanup(); /* cleanup function for the test */

> > @@ -95,7 +93,7 @@ struct test_case_t { /* test case struct. to hold ref. test cond's */
> > int list;
> > char *desc;
> > int exp_errno;
> > - int (*setupfunc) ();
> > + void (*setupfunc)(const char *uid, uid_t euid);
> > } Test_cases[] = {
> > {
> > 1, 1, "Size is > sysconf(_SC_NGROUPS_MAX)", EINVAL, NULL}, {
> > @@ -126,7 +124,7 @@ int main(int ac, char **av)

> > for (i = 0; i < TST_TOTAL; i++) {
> > if (Test_cases[i].setupfunc != NULL) {
> > - Test_cases[i].setupfunc();
> > + Test_cases[i].setupfunc(nobody_uid, ltpuser->pw_uid);
> > }

> > gidsetsize = ngroups_max + Test_cases[i].gsize_add;
> > @@ -156,8 +154,11 @@ int main(int ac, char **av)
> > gidsetsize, test_desc, TEST_ERRNO,
> > Test_cases[i].exp_errno);
> > }
> > - }

> > + if (Test_cases[i].setupfunc != NULL) {
> > + Test_cases[i].setupfunc("root", ltpuser->pw_uid);
> > + }
> > + }
> > }

> > cleanup();
> > @@ -176,8 +177,9 @@ void setup(void)

> > tst_sig(NOFORK, DEF_HANDLER, cleanup);

> > - TEST_PAUSE;
> > + ltpuser = SAFE_GETPWNAM(cleanup, uid);

> > + TEST_PAUSE;
> > }

> > /*
> > @@ -187,29 +189,21 @@ void setup(void)
> > * Get the user info. from /etc/passwd file.
> > * This function returns 0 on success.
> > */
> > -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