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

[Petr Vorel <pvorel@suse.cz>]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp