[LTP] [PATCH] setgroups03: Fix running more iterations (-i 2)

Petr Vorel pvorel@suse.cz
Mon Oct 4 10:36:14 CEST 2021


Hi Zhao,

> When run the test with option "-i 2", test will fail and
> report "setgroups03.c:157: setgroups(65537) fails, Size
> is > sysconf(_SC_NGROUPS_MAX), errno=1, expected errno=22".
good point, thanks for addressing this.

> Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
> ---
>  .../kernel/syscalls/setgroups/setgroups03.c   | 24 ++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)

> diff --git a/testcases/kernel/syscalls/setgroups/setgroups03.c b/testcases/kernel/syscalls/setgroups/setgroups03.c
> index 490b06996..3ddea5635 100644
> --- a/testcases/kernel/syscalls/setgroups/setgroups03.c
> +++ b/testcases/kernel/syscalls/setgroups/setgroups03.c
> @@ -88,6 +88,7 @@ GID_T *groups_list;		/* Array to hold gids for getgroups() */

>  int setup1();			/* setup function to test error EPERM */
>  void setup();			/* setup function for the test */
> +void cleanup1();		/* cleanup function for setup1 */
>  void cleanup();			/* cleanup function for the test */
This test really deserves porting to new API. cleanup() function is empty
(thus we should actually delete it and use NULL instead of cleanup in tst_resm
and other functions, but that'll be cleaned in new API rewrite) and there are
other wrong parts (e.g. setup1() return only 0, but not 1 on failure) and really
ugly style (useless comments).

>  struct test_case_t {		/* test case struct. to hold ref. test cond's */
> @@ -156,6 +157,9 @@ int main(int ac, char **av)
>  					 gidsetsize, test_desc, TEST_ERRNO,
>  					 Test_cases[i].exp_errno);
>  			}
> +			if (Test_cases[i].setupfunc != NULL) {
> +				cleanup1();
> +			}
>  		}

>  	}
> @@ -191,7 +195,7 @@ int setup1(void)
>  {
>  	struct passwd *user_info;	/* struct. to hold test user info */

> -/* Switch to nobody user for correct error code collection */
> +	/* 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 "
> @@ -212,6 +216,24 @@ int setup1(void)
>  	return 0;
>  }

> +void cleanup1(void)
> +{
> +	struct passwd *user_info;
nit: blank line here.
> +	if (seteuid(0) < 0)
> +		tst_brkm(TBROK, cleanup, "seteuid failed");
SAFE_SETEUID(cleanup, 0);

> +
> +	if ((user_info = getpwnam("root")) == NULL)
> +		tst_brkm(TBROK, cleanup, "getpwnam(2) of root Failed");
SAFE_GETPWNAM(cleanup, NULL)

> +
> +	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;
> +}

Actually setup1() and cleanup1() could be single function if it takes uid_t euid
parameter, because this part of setup1() should IMHO be SAFE_GETPWNAM():

	if (seteuid(ltpuser->pw_uid) == -1) {
		tst_resm(TINFO, "setreuid failed to "
			 "to set the effective uid to %d", ltpuser->pw_uid);
		perror("setreuid");
	}

I'll send v2 under your name.

Kind regards,
Petr


More information about the ltp mailing list