* [LTP] [PATCH v2] syscalls/setfsgid02: Rewrite setfsgid02
@ 2021-05-08 6:43 Zhao Gongyi
2021-05-11 8:23 ` Petr Vorel
0 siblings, 1 reply; 3+ messages in thread
From: Zhao Gongyi @ 2021-05-08 6:43 UTC (permalink / raw)
To: ltp
1)update to new API
2)rewrite setfsgid02 according man 2
Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
---
v1->v2:rewrite setfsgid02.c
.../kernel/syscalls/setfsgid/setfsgid02.c | 114 ++++++++----------
1 file changed, 47 insertions(+), 67 deletions(-)
diff --git a/testcases/kernel/syscalls/setfsgid/setfsgid02.c b/testcases/kernel/syscalls/setfsgid/setfsgid02.c
index 4788957e2..8951ee83f 100644
--- a/testcases/kernel/syscalls/setfsgid/setfsgid02.c
+++ b/testcases/kernel/syscalls/setfsgid/setfsgid02.c
@@ -1,91 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
/*
* Copyright (C) International Business Machines Corp., 2001
* Ported by Wayne Boyer
* Adapted by Dustin Kirkland (k1rkland@us.ibm.com)
- *
- * 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
+ * Adapted by Zhao gongyi (zhaogongyi@huawei.com)
*/
-/*
- * Testcase to check the basic functionality of setfsgid(2) system
- * call failures.
+/*\
+ * [Description]
+ * Testcase to check the basic functionality of setfsgid(2) system
+ * call failures with priviledged or unpriviledged user.
*/
-#include <stdio.h>
-#include <unistd.h>
-#include <grp.h>
#include <pwd.h>
-#include <sys/types.h>
-#include <errno.h>
-
-#include "test.h"
-#include "compat_16.h"
+#include "tst_test.h"
+#include "compat_tst_16.h"
-TCID_DEFINE(setfsgid02);
-int TST_TOTAL = 1;
+static gid_t gid;
+static gid_t pre_gid;
+static const char nobody_uid[] = "nobody";
+static struct passwd *ltpuser;
-static void setup(void);
-static void cleanup(void);
-
-int main(int ac, char **av)
+static void run(unsigned int i)
{
- int lc;
-
- gid_t gid;
-
- tst_parse_opts(ac, av, NULL, NULL);
-
- setup();
-
- for (lc = 0; TEST_LOOPING(lc); lc++) {
- tst_count = 0;
-
- gid = 1;
- while (getgrgid(gid))
- gid++;
+ int cnt;
- GID16_CHECK(gid, setfsgid, cleanup);
+ GID16_CHECK(gid, setfsgid);
- TEST(SETFSGID(cleanup, gid));
-
- if (TEST_RETURN == -1) {
- tst_resm(TFAIL | TTERRNO,
- "setfsgid() failed unexpectedly");
- continue;
- }
+ if (i == 0) {
+ ltpuser = SAFE_GETPWNAM(nobody_uid);
+ SAFE_SETEUID(ltpuser->pw_uid);
+ }
- if (TEST_RETURN == gid) {
- tst_resm(TFAIL, "setfsgid() returned %ld, expected %d",
- TEST_RETURN, gid);
- } else {
- tst_resm(TPASS, "setfsgid() returned expected value : "
- "%ld", TEST_RETURN);
+ /*
+ * Run SETFSGID() twice to check the second running have changed
+ * the gid for priviledged user, and have not changed the gid
+ * for unpriviledged user.
+ */
+ for (cnt = 0; cnt < 2; cnt++) {
+ TEST(SETFSGID(gid));
+ if (TST_RET != pre_gid)
+ tst_res(TFAIL, "setfsgid() returned %ld", TST_RET);
+ else {
+ tst_res(TPASS,
+ "setfsgid() returned expected value : %ld",
+ TST_RET);
+ if (i == 1) {
+ pre_gid = gid;
+ gid++;
+ }
}
}
- cleanup();
- tst_exit();
+ if (i == 0) {
+ SAFE_SETEUID(0);
+ }
}
static void setup(void)
{
- tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
- TEST_PAUSE;
+ pre_gid = 0;
+ gid = 1;
}
-static void cleanup(void)
-{
-}
+static struct tst_test test = {
+ .needs_root = 1,
+ .setup = setup,
+ .test = run,
+ .tcnt = 2,
+};
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [LTP] [PATCH v2] syscalls/setfsgid02: Rewrite setfsgid02
2021-05-08 6:43 [LTP] [PATCH v2] syscalls/setfsgid02: Rewrite setfsgid02 Zhao Gongyi
@ 2021-05-11 8:23 ` Petr Vorel
2021-09-01 7:57 ` Petr Vorel
0 siblings, 1 reply; 3+ messages in thread
From: Petr Vorel @ 2021-05-11 8:23 UTC (permalink / raw)
To: ltp
Hi Zhao,
Thank you for rewriting the test.
> 1)update to new API
> 2)rewrite setfsgid02 according man 2
I'd actually specify, what exactly you changed.
Cyril asked for:
- unprivileged process cannot change the value i.e. value that is
different from return from ret=setfsgid(-1) is passed as
setfsgid(ret+1) followed by setfsgid(-1) and all of these returns the
same value and the value also matches process effective group ID
- privileged process can change the value i.e. the same as
unprivileged but we expect the last setfsgid(-1) return
the new value. We either have to reset the setfsgid() at the end of
the test or run it in a fork()-ed process so that we start with a
clean plate for each iteration
Your code expects root has euid 0 and nobody does not have euid 1,
which of course works, but wouldn't be better to run setfsgid(-1) before testing
to verify it, as Cyril suggests?
...
> diff --git a/testcases/kernel/syscalls/setfsgid/setfsgid02.c b/testcases/kernel/syscalls/setfsgid/setfsgid02.c
> -/*
> - * Testcase to check the basic functionality of setfsgid(2) system
> - * call failures.
> +/*\
> + * [Description]
> + * Testcase to check the basic functionality of setfsgid(2) system
> + * call failures with priviledged or unpriviledged user.
typo: (un)priviledged => (un)privileged
And here be more verbose as well (IMHO "basic functionality" does not say much),
e.g. something like (in case you update the code):
* Testcase for setfsgid() syscall to check that
* - privileged user can change a filesystem group ID different from saved
* value of previous setfsgid() call
* - unprivileged user cannot change it
> */
> -#include <stdio.h>
> -#include <unistd.h>
> -#include <grp.h>
> #include <pwd.h>
> -#include <sys/types.h>
> -#include <errno.h>
> -
> -#include "test.h"
> -#include "compat_16.h"
> +#include "tst_test.h"
> +#include "compat_tst_16.h"
> -TCID_DEFINE(setfsgid02);
> -int TST_TOTAL = 1;
> +static gid_t gid;
> +static gid_t pre_gid;
> +static const char nobody_uid[] = "nobody";
> +static struct passwd *ltpuser;
> -static void setup(void);
> -static void cleanup(void);
> -
> -int main(int ac, char **av)
> +static void run(unsigned int i)
> {
> - int lc;
> -
> - gid_t gid;
> -
> - tst_parse_opts(ac, av, NULL, NULL);
> -
> - setup();
> -
> - for (lc = 0; TEST_LOOPING(lc); lc++) {
> - tst_count = 0;
> -
> - gid = 1;
> - while (getgrgid(gid))
> - gid++;
> + int cnt;
> - GID16_CHECK(gid, setfsgid, cleanup);
> + GID16_CHECK(gid, setfsgid);
> - TEST(SETFSGID(cleanup, gid));
> -
> - if (TEST_RETURN == -1) {
> - tst_resm(TFAIL | TTERRNO,
> - "setfsgid() failed unexpectedly");
> - continue;
> - }
> + if (i == 0) {
> + ltpuser = SAFE_GETPWNAM(nobody_uid);
> + SAFE_SETEUID(ltpuser->pw_uid);
> + }
> - if (TEST_RETURN == gid) {
> - tst_resm(TFAIL, "setfsgid() returned %ld, expected %d",
> - TEST_RETURN, gid);
> - } else {
> - tst_resm(TPASS, "setfsgid() returned expected value : "
> - "%ld", TEST_RETURN);
> + /*
> + * Run SETFSGID() twice to check the second running have changed
> + * the gid for priviledged user, and have not changed the gid
> + * for unpriviledged user.
And here typos.
> + */
> + for (cnt = 0; cnt < 2; cnt++) {
> + TEST(SETFSGID(gid));
> + if (TST_RET != pre_gid)
Cast to long is needed to prevent introducing warnings for 32bit compilation:
if ((long)pre_gid != TST_RET).
And move pre_gid to be first (WARNING: Comparisons should place the constant on
the right side of the test).
> + tst_res(TFAIL, "setfsgid() returned %ld", TST_RET);
> + else {
> + tst_res(TPASS,
> + "setfsgid() returned expected value : %ld",
> + TST_RET);
> + if (i == 1) {
> + pre_gid = gid;
> + gid++;
> + }
> }
> }
> + if (i == 0) {
> + SAFE_SETEUID(0);
> + }
nit: no brackets needed here.
scripts/checkpatch.pl from kernel is your friend
> }
IMHO improvements are good enough for test to be merged before release.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-01 7:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-08 6:43 [LTP] [PATCH v2] syscalls/setfsgid02: Rewrite setfsgid02 Zhao Gongyi
2021-05-11 8:23 ` Petr Vorel
2021-09-01 7:57 ` Petr Vorel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.