diff mbox series

[v2,2/2] Refactor getegid02 using new LTP API

Message ID 20230908141414.28359-3-andrea.cervesato@suse.de
State Accepted
Headers show
Series Refactor getegid testing suite | expand

Commit Message

Andrea Cervesato Sept. 8, 2023, 2:14 p.m. UTC
From: Andrea Cervesato <andrea.cervesato@suse.com>

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 testcases/kernel/syscalls/getegid/getegid02.c | 93 +++++--------------
 1 file changed, 21 insertions(+), 72 deletions(-)

Comments

Petr Vorel Oct. 17, 2023, 11:35 a.m. UTC | #1
Hi all,

This code is based on Cyril's suggestion in v1 [1]
	if (GID_SIZE_CHECK(st_egid))
		TST_EXP_EQ_LI(gid, st_egid);
	else
		tst_res(TPASS, "getegid() passed");

I wonder which system returns 16 bit gid?

man getgid(2) says that originally there was only 16 bit, than kernel 2.4 added
support for 32 bit and the glibc getegid() wrapper transparently deal with this.

Because even I compile as 32 bit:

$ file getegid01_16
getegid01_16: ELF 32-bit LSB pie executable, Intel 80386

I get comparison, thus 32 bit:
$ ./getegid01_16
...
getegid01.c:25: TPASS: gid == st_egid (1000)

What am I missing?

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/ZPCWWXXHG-oEB5qO@yuki/
Petr Vorel Oct. 17, 2023, 11:40 a.m. UTC | #2
Hi Andrea,

>  /*
> - * Copyright (c) International Business Machines  Corp., 2001
> - *  Ported by Wayne Boyer
> - *
> - * This program is free software;  you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY;  without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> - * the GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program;  if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
> + *   William Roske, Dave Fenner

These two developers were mentioned only in getegid01.c IMHO it should have not
been here. Copy paste error?

Kind regards,
Petr
Cyril Hrubis Oct. 17, 2023, 12:41 p.m. UTC | #3
Hi!
> This code is based on Cyril's suggestion in v1 [1]
> 	if (GID_SIZE_CHECK(st_egid))
> 		TST_EXP_EQ_LI(gid, st_egid);
> 	else
> 		tst_res(TPASS, "getegid() passed");
> 
> I wonder which system returns 16 bit gid?
> 
> man getgid(2) says that originally there was only 16 bit, than kernel 2.4 added
> support for 32 bit and the glibc getegid() wrapper transparently deal with this.
> 
> Because even I compile as 32 bit:
> 
> $ file getegid01_16
> getegid01_16: ELF 32-bit LSB pie executable, Intel 80386
> 
> I get comparison, thus 32 bit:
> $ ./getegid01_16
> ...
> getegid01.c:25: TPASS: gid == st_egid (1000)
> 
> What am I missing?

This obviously works since 1000 < 32767. As long as the GID fits 16bit
integer everything works fine with 16bit syscalls. The check returns if
the egid does fit and returns true if so.

When the GID does not fit, kernel returns overflow GID, i.e. for all
values bigger than 16 bit you get the same GID value that does not
correspond to the actual GID value at all.

So to make the check false, you have to set up the test user so that it
has GID > 16bit integer maximum value.
Petr Vorel Oct. 17, 2023, 12:41 p.m. UTC | #4
> Hi!
> > This code is based on Cyril's suggestion in v1 [1]
> > 	if (GID_SIZE_CHECK(st_egid))
> > 		TST_EXP_EQ_LI(gid, st_egid);
> > 	else
> > 		tst_res(TPASS, "getegid() passed");

> > I wonder which system returns 16 bit gid?

> > man getgid(2) says that originally there was only 16 bit, than kernel 2.4 added
> > support for 32 bit and the glibc getegid() wrapper transparently deal with this.

> > Because even I compile as 32 bit:

> > $ file getegid01_16
> > getegid01_16: ELF 32-bit LSB pie executable, Intel 80386

> > I get comparison, thus 32 bit:
> > $ ./getegid01_16
> > ...
> > getegid01.c:25: TPASS: gid == st_egid (1000)

> > What am I missing?

> This obviously works since 1000 < 32767. As long as the GID fits 16bit
> integer everything works fine with 16bit syscalls. The check returns if
> the egid does fit and returns true if so.

> When the GID does not fit, kernel returns overflow GID, i.e. for all
> values bigger than 16 bit you get the same GID value that does not
> correspond to the actual GID value at all.

> So to make the check false, you have to set up the test user so that it
> has GID > 16bit integer maximum value.

Thanks a lot, Cyril!

Kind regards,
Petr
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/getegid/getegid02.c b/testcases/kernel/syscalls/getegid/getegid02.c
index 60f09501e..2f64bd869 100644
--- a/testcases/kernel/syscalls/getegid/getegid02.c
+++ b/testcases/kernel/syscalls/getegid/getegid02.c
@@ -1,90 +1,39 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) International Business Machines  Corp., 2001
- *  Ported by Wayne Boyer
- *
- * This program is free software;  you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY;  without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- * the GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program;  if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
+ *   William Roske, Dave Fenner
+ * Copyright (C) 2023 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- * Testcase to check the basic functionality of getegid().
+/*\
+ * [Description]
  *
- * For functionality test the return value from getegid() is compared to passwd
- * entry.
+ * This test checks if getegid() returns the same effective group given by
+ * passwd entry via getpwuid().
  */
 
 #include <pwd.h>
-#include <errno.h>
-
-#include "test.h"
-#include "compat_16.h"
 
-static void cleanup(void);
-static void setup(void);
+#include "tst_test.h"
+#include "compat_tst_16.h"
 
-TCID_DEFINE(getegid02);
-int TST_TOTAL = 1;
-
-int main(int ac, char **av)
+static void run(void)
 {
-	int lc;
 	uid_t euid;
+	gid_t egid;
 	struct passwd *pwent;
 
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		TEST(GETEGID(cleanup));
+	UID16_CHECK((euid = geteuid()), "geteuid");
 
-		if (TEST_RETURN < 0) {
-			tst_brkm(TBROK, cleanup, "This should never happen");
-		}
+	pwent = getpwuid(euid);
+	if (!pwent)
+		tst_brk(TBROK | TERRNO, "getpwuid() error");
 
-		euid = geteuid();
-		pwent = getpwuid(euid);
+	GID16_CHECK((egid = getegid()), "getegid");
 
-		if (pwent == NULL)
-			tst_brkm(TBROK, cleanup, "geteuid() returned "
-				 "unexpected value %d", euid);
-
-		GID16_CHECK(pwent->pw_gid, getegid, cleanup);
-
-		if (pwent->pw_gid != TEST_RETURN) {
-			tst_resm(TFAIL, "getegid() return value"
-				 " %ld unexpected - expected %d",
-				 TEST_RETURN, pwent->pw_gid);
-		} else {
-			tst_resm(TPASS,
-				 "effective group id %ld "
-				 "is correct", TEST_RETURN);
-		}
-	}
-
-	cleanup();
-	tst_exit();
-}
-
-static void setup(void)
-{
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-	TEST_PAUSE;
+	TST_EXP_EQ_LI(pwent->pw_gid, egid);
 }
 
-static void cleanup(void)
-{
-}
+static struct tst_test test = {
+	.test_all = run,
+};