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

Cyril Hrubis
2021-10-08T10:22:48Z

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.

> 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
>

--
Cyril Hrubis
chrubis@suse.cz