[ptest-runner,1/4] utils: ensure child can be session leader

Submitted by Randy MacLeod on July 17, 2019, 6:35 p.m. | Patch ID: 163137

Details

Message ID 20190717183530.24893-1-Randy.MacLeod@windriver.com
State New
Headers show

Commit Message

Randy MacLeod July 17, 2019, 6:35 p.m.
When running the run-execscript bash ptest as a user rather than root, a warning:
  bash: cannot set terminal process group (16036): Inappropriate ioctl for device
  bash: no job control in this shell
contaminates the bash log files causing the test to fail. This happens only
when run under ptest-runner and not when interactively testing!

The changes made to fix this include:
1. Get the process group id (pgid) before forking,
2. Set the pgid in both the parent and child to avoid a race,
3. Find, open and set permission on the child tty, and
4. Allow the child to attach to controlling tty.

Also add '-lutil' to Makefile. This lib is from libc and provides openpty.

Upstream-Status: Submitted [yocto@yoctoproject.org]

Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
---
 Makefile |   2 +-
 utils.c  | 102 +++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 92 insertions(+), 12 deletions(-)

Patch hide | download patch | download mbox

diff --git a/Makefile b/Makefile
index 1bde7be..439eb79 100644
--- a/Makefile
+++ b/Makefile
@@ -29,7 +29,7 @@  TEST_DATA=$(shell echo `pwd`/tests/data)
 all: $(SOURCES) $(EXECUTABLE)
 
 $(EXECUTABLE): $(OBJECTS)
-	$(CC) $(LDFLAGS) $(OBJECTS) -o $@
+	$(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@
 
 tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)
 
diff --git a/utils.c b/utils.c
index ad737c2..f11ce39 100644
--- a/utils.c
+++ b/utils.c
@@ -1,5 +1,6 @@ 
 /**
  * Copyright (c) 2016 Intel Corporation
+ * Copyright (C) 2019 Wind River Systems, Inc.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -22,23 +23,27 @@ 
  */
 
 #define _GNU_SOURCE 
+
 #include <stdio.h>
 
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <grp.h>
 #include <libgen.h>
-#include <signal.h>
 #include <poll.h>
-#include <fcntl.h>
+#include <pty.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <string.h>
 #include <time.h>
-#include <dirent.h>
+#include <unistd.h>
+
+#include <sys/ioctl.h>
 #include <sys/resource.h>
+#include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
-#include <sys/stat.h>
-#include <unistd.h>
-#include <string.h>
-#include <stdlib.h>
-
-#include <errno.h>
 
 #include "ptest_list.h"
 #include "utils.h"
@@ -346,6 +351,53 @@  wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
 	return status;
 }
 
+/* Returns an integer file descriptor.
+ * If it returns < 0, an error has occurred.
+ * Otherwise, it has returned the slave pty file descriptor.
+ * fp should be writable, likely stdout/err.
+ */
+static int
+setup_slave_pty(FILE *fp) { 
+	int pty_master = -1;
+	int pty_slave = -1;
+	char pty_name[256];
+	struct group *gptr;
+	gid_t gid;
+	int slave = -1;
+
+	if (openpty(&pty_master, &pty_slave, pty_name, NULL, NULL) < 0) {
+		fprintf(fp, "ERROR: openpty() failed with: %s.\n", strerror(errno));
+		return -1;
+	}
+
+	if ((gptr = getgrnam(pty_name)) != 0) {
+		gid = gptr->gr_gid;
+	} else {
+		/* If the tty group does not exist, don't change the
+		 * group on the slave pty, only the owner
+		 */
+		gid = -1;
+	}
+
+	/* chown/chmod the corresponding pty, if possible.
+	 * This will only work if the process has root permissions.
+	 */
+	if (chown(pty_name, getuid(), gid) != 0) {
+		fprintf(fp, "ERROR; chown() failed with: %s.\n", strerror(errno));
+	}
+
+	/* Makes the slave read/writeable for the user. */
+	if (chmod(pty_name, S_IRUSR|S_IWUSR) != 0) {
+		fprintf(fp, "ERROR: chmod() failed with: %s.\n", strerror(errno));
+	}
+
+	if ((slave = open(pty_name, O_RDWR)) == -1) {
+		fprintf(fp, "ERROR: open() failed with: %s.\n", strerror(errno));
+	}
+	return (slave);
+}
+
+
 int
 run_ptests(struct ptest_list *head, const struct ptest_options opts,
 		const char *progname, FILE *fp, FILE *fp_stderr)
@@ -362,6 +414,8 @@  run_ptests(struct ptest_list *head, const struct ptest_options opts,
 	int timeouted;
 	time_t sttime, entime;
 	int duration;
+	int slave;
+	int pgid = -1;
 
 	if (opts.xml_filename) {
 		xh = xml_create(ptest_list_length(head), opts.xml_filename);
@@ -379,7 +433,6 @@  run_ptests(struct ptest_list *head, const struct ptest_options opts,
 			close(pipefd_stdout[1]);
 			break;
 		}
-
 		fprintf(fp, "START: %s\n", progname);
 		PTEST_LIST_ITERATE_START(head, p);
 			char *ptest_dir = strdup(p->run_ptest);
@@ -388,6 +441,13 @@  run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				break;
 			}
 			dirname(ptest_dir);
+			if (ioctl(0, TIOCNOTTY) == -1) {
+				fprintf(fp, "ERROR: Unable to detach from controlling tty, %s\n", strerror(errno));
+			}
+
+			if ((pgid = getpgid(0)) == -1) {
+				fprintf(fp, "ERROR: getpgid() failed, %s\n", strerror(errno));
+			}
 
 			child = fork();
 			if (child == -1) {
@@ -395,13 +455,33 @@  run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				rc = -1;
 				break;
 			} else if (child == 0) {
-				setsid();
+				close(0);
+				if ((slave = setup_slave_pty(fp)) < 0) {
+					fprintf(fp, "ERROR: could not setup pty (%d).", slave);
+				}
+				if (setpgid(0,pgid) == -1) {
+					fprintf(fp, "ERROR: setpgid() failed, %s\n", strerror(errno));
+				}
+
+				if (setsid() ==  -1) {
+					fprintf(fp, "ERROR: setsid() failed, %s\n", strerror(errno));
+				}
+
+				if (ioctl(0, TIOCSCTTY, NULL) == -1) {
+					fprintf(fp, "ERROR: Unable to attach to controlling tty, %s\n", strerror(errno));
+				}
+
 				run_child(p->run_ptest, pipefd_stdout[1], pipefd_stderr[1]);
+
 			} else {
 				int status;
 				int fds[2]; fds[0] = pipefd_stdout[0]; fds[1] = pipefd_stderr[0];
 				FILE *fps[2]; fps[0] = fp; fps[1] = fp_stderr;
 
+				if (setpgid(child, pgid) == -1) {
+					fprintf(fp, "ERROR: setpgid() failed, %s\n", strerror(errno));
+				}
+
 				sttime = time(NULL);
 				fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, sttime));
 				fprintf(fp, "BEGIN: %s\n", ptest_dir);

Comments

Aníbal Limón July 21, 2019, 7:22 p.m.
Hi Randy,

Thanks for the patches, I pushed this one to master, I will review the
clang warning patches.

Cheers,
Anibal

On Wed, 17 Jul 2019 at 13:35, Randy MacLeod <Randy.MacLeod@windriver.com>
wrote:

