diff mbox series

setresuid04.c: Rewrite the test using new LTP API

Message ID 20221206075747.17352-1-akumar@suse.de
State Accepted
Headers show
Series setresuid04.c: Rewrite the test using new LTP API | expand

Commit Message

Avinesh Kumar Dec. 6, 2022, 7:57 a.m. UTC
Signed-off-by: Avinesh Kumar <akumar@suse.de>
---
 .../kernel/syscalls/setresuid/setresuid04.c   | 255 ++++--------------
 1 file changed, 52 insertions(+), 203 deletions(-)

Comments

Li Wang Dec. 8, 2022, 3:37 a.m. UTC | #1
Hi Avinesh,

Avinesh Kumar <akumar@suse.de> wrote:

> -               /* Test 2: Check a son process cannot open the file
> -                *         with RDWR permissions.
> -                */
> -               pid = FORK_OR_VFORK();
> -               if (pid < 0)
> -                       tst_brkm(TBROK, NULL, "Fork failed");
> -
> -               if (pid == 0) {
> -                       int tst_fd2;
> +       SAFE_SETRESUID(0, ltpuser->pw_uid, 0);

There is no reason to modify 'real UID' and 'saved set-user-ID'
at this time, we do only care about 'effective UID' made changes
successfully or not.

So I tweaked the patch a tiny and merged it:

--- a/testcases/kernel/syscalls/setresuid/setresuid04.c
+++ b/testcases/kernel/syscalls/setresuid/setresuid04.c
@@ -40,7 +40,7 @@ static void run(void)
        pid_t pid;
        int status;

-       SAFE_SETRESUID(0, ltpuser->pw_uid, 0);
+       SAFE_SETRESUID(-1, ltpuser->pw_uid, -1);
        TST_EXP_FAIL2(open(TEMP_FILE, O_RDWR), EACCES);

        pid = SAFE_FORK();
Avinesh Kumar Dec. 8, 2022, 6:35 a.m. UTC | #2
Hi Li,

On Thursday, December 8, 2022 9:07:08 AM IST Li Wang wrote:
> Hi Avinesh,
> 
> Avinesh Kumar <akumar@suse.de> wrote:
> 
> > -               /* Test 2: Check a son process cannot open the file
> > -                *         with RDWR permissions.
> > -                */
> > -               pid = FORK_OR_VFORK();
> > -               if (pid < 0)
> > -                       tst_brkm(TBROK, NULL, "Fork failed");
> > -
> > -               if (pid == 0) {
> > -                       int tst_fd2;
> > +       SAFE_SETRESUID(0, ltpuser->pw_uid, 0);
> 
> There is no reason to modify 'real UID' and 'saved set-user-ID'
> at this time, we do only care about 'effective UID' made changes
> successfully or not.
> 
Yes, Thank you, I realized this while working on setresuid05.c test.
I think we should leave the 'real UID' and 'saved set-user-ID'
untouched even when resetting 'effective UID' to root:
       SAFE_SETRESUID(0, 0, 0); should be
       SAFE_SETRESUID(-1, 0, -1);

I will send another patch as we also need to add .needs_tmpdir=1
because we are creating a temp file.


> So I tweaked the patch a tiny and merged it:
> 
> --- a/testcases/kernel/syscalls/setresuid/setresuid04.c
> +++ b/testcases/kernel/syscalls/setresuid/setresuid04.c
> @@ -40,7 +40,7 @@ static void run(void)
>         pid_t pid;
>         int status;
> 
> -       SAFE_SETRESUID(0, ltpuser->pw_uid, 0);
> +       SAFE_SETRESUID(-1, ltpuser->pw_uid, -1);
>         TST_EXP_FAIL2(open(TEMP_FILE, O_RDWR), EACCES);
> 
>         pid = SAFE_FORK();
> 
> 
> 

Best regards,
Avinesh
Li Wang Dec. 8, 2022, 6:54 a.m. UTC | #3
On Thu, Dec 8, 2022 at 2:35 PM Avinesh Kumar <akumar@suse.de> wrote:
>
> Hi Li,
>
> On Thursday, December 8, 2022 9:07:08 AM IST Li Wang wrote:
> > Hi Avinesh,
> >
> > Avinesh Kumar <akumar@suse.de> wrote:
> >
> > > -               /* Test 2: Check a son process cannot open the file
> > > -                *         with RDWR permissions.
> > > -                */
> > > -               pid = FORK_OR_VFORK();
> > > -               if (pid < 0)
> > > -                       tst_brkm(TBROK, NULL, "Fork failed");
> > > -
> > > -               if (pid == 0) {
> > > -                       int tst_fd2;
> > > +       SAFE_SETRESUID(0, ltpuser->pw_uid, 0);
> >
> > There is no reason to modify 'real UID' and 'saved set-user-ID'
> > at this time, we do only care about 'effective UID' made changes
> > successfully or not.
> >
> Yes, Thank you, I realized this while working on setresuid05.c test.
> I think we should leave the 'real UID' and 'saved set-user-ID'
> untouched even when resetting 'effective UID' to root:
>        SAFE_SETRESUID(0, 0, 0); should be
>        SAFE_SETRESUID(-1, 0, -1);

Agree, and with ".needs_root = 1" setting at the beginning
of the test, they are definitely 0 so I didn't correct them
because the test is already done when going there.

But yes, if you'd pursuing perfection I'd help ack your new patch :).

>
> I will send another patch as we also need to add .needs_tmpdir=1
> because we are creating a temp file.

+1

>
>
> > So I tweaked the patch a tiny and merged it:
> >
> > --- a/testcases/kernel/syscalls/setresuid/setresuid04.c
> > +++ b/testcases/kernel/syscalls/setresuid/setresuid04.c
> > @@ -40,7 +40,7 @@ static void run(void)
> >         pid_t pid;
> >         int status;
> >
> > -       SAFE_SETRESUID(0, ltpuser->pw_uid, 0);
> > +       SAFE_SETRESUID(-1, ltpuser->pw_uid, -1);
> >         TST_EXP_FAIL2(open(TEMP_FILE, O_RDWR), EACCES);
> >
> >         pid = SAFE_FORK();
> >
> >
> >
>
> Best regards,
> Avinesh
>
>
>
>
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/setresuid/setresuid04.c b/testcases/kernel/syscalls/setresuid/setresuid04.c
index e197476ff..20840e2a2 100644
--- a/testcases/kernel/syscalls/setresuid/setresuid04.c
+++ b/testcases/kernel/syscalls/setresuid/setresuid04.c
@@ -1,223 +1,72 @@ 
-/******************************************************************************/
-/* Copyright (c) Kerlabs 2008.                                                */
-/* Copyright (c) International Business Machines  Corp., 2008                 */
-/*                                                                            */
-/* 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    */
-/*                                                                            */
-/******************************************************************************/
-/*
- * NAME
- * 	setresuid04.c
- *
- * DESCRIPTION
- * 	Check if setresuid behaves correctly with file permissions.
- *      The test creates a file as ROOT with permissions 0644, does a setresuid
- *      and then tries to open the file with RDWR permissions.
- *      The same test is done in a fork to check if new UIDs are correctly
- *      passed to the son.
- *
- * USAGE:  <for command-line>
- *  setresuid04 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -e   : Turn on errno logging.
- *             -i n : Execute test n times.
- *             -I x : Execute test for x seconds.
- *             -P x : Pause for x seconds between iterations.
- *             -t   : Turn on syscall timing.
- *
- * HISTORY
- *	07/2001 Created by Renaud Lottiaux
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright (c) Kerlabs 2008.
+ * Copyright (c) International Business Machines  Corp., 2008
+ * Copyright (c) 2022 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com>
+ */
+
+/*\
+ * [Description]
  *
- * RESTRICTIONS
- * 	Must be run as root.
+ * Verify that setresuid() behaves correctly with file permissions.
+ * The test creates a file as ROOT with permissions 0644, does a setresuid
+ * to change euid to a non-root user and tries to open the file with RDWR
+ * permissions, which should fail with EACCES errno.
+ * The same test is done in a fork also to check that child process also
+ * inherits new euid and open fails with EACCES.
+ * Test verifies the successful open action after reverting the euid back
+ * ROOT user.
  */
-#define _GNU_SOURCE 1
-#include <errno.h>
-#include <sys/types.h>
-#include <sys/stat.h>
+
 #include <sys/wait.h>
-#include <fcntl.h>
-#include <unistd.h>
-#include "test.h"
-#include "safe_macros.h"
 #include <pwd.h>
-#include "compat_16.h"
+#include "tst_test.h"
+#include "compat_tst_16.h"
 
-TCID_DEFINE(setresuid04);
-int TST_TOTAL = 1;
-char nobody_uid[] = "nobody";
-char testfile[256] = "";
-struct passwd *ltpuser;
+#define TEMP_FILE	"testfile"
+static char nobody_uid[] = "nobody";
+static struct passwd *ltpuser;
+static int fd = -1;
 
-int fd = -1;
-
-void setup(void);
-void cleanup(void);
-void do_master_child();
-
-int main(int ac, char **av)
+static void setup(void)
 {
-	pid_t pid;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-	setup();
-
-	pid = FORK_OR_VFORK();
-	if (pid < 0)
-		tst_brkm(TBROK, cleanup, "Fork failed");
+	ltpuser = SAFE_GETPWNAM(nobody_uid);
+	UID16_CHECK(ltpuser->pw_uid, "setresuid");
 
-	if (pid == 0)
-		do_master_child();
-
-	tst_record_childstatus(cleanup, pid);
-
-	cleanup();
-	tst_exit();
+	fd = SAFE_OPEN(TEMP_FILE, O_CREAT | O_RDWR, 0644);
 }
 
-/*
- * do_master_child()
- */
-void do_master_child(void)
+static void run(void)
 {
-	int lc;
-	int pid;
+	pid_t pid;
 	int status;
 
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		int tst_fd;
-
-		/* Reset tst_count in case we are looping */
-		tst_count = 0;
-
-		if (SETRESUID(NULL, 0, ltpuser->pw_uid, 0) == -1) {
-			perror("setresuid failed");
-			exit(TFAIL);
-		}
-
-		/* Test 1: Check the process with new uid cannot open the file
-		 *         with RDWR permissions.
-		 */
-		TEST(tst_fd = open(testfile, O_RDWR));
-
-		if (TEST_RETURN != -1) {
-			printf("open succeeded unexpectedly\n");
-			close(tst_fd);
-			exit(TFAIL);
-		}
-
-		if (TEST_ERRNO == EACCES) {
-			printf("open failed with EACCES as expected\n");
-		} else {
-			perror("open failed unexpectedly");
-			exit(TFAIL);
-		}
-
-		/* Test 2: Check a son process cannot open the file
-		 *         with RDWR permissions.
-		 */
-		pid = FORK_OR_VFORK();
-		if (pid < 0)
-			tst_brkm(TBROK, NULL, "Fork failed");
-
-		if (pid == 0) {
-			int tst_fd2;
+	SAFE_SETRESUID(0, ltpuser->pw_uid, 0);
+	TST_EXP_FAIL2(open(TEMP_FILE, O_RDWR), EACCES);
 
-			/* Test to open the file in son process */
-			TEST(tst_fd2 = open(testfile, O_RDWR));
-
-			if (TEST_RETURN != -1) {
-				printf("call succeeded unexpectedly\n");
-				close(tst_fd2);
-				exit(TFAIL);
-			}
-
-			if (TEST_ERRNO == EACCES) {
-				printf("open failed with EACCES as expected\n");
-				exit(TPASS);
-			} else {
-				printf("open failed unexpectedly\n");
-				exit(TFAIL);
-			}
-		} else {
-			/* Wait for son completion */
-			if (waitpid(pid, &status, 0) == -1) {
-				perror("waitpid failed");
-				exit(TFAIL);
-			}
-
-			if (!WIFEXITED(status))
-				exit(TFAIL);
-
-			if (WEXITSTATUS(status) != TPASS)
-				exit(WEXITSTATUS(status));
-		}
-
-		/* Test 3: Fallback to initial uid and check we can again open
-		 *         the file with RDWR permissions.
-		 */
-		tst_count++;
-		if (SETRESUID(NULL, 0, 0, 0) == -1) {
-			perror("setresuid failed");
-			exit(TFAIL);
-		}
-
-		TEST(tst_fd = open(testfile, O_RDWR));
-
-		if (TEST_RETURN == -1) {
-			perror("open failed unexpectedly");
-			exit(TFAIL);
-		} else {
-			printf("open call succeeded\n");
-			close(tst_fd);
-		}
+	pid = SAFE_FORK();
+	if (!pid) {
+		TST_EXP_FAIL2(open(TEMP_FILE, O_RDWR), EACCES);
+		return;
 	}
-	exit(TPASS);
-}
-
-/*
- * setup() - performs all ONE TIME setup for this test
- */
-void setup(void)
-{
-	tst_require_root();
+	SAFE_WAITPID(pid, &status, 0);
+	if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
+		tst_res(TFAIL, "child process exited with status: %d", status);
 
-	ltpuser = getpwnam(nobody_uid);
-
-	UID16_CHECK(ltpuser->pw_uid, "setresuid", cleanup)
-
-	tst_tmpdir();
-
-	sprintf(testfile, "setresuid04file%d.tst", getpid());
-
-	/* Create test file */
-	fd = SAFE_OPEN(cleanup, testfile, O_CREAT | O_RDWR, 0644);
-
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
+	SAFE_SETRESUID(0, 0, 0);
+	TST_EXP_FD(open(TEMP_FILE, O_RDWR));
+	SAFE_CLOSE(TST_RET);
 }
 
-/*
- * cleanup() - performs all the ONE TIME cleanup for this test at completion
- * 	       or premature exit
- */
-void cleanup(void)
+static void cleanup(void)
 {
-	close(fd);
-
-	tst_rmdir();
-
+	if (fd > 0)
+		SAFE_CLOSE(fd);
 }
+
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = run,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.forks_child = 1
+};