* [Patch v3 3/4] ptest-runner: enable child procs as session leader
2019-06-16 15:48 [Patch v3 1/4] util-linux: add setpriv utility Randy MacLeod
2019-06-16 15:48 ` [Patch v3 2/4] libcap-ng: split into libcap-ng/libcap-ng-python Randy MacLeod
@ 2019-06-16 15:48 ` Randy MacLeod
2019-06-16 15:48 ` [Patch v3 4/4] bash: use setpriv, sed.sed to run ptests Randy MacLeod
2 siblings, 0 replies; 5+ messages in thread
From: Randy MacLeod @ 2019-06-16 15:48 UTC (permalink / raw)
To: openembedded-core
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.
Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
---
...s-ensure-child-can-be-session-leader.patch | 212 ++++++++++++++++++
.../ptest-runner/ptest-runner_2.3.1.bb | 4 +-
2 files changed, 215 insertions(+), 1 deletion(-)
create mode 100644 meta/recipes-support/ptest-runner/ptest-runner/0004-utils-ensure-child-can-be-session-leader.patch
diff --git a/meta/recipes-support/ptest-runner/ptest-runner/0004-utils-ensure-child-can-be-session-leader.patch b/meta/recipes-support/ptest-runner/ptest-runner/0004-utils-ensure-child-can-be-session-leader.patch
new file mode 100644
index 0000000000..13b4cbc7fb
--- /dev/null
+++ b/meta/recipes-support/ptest-runner/ptest-runner/0004-utils-ensure-child-can-be-session-leader.patch
@@ -0,0 +1,212 @@
+From 79698d3205dedba887e0d2492de945d3079de029 Mon Sep 17 00:00:00 2001
+From: Randy MacLeod <Randy.MacLeod@windriver.com>
+Date: Thu, 6 Jun 2019 17:03:50 -0400
+Subject: [PATCH] utils: ensure child can be session leader
+
+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
+
diff --git a/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb b/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb
index 0450e18e4e..dec60fcc9b 100644
--- a/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb
+++ b/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb
@@ -13,7 +13,9 @@ PV = "2.3.1+git${SRCPV}"
SRC_URI = "git://git.yoctoproject.org/ptest-runner2 \
file://0001-utils-Ensure-stdout-stderr-are-flushed.patch \
file://0002-use-process-groups-when-spawning.patch \
- file://0003-utils-Ensure-pipes-are-read-after-exit.patch"
+ file://0003-utils-Ensure-pipes-are-read-after-exit.patch \
+ file://0004-utils-ensure-child-can-be-session-leader.patch \
+"
S = "${WORKDIR}/git"
--
2.17.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Patch v3 4/4] bash: use setpriv, sed.sed to run ptests
2019-06-16 15:48 [Patch v3 1/4] util-linux: add setpriv utility Randy MacLeod
2019-06-16 15:48 ` [Patch v3 2/4] libcap-ng: split into libcap-ng/libcap-ng-python Randy MacLeod
2019-06-16 15:48 ` [Patch v3 3/4] ptest-runner: enable child procs as session leader Randy MacLeod
@ 2019-06-16 15:48 ` Randy MacLeod
2 siblings, 0 replies; 5+ messages in thread
From: Randy MacLeod @ 2019-06-16 15:48 UTC (permalink / raw)
To: openembedded-core
The execscript test in bash fails when run with ptest-runner calling
'su', with the error:
bash: cannot set terminal process group (16036): Inappropriate ioctl for device
Even with ptest-runner fixed to make a child process use the right
process group, 'su' still results in the warning above. Use 'setpriv'
instead. 'runuser' was considered and works but depends on pam so it's
ruled out.
Now that all bash tests are run as a user, the patch:
fix-run-coproc-run-heredoc-run-execscript-run-test-f.patch
can be removed. Also to create the account 'bashtest' in the
'run-ptest' script the bash-ptest must depend on 'shadow'. Also,
in 'run-ptest', ensure that the bash ptests are owned by the 'bashtest' user.
Add 'sed' as a dependency for ptests since tests/exp8.sub runs:
var=$'x\001y\177z'
declare -p var | sed -n l
and that results in:
sed.busybox: ""
sed.sed: declare -- var="x\001y\177z"$
This appears to be a feature that busybox sed has not implemented.
With this series of changes, bash-ptest for qemux86-64 passes
79 of 81 tests. The remaining failures are:
1. run-read:
# cat tests/read6.sub
# test read with a timeout of 0 -- input polling
# sleep with fractional seconds argument is not universal
echo abcde | { sleep 0.25 2>/dev/null ; read -t 0; }
echo $?
read -t 0 < $0
echo $?
read -t 0
echo $? <-- returns 1, when 0 is expected.
I can reproduce this on my workstation but only when using ptest-runner
and initially logging into the console as root. That's a little odd and
seems like I need to continue to improve ptest-runner.
2. run-trap:
# cat tests/trap3.sub
PS4='+[$LINENO] '
trap 'echo trap: $LINENO' ERR
set -x
echo 1
echo 2
echo 3 | cat | false <--- error
echo 4
This is a scheduler behaviour difference between the common case
on a workstation and the common case in qemu. The test case does
warn about the completion order not being deterministic so I plan
to ignore it.
From tests/run-trap:
UNIX versions number signals and schedule processes differently.
If output differing only in line numbers is produced, please
do not consider this a test failure.
Still, it's notable and slightly odd that the common case output
is different.
Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
---
meta/recipes-extended/bash/bash.inc | 5 +--
...un-heredoc-run-execscript-run-test-f.patch | 45 -------------------
.../bash/bash/run-bash-ptests | 4 ++
meta/recipes-extended/bash/bash/run-ptest | 10 ++---
meta/recipes-extended/bash/bash_5.0.bb | 2 +-
5 files changed, 12 insertions(+), 54 deletions(-)
delete mode 100644 meta/recipes-extended/bash/bash/fix-run-coproc-run-heredoc-run-execscript-run-test-f.patch
create mode 100644 meta/recipes-extended/bash/bash/run-bash-ptests
diff --git a/meta/recipes-extended/bash/bash.inc b/meta/recipes-extended/bash/bash.inc
index 4cf1054967..c91cc8ada8 100644
--- a/meta/recipes-extended/bash/bash.inc
+++ b/meta/recipes-extended/bash/bash.inc
@@ -23,7 +23,7 @@ ALTERNATIVE_PRIORITY = "100"
RDEPENDS_${PN} += "base-files"
RDEPENDS_${PN}_class-nativesdk = ""
-RDEPENDS_${PN}-ptest += "make coreutils perl"
+RDEPENDS_${PN}-ptest += "make coreutils perl sed shadow util-linux-setpriv"
DEPENDS_append_libc-glibc = " virtual/libc-locale"
RDEPENDS_${PN}-ptest_append_libc-glibc = " \
@@ -36,8 +36,6 @@ RDEPENDS_${PN}-ptest_append_libc-glibc = " \
locale-base-fr-fr.iso-8859-1 \
locale-base-zh-hk.big5-hkscs \
"
-USERADD_PACKAGES = "${PN}-ptest"
-USERADD_PARAM_${PN}-ptest = "--create-home --user-group test"
CACHED_CONFIGUREVARS += "headersdir=${includedir}/${PN}"
@@ -74,6 +72,7 @@ do_install_append_class-target () {
do_install_ptest () {
make INSTALL_TEST_DIR=${D}${PTEST_PATH}/tests install-test
cp ${B}/Makefile ${D}${PTEST_PATH}
+ install -D ${WORKDIR}/run-bash-ptests ${D}${PTEST_PATH}/run-bash-ptests
sed -i -e 's/^Makefile/_Makefile/' -e "s,--sysroot=${STAGING_DIR_TARGET},,g" \
-e 's|${DEBUG_PREFIX_MAP}||g' \
-e "s,${S},,g" -e "s,${B},,g" -e "s,${STAGING_DIR_NATIVE},,g" \
diff --git a/meta/recipes-extended/bash/bash/fix-run-coproc-run-heredoc-run-execscript-run-test-f.patch b/meta/recipes-extended/bash/bash/fix-run-coproc-run-heredoc-run-execscript-run-test-f.patch
deleted file mode 100644
index 9ac2461ab6..0000000000
--- a/meta/recipes-extended/bash/bash/fix-run-coproc-run-heredoc-run-execscript-run-test-f.patch
+++ /dev/null
@@ -1,45 +0,0 @@
-From d1cd4c31ea0ed7406a3ad4bdaa211f581063f655 Mon Sep 17 00:00:00 2001
-From: Hongxu Jia <hongxu.jia@windriver.com>
-Date: Tue, 15 Aug 2017 10:21:21 +0800
-Subject: [PATCH 2/2] fix run-execscript/run-test/ failed
-
-FAIL: run-execscript:
-the test suite should not be run as root
-
-FAIL: run-test
-the test suite should not be run as root
-
-Upstream-Status: Pending
-
-Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
----
- tests/run-execscript | 3 ++-
- tests/run-test | 3 ++-
- 2 files changed, 4 insertions(+), 2 deletions(-)
-
-diff --git a/tests/run-execscript b/tests/run-execscript
-index de78644..38397c1 100644
---- a/tests/run-execscript
-+++ b/tests/run-execscript
-@@ -5,5 +5,6 @@ echo "warning: \`/tmp/bash-notthere' not being found or \`/' being a directory"
- echo "warning: produce diff output, please do not consider this a test failure" >&2
- echo "warning: if diff output differing only in the location of the bash" >&2
- echo "warning: binary appears, please do not consider this a test failure" >&2
--${THIS_SH} ./execscript > ${BASH_TSTOUT} 2>&1
-+rm -f ${BASH_TSTOUT}
-+su -c "${THIS_SH} ./execscript > ${BASH_TSTOUT} 2>&1" test
- diff ${BASH_TSTOUT} exec.right && rm -f ${BASH_TSTOUT}
-diff --git a/tests/run-test b/tests/run-test
-index d68791c..d6317d2 100644
---- a/tests/run-test
-+++ b/tests/run-test
-@@ -1,4 +1,5 @@
- unset GROUPS UID 2>/dev/null
-
--${THIS_SH} ./test.tests >${BASH_TSTOUT} 2>&1
-+rm -f ${BASH_TSTOUT}
-+su -c "${THIS_SH} ./test.tests > ${BASH_TSTOUT} 2>&1" test
- diff ${BASH_TSTOUT} test.right && rm -f ${BASH_TSTOUT}
---
-1.8.3.1
-
diff --git a/meta/recipes-extended/bash/bash/run-bash-ptests b/meta/recipes-extended/bash/bash/run-bash-ptests
new file mode 100644
index 0000000000..d73a27224d
--- /dev/null
+++ b/meta/recipes-extended/bash/bash/run-bash-ptests
@@ -0,0 +1,4 @@
+#!/bin/sh
+cd "$(dirname "$0")"
+make -k THIS_SH=/bin/bash BUILD_DIR=`pwd` srcdir=`pwd` runtest
+
diff --git a/meta/recipes-extended/bash/bash/run-ptest b/meta/recipes-extended/bash/bash/run-ptest
index c0cf27b6c2..738ad3c42c 100644
--- a/meta/recipes-extended/bash/bash/run-ptest
+++ b/meta/recipes-extended/bash/bash/run-ptest
@@ -19,8 +19,8 @@ then
echo "Warning: The de_DE* locales is needed to run the intl.tests, please add it."
fi
-useradd bash_user
-chown -R bash_user:bash_user ./tests
-su bash_user -c "make -k THIS_SH=/bin/bash BUILD_DIR=`pwd` srcdir=`pwd` runtest"
-chown -R root:root ./tests
-userdel bash_user
+useradd bashtest
+chown -R bashtest:bashtest tests
+setpriv --reuid bashtest --rgid bashtest --clear-groups --reset-env $(dirname "$0")/run-bash-ptests
+chown -R root:root tests
+userdel -r bashtest
diff --git a/meta/recipes-extended/bash/bash_5.0.bb b/meta/recipes-extended/bash/bash_5.0.bb
index e60e5304a5..eadc82279d 100644
--- a/meta/recipes-extended/bash/bash_5.0.bb
+++ b/meta/recipes-extended/bash/bash_5.0.bb
@@ -16,8 +16,8 @@ SRC_URI = "${GNU_MIRROR}/bash/${BP}.tar.gz;name=tarball \
file://mkbuiltins_have_stringize.patch \
file://build-tests.patch \
file://test-output.patch \
- file://fix-run-coproc-run-heredoc-run-execscript-run-test-f.patch \
file://run-ptest \
+ file://run-bash-ptests \
file://fix-run-builtins.patch \
"
--
2.17.0
^ permalink raw reply related [flat|nested] 5+ messages in thread