> When running the run-execscript bash ptest as a user rather than root, a
> warning:
>   bash: cannot set terminal process group (16036): Inappropriate ioctl for
> device
>   bash: no job control in this shell
> contaminates the bash log files causing the test to fail. This happens only
> when run under ptest-runner and not when interactively testing!
>
> The changes made to fix this include:
> 1. Get the process group id (pgid) before forking,
> 2. Set the pgid in both the parent and child to avoid a race,
> 3. Find, open and set permission on the child tty, and
> 4. Allow the child to attach to controlling tty.
>
> Also add '-lutil' to Makefile. This lib is from libc and provides openpty.
>
> Upstream-Status: Submitted [yocto@yoctoproject.org]
>
> Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
> Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
> ---
>  Makefile |   2 +-
>  utils.c  | 102 +++++++++++++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 92 insertions(+), 12 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 1bde7be..439eb79 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -29,7 +29,7 @@ TEST_DATA=$(shell echo `pwd`/tests/data)
>  all: $(SOURCES) $(EXECUTABLE)
>
>  $(EXECUTABLE): $(OBJECTS)
> -       $(CC) $(LDFLAGS) $(OBJECTS) -o $@
> +       $(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@
>
>  tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)
>
> diff --git a/utils.c b/utils.c
> index ad737c2..f11ce39 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1,5 +1,6 @@
>  /**
>   * Copyright (c) 2016 Intel Corporation
> + * Copyright (C) 2019 Wind River Systems, Inc.
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
> @@ -22,23 +23,27 @@
>   */
>
>  #define _GNU_SOURCE
> +
>  #include <stdio.h>
>
> +#include <dirent.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <grp.h>
>  #include <libgen.h>
> -#include <signal.h>
>  #include <poll.h>
> -#include <fcntl.h>
> +#include <pty.h>
> +#include <signal.h>
> +#include <stdlib.h>
> +#include <string.h>
>  #include <time.h>
> -#include <dirent.h>
> +#include <unistd.h>
> +
> +#include <sys/ioctl.h>
>  #include <sys/resource.h>
> +#include <sys/stat.h>
>  #include <sys/types.h>
>  #include <sys/wait.h>
> -#include <sys/stat.h>
> -#include <unistd.h>
> -#include <string.h>
> -#include <stdlib.h>
> -
> -#include <errno.h>
>
>  #include "ptest_list.h"
>  #include "utils.h"
> @@ -346,6 +351,53 @@ wait_child(const char *ptest_dir, const char
> *run_ptest, pid_t pid,
>         return status;
>  }
>
> +/* Returns an integer file descriptor.
> + * If it returns < 0, an error has occurred.
> + * Otherwise, it has returned the slave pty file descriptor.
> + * fp should be writable, likely stdout/err.
> + */
> +static int
> +setup_slave_pty(FILE *fp) {
> +       int pty_master = -1;
> +       int pty_slave = -1;
> +       char pty_name[256];
> +       struct group *gptr;
> +       gid_t gid;
> +       int slave = -1;
> +
> +       if (openpty(&pty_master, &pty_slave, pty_name, NULL, NULL) < 0) {
> +               fprintf(fp, "ERROR: openpty() failed with: %s.\n",
> strerror(errno));
> +               return -1;
> +       }
> +
> +       if ((gptr = getgrnam(pty_name)) != 0) {
> +               gid = gptr->gr_gid;
> +       } else {
> +               /* If the tty group does not exist, don't change the
> +                * group on the slave pty, only the owner
> +                */
> +               gid = -1;
> +       }
> +
> +       /* chown/chmod the corresponding pty, if possible.
> +        * This will only work if the process has root permissions.
> +        */
> +       if (chown(pty_name, getuid(), gid) != 0) {
> +               fprintf(fp, "ERROR; chown() failed with: %s.\n",
> strerror(errno));
> +       }
> +
> +       /* Makes the slave read/writeable for the user. */
> +       if (chmod(pty_name, S_IRUSR|S_IWUSR) != 0) {
> +               fprintf(fp, "ERROR: chmod() failed with: %s.\n",
> strerror(errno));
> +       }
> +
> +       if ((slave = open(pty_name, O_RDWR)) == -1) {
> +               fprintf(fp, "ERROR: open() failed with: %s.\n",
> strerror(errno));
> +       }
> +       return (slave);
> +}
> +
> +
>  int
>  run_ptests(struct ptest_list *head, const struct ptest_options opts,
>                 const char *progname, FILE *fp, FILE *fp_stderr)
> @@ -362,6 +414,8 @@ run_ptests(struct ptest_list *head, const struct
> ptest_options opts,
>         int timeouted;
>         time_t sttime, entime;
>         int duration;
> +       int slave;
> +       int pgid = -1;
>
>         if (opts.xml_filename) {
>                 xh = xml_create(ptest_list_length(head),
> opts.xml_filename);
> @@ -379,7 +433,6 @@ run_ptests(struct ptest_list *head, const struct
> ptest_options opts,
>                         close(pipefd_stdout[1]);
>                         break;
>                 }
> -
>                 fprintf(fp, "START: %s\n", progname);
>                 PTEST_LIST_ITERATE_START(head, p);
>                         char *ptest_dir = strdup(p->run_ptest);
> @@ -388,6 +441,13 @@ run_ptests(struct ptest_list *head, const struct
> ptest_options opts,
>                                 break;
>                         }
>                         dirname(ptest_dir);
> +                       if (ioctl(0, TIOCNOTTY) == -1) {
> +                               fprintf(fp, "ERROR: Unable to detach from
> controlling tty, %s\n", strerror(errno));
> +                       }
> +
> +                       if ((pgid = getpgid(0)) == -1) {
> +                               fprintf(fp, "ERROR: getpgid() failed,
> %s\n", strerror(errno));
> +                       }
>
>                         child = fork();
>                         if (child == -1) {
> @@ -395,13 +455,33 @@ run_ptests(struct ptest_list *head, const struct
> ptest_options opts,
>                                 rc = -1;
>                                 break;
>                         } else if (child == 0) {
> -                               setsid();
> +                               close(0);
> +                               if ((slave = setup_slave_pty(fp)) < 0) {
> +                                       fprintf(fp, "ERROR: could not
> setup pty (%d).", slave);
> +                               }
> +                               if (setpgid(0,pgid) == -1) {
> +                                       fprintf(fp, "ERROR: setpgid()
> failed, %s\n", strerror(errno));
> +                               }
> +
> +                               if (setsid() ==  -1) {
> +                                       fprintf(fp, "ERROR: setsid()
> failed, %s\n", strerror(errno));
> +                               }
> +
> +                               if (ioctl(0, TIOCSCTTY, NULL) == -1) {
> +                                       fprintf(fp, "ERROR: Unable to
> attach to controlling tty, %s\n", strerror(errno));
> +                               }
> +
>                                 run_child(p->run_ptest, pipefd_stdout[1],
> pipefd_stderr[1]);
> +
>                         } else {
>                                 int status;
>                                 int fds[2]; fds[0] = pipefd_stdout[0];
> fds[1] = pipefd_stderr[0];
>                                 FILE *fps[2]; fps[0] = fp; fps[1] =
> fp_stderr;
>
> +                               if (setpgid(child, pgid) == -1) {
> +                                       fprintf(fp, "ERROR: setpgid()
> failed, %s\n", strerror(errno));
> +                               }
> +
>                                 sttime = time(NULL);
>                                 fprintf(fp, "%s\n", get_stime(stime,
> GET_STIME_BUF_SIZE, sttime));
>                                 fprintf(fp, "BEGIN: %s\n", ptest_dir);
> --
> 2.17.0
>
>