| Message ID | 20220614102029.13006-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/4] support Dash as default shell | expand |
On Tue, Jun 14, 2022 at 12:21 PM Christian Göttsche <cgzones@googlemail.com> wrote: > Debian uses Dash as default shell and switching via > > dpkg-reconfigure dash > > has become deprecated. > > * Use POSIX compliant `> target 2>&1` instead of `>& target`. I'm fine with this subset of changes. > * Call runcon directly to avoid a fork within Dash, which breaks tests > requiring to not change the PID of executing commands I don't seem to have such problem when I change the default shell to dash on Fedora. Can you provide a minimal reproducer? > * Use bash explicitly for non POSIX read option -t I'd like to try to find some nicer alternative for this one first... If I don't find one, then yours will have to do, I guess. Any specific reason why you used `` instead of system()? AFAIK the only difference is that `` return the command's stdout as a string, while system() returns the exit code and forwards stdout. > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > README.md | 7 ------- > tests/binder/test | 2 +- > tests/bpf/test | 4 ++-- > tests/fdreceive/test | 2 +- > tests/filesystem/Filesystem.pm | 14 +++++++------- > tests/inet_socket/test | 2 +- > tests/ptrace/test | 6 +++--- > tests/sctp/test | 2 +- > tests/sigkill/test | 2 +- > tests/task_getpgid/test | 6 +++--- > tests/task_getscheduler/test | 6 +++--- > tests/task_getsid/test | 6 +++--- > tests/task_setnice/test | 6 +++--- > tests/task_setscheduler/test | 6 +++--- > tests/unix_socket/test | 2 +- > tests/vsock_socket/test | 2 +- > 16 files changed, 34 insertions(+), 41 deletions(-) > (snip) -- Ondrej Mosnacek Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.
On Tue, 14 Jun 2022 at 16:50, Ondrej Mosnacek <omosnace@redhat.com> wrote: > > On Tue, Jun 14, 2022 at 12:21 PM Christian Göttsche > <cgzones@googlemail.com> wrote: > > Debian uses Dash as default shell and switching via > > > > dpkg-reconfigure dash > > > > has become deprecated. > > > > * Use POSIX compliant `> target 2>&1` instead of `>& target`. > > I'm fine with this subset of changes. > > > * Call runcon directly to avoid a fork within Dash, which breaks tests > > requiring to not change the PID of executing commands > > I don't seem to have such problem when I change the default shell to > dash on Fedora. Can you provide a minimal reproducer? ==== test.pl ==== #!/usr/bin/perl $basedir = $0; $basedir =~ s|(.*)/[^/]*|$1|; print "current PID: $$\n"; if ( ( $pid = fork() ) == 0 ) { print "child PID: $$\n"; exec "runcon -t unconfined_execmem_t sh -c 'echo >$basedir/flag; while :; do :; done'"; #alternative: exec 'runcon', '-t', 'unconfined_execmem_t', 'sh', '-c', "echo >$basedir/flag; while :; do :; done"; exit; } # Wait for it to start. #system("bash -c 'read -t 5 <>$basedir/flag'"); `/bin/bash -c 'read -t 5 <>$basedir/flag'`; $exists = kill 0, $pid; if ( $exists ) { print "Process $pid is running:\n"; system("pstree -alpZ $pid"); } else { print "Process $pid is NOT running\n"; } # Kill the process. kill KILL, $pid; exit; ==== test.pl ==== normal; current PID: 8558 child PID: 8559 Process 8559 is running: sh,8559,`unconfined_u:unconfined_r:unconfined_execmem_t:s0-s0:c0.c1023 -c runcon -t unconfined_execmem_t bash -c 'echo >./flag; while :; do :; done' └─bash,8561,`unconfined_u:unconfined_r:unconfined_execmem_t:s0-s0:c0.c1023 -c echo >./flag; while :; do :; done alternative: current PID: 8599 child PID: 8600 Process 8600 is running: sh,8600,`unconfined_u:unconfined_r:unconfined_execmem_t:s0-s0:c0.c1023 -c echo >./flag; while :; do :; done > > * Use bash explicitly for non POSIX read option -t > > I'd like to try to find some nicer alternative for this one first... > If I don't find one, then yours will have to do, I guess. > > Any specific reason why you used `` instead of system()? AFAIK the > only difference is that `` return the command's stdout as a string, > while system() returns the exit code and forwards stdout. > > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > --- > > README.md | 7 ------- > > tests/binder/test | 2 +- > > tests/bpf/test | 4 ++-- > > tests/fdreceive/test | 2 +- > > tests/filesystem/Filesystem.pm | 14 +++++++------- > > tests/inet_socket/test | 2 +- > > tests/ptrace/test | 6 +++--- > > tests/sctp/test | 2 +- > > tests/sigkill/test | 2 +- > > tests/task_getpgid/test | 6 +++--- > > tests/task_getscheduler/test | 6 +++--- > > tests/task_getsid/test | 6 +++--- > > tests/task_setnice/test | 6 +++--- > > tests/task_setscheduler/test | 6 +++--- > > tests/unix_socket/test | 2 +- > > tests/vsock_socket/test | 2 +- > > 16 files changed, 34 insertions(+), 41 deletions(-) > > > (snip) > > -- > Ondrej Mosnacek > Software Engineer, Linux Security - SELinux kernel > Red Hat, Inc. >
On Wed, Jun 15, 2022 at 4:31 PM Christian Göttsche <cgzones@googlemail.com> wrote: > > On Tue, 14 Jun 2022 at 16:50, Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > On Tue, Jun 14, 2022 at 12:21 PM Christian Göttsche > > <cgzones@googlemail.com> wrote: > > > Debian uses Dash as default shell and switching via > > > > > > dpkg-reconfigure dash > > > > > > has become deprecated. > > > > > > * Use POSIX compliant `> target 2>&1` instead of `>& target`. > > > > I'm fine with this subset of changes. > > > > > * Call runcon directly to avoid a fork within Dash, which breaks tests > > > requiring to not change the PID of executing commands > > > > I don't seem to have such problem when I change the default shell to > > dash on Fedora. Can you provide a minimal reproducer? > > > ==== test.pl ==== > #!/usr/bin/perl > > $basedir = $0; > $basedir =~ s|(.*)/[^/]*|$1|; > > print "current PID: $$\n"; > > if ( ( $pid = fork() ) == 0 ) { > print "child PID: $$\n"; > exec "runcon -t unconfined_execmem_t sh -c 'echo >$basedir/flag; > while :; do :; done'"; > #alternative: exec 'runcon', '-t', 'unconfined_execmem_t', 'sh', > '-c', "echo >$basedir/flag; while :; do :; done"; > exit; > } > > # Wait for it to start. > #system("bash -c 'read -t 5 <>$basedir/flag'"); > `/bin/bash -c 'read -t 5 <>$basedir/flag'`; > > $exists = kill 0, $pid; > if ( $exists ) { > print "Process $pid is running:\n"; > system("pstree -alpZ $pid"); > } else { > print "Process $pid is NOT running\n"; > } > > # Kill the process. > kill KILL, $pid; > > exit; > ==== test.pl ==== > > normal; > current PID: 8558 > child PID: 8559 > Process 8559 is running: > sh,8559,`unconfined_u:unconfined_r:unconfined_execmem_t:s0-s0:c0.c1023 > -c runcon -t unconfined_execmem_t bash -c 'echo >./flag; while :; do > :; done' > └─bash,8561,`unconfined_u:unconfined_r:unconfined_execmem_t:s0-s0:c0.c1023 > -c echo >./flag; while :; do :; done Hm, still not able to reproduce this behavior... Perhaps Debian's version of dash doesn't implicitly exec the last command like bash and Fedora 36's dash seem to do? Can you try if just adding "exec " before "runcon" also fixes the issue? exec "exec runcon -t unconfined_execmem_t sh -c 'echo >$basedir/flag; while :; do :; done'"; > > alternative: > current PID: 8599 > child PID: 8600 > Process 8600 is running: > sh,8600,`unconfined_u:unconfined_r:unconfined_execmem_t:s0-s0:c0.c1023 > -c echo >./flag; while :; do :; done -- Ondrej Mosnacek Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.
diff --git a/README.md b/README.md index 29e3421..e90a20d 100644 --- a/README.md +++ b/README.md @@ -147,13 +147,6 @@ On Debian prior to version 11 (bullseye) you need to build and install netlabel_ # make # sudo make install -Debian further requires reconfiguring the default /bin/sh to be bash -to support bashisms employed in the testsuite Makefiles and scripts: - - # dpkg-reconfigure dash - -Select "No" when asked if you want to use dash as the default system shell. - #### Other Distributions The testsuite requires a pre-existing base policy configuration of SELinux, diff --git a/tests/binder/test b/tests/binder/test index 14f2096..b35aee1 100755 --- a/tests/binder/test +++ b/tests/binder/test @@ -80,7 +80,7 @@ sub service_start { } # Wait for it to initialize. - system("read -t 5 <>$basedir/$flag"); + `bash -c 'read -t 5 <>$basedir/$flag'`; return $pid; } diff --git a/tests/bpf/test b/tests/bpf/test index 6ab7686..f3147a8 100755 --- a/tests/bpf/test +++ b/tests/bpf/test @@ -106,7 +106,7 @@ if ( ( $pid = fork() ) == 0 ) { } # Wait for it to initialize. -system("read -t 5 <>$basedir/flag"); +`bash -c 'read -t 5 <>$basedir/flag'`; # Test BPF map & prog fd on transfer: $result = system @@ -149,7 +149,7 @@ sub service_start { } # Wait for it to initialize. - system("read -t 5 <>$basedir/$flag"); + `bash -c 'read -t 5 <>$basedir/$flag'`; return $pid; } diff --git a/tests/fdreceive/test b/tests/fdreceive/test index 2415361..4451f7d 100755 --- a/tests/fdreceive/test +++ b/tests/fdreceive/test @@ -22,7 +22,7 @@ if ( ( $pid = fork() ) == 0 ) { } # Wait for it to initialize. -system("read -t 5 <>$basedir/flag"); +`bash -c 'read -t 5 <>$basedir/flag'`; # Verify that test_fdreceive_server_t can receive a rw fd to the test_file # from test_fdreceive_client_t. diff --git a/tests/filesystem/Filesystem.pm b/tests/filesystem/Filesystem.pm index c14e760..e3cd8ee 100644 --- a/tests/filesystem/Filesystem.pm +++ b/tests/filesystem/Filesystem.pm @@ -49,12 +49,12 @@ sub udisks2_stop { $status = 0; if ( -e "/usr/bin/systemctl" ) { - $u_status_cmd = "/usr/bin/systemctl status udisks2 >& /dev/null"; - $u_stop_cmd = "/usr/bin/systemctl stop udisks2 >& /dev/null"; + $u_status_cmd = "/usr/bin/systemctl status udisks2 > /dev/null 2>&1"; + $u_stop_cmd = "/usr/bin/systemctl stop udisks2 > /dev/null 2>&1"; } elsif ( -e "/usr/sbin/service" ) { - $u_status_cmd = "/usr/sbin/service udisks2 status >& /dev/null"; - $u_stop_cmd = "/usr/sbin/service udisks2 stop >& /dev/null"; + $u_status_cmd = "/usr/sbin/service udisks2 status > /dev/null 2>&1"; + $u_stop_cmd = "/usr/sbin/service udisks2 stop > /dev/null 2>&1"; } if ($u_status_cmd) { @@ -78,10 +78,10 @@ sub udisks2_restart { if ( $status eq 3 ) { print "Restarting udisks2 service.\n"; if ( -e "/usr/bin/systemctl" ) { - system("/usr/bin/systemctl start udisks2 >& /dev/null"); + system("/usr/bin/systemctl start udisks2 > /dev/null 2>&1"); } elsif ( -e "/usr/sbin/service" ) { - system("/usr/sbin/service udisks2 start >& /dev/null"); + system("/usr/sbin/service udisks2 start > /dev/null 2>&1"); } } } @@ -133,7 +133,7 @@ sub make_fs { attach_dev( $mk_dev, $mk_dir ); print "Make $mk_type filesystem on $mk_dev\n"; - $result = system("yes | mkfs.$mk_type $mk_dev >& /dev/null"); + $result = system("yes | mkfs.$mk_type $mk_dev > /dev/null 2>&1"); if ( $result != 0 ) { system("losetup -d $mk_dev 2>/dev/null"); print "mkfs.$mk_type failed to create filesystem on $mk_dev\n"; diff --git a/tests/inet_socket/test b/tests/inet_socket/test index f09b4e3..18b1014 100755 --- a/tests/inet_socket/test +++ b/tests/inet_socket/test @@ -59,7 +59,7 @@ sub server_start { } # Wait for it to initialize. - system("read -t 5 <>$basedir/flag"); + `bash -c 'read -t 5 <>$basedir/flag'`; return $pid; } diff --git a/tests/ptrace/test b/tests/ptrace/test index 78589c6..dbbfe5f 100755 --- a/tests/ptrace/test +++ b/tests/ptrace/test @@ -9,13 +9,13 @@ $basedir =~ s|(.*)/[^/]*|$1|; # Start the process to be traced. system("mkfifo $basedir/flag"); if ( ( $pid = fork() ) == 0 ) { - exec -"runcon -t test_ptrace_traced_t sh -c 'echo >$basedir/flag; while :; do :; done'"; + exec 'runcon', '-t', 'test_ptrace_traced_t', 'sh', '-c', + "echo >$basedir/flag; while :; do :; done"; exit; } # Wait for it to start. -system("read -t 5 <>$basedir/flag"); +`bash -c 'read -t 5 <>$basedir/flag'`; # Verify that the nottracer domain cannot attach to the process. # Should fail on the ptrace permission check. diff --git a/tests/sctp/test b/tests/sctp/test index e28d214..078f762 100755 --- a/tests/sctp/test +++ b/tests/sctp/test @@ -120,7 +120,7 @@ sub server_start { } # Wait for it to initialize. - system("read -t 5 <>$basedir/flag"); + `bash -c 'read -t 5 <>$basedir/flag'`; return $pid; } diff --git a/tests/sigkill/test b/tests/sigkill/test index 6c7289a..cd50952 100755 --- a/tests/sigkill/test +++ b/tests/sigkill/test @@ -13,7 +13,7 @@ if ( ( $pid = fork() ) == 0 ) { } # Wait for it to initialize. -system("read -t 5 <>$basedir/flag"); +`bash -c 'read -t 5 <>$basedir/flag'`; # Verify that test_kill_signal_t cannot send CHLD, STOP, or KILL to the server. $result = system "runcon -t test_kill_signal_t -- kill -s CHLD $pid 2>&1"; diff --git a/tests/task_getpgid/test b/tests/task_getpgid/test index ff9ccc6..d1d1847 100755 --- a/tests/task_getpgid/test +++ b/tests/task_getpgid/test @@ -9,12 +9,12 @@ $basedir =~ s|(.*)/[^/]*|$1|; # Start the target process. system("mkfifo $basedir/flag"); if ( ( $pid = fork() ) == 0 ) { - exec -"runcon -t test_getpgid_target_t sh -c 'echo >$basedir/flag; while :; do :; done'"; + exec 'runcon', '-t', 'test_getpgid_target_t', 'sh', '-c', + "echo >$basedir/flag; while :; do :; done"; } # Wait for it to start. -system("read -t 5 <>$basedir/flag"); +`bash -c 'read -t 5 <>$basedir/flag'`; # Verify that test_getpgid_yes_t can get the target's process group ID. $result = system "runcon -t test_getpgid_yes_t -- $basedir/source $pid 2>&1"; diff --git a/tests/task_getscheduler/test b/tests/task_getscheduler/test index ce7f047..e38dd9e 100755 --- a/tests/task_getscheduler/test +++ b/tests/task_getscheduler/test @@ -9,12 +9,12 @@ $basedir =~ s|(.*)/[^/]*|$1|; # Start the target process. system("mkfifo $basedir/flag"); if ( ( $pid = fork() ) == 0 ) { - exec -"runcon -t test_getsched_target_t sh -c 'echo >$basedir/flag; while :; do :; done'"; + exec 'runcon', '-t', 'test_getsched_target_t', 'sh', '-c', + "echo >$basedir/flag; while :; do :; done"; } # Wait for it to start. -system("read -t 5 <>$basedir/flag"); +`bash -c 'read -t 5 <>$basedir/flag'`; # Verify that test_getsched_yes_t can get the scheduling. # SCHED_OTHER 0 priority must == 0 diff --git a/tests/task_getsid/test b/tests/task_getsid/test index 16190c5..30efbfc 100755 --- a/tests/task_getsid/test +++ b/tests/task_getsid/test @@ -9,12 +9,12 @@ $basedir =~ s|(.*)/[^/]*|$1|; # Start the target process. system("mkfifo $basedir/flag"); if ( ( $pid = fork() ) == 0 ) { - exec -"runcon -t test_getsid_target_t sh -c 'echo >$basedir/flag; while :; do :; done'"; + exec 'runcon', '-t', 'test_getsid_target_t', 'sh', '-c', + "echo >$basedir/flag; while :; do :; done"; } # Wait for it to start. -system("read -t 5 <>$basedir/flag"); +`bash -c 'read -t 5 <>$basedir/flag'`; # Verify that test_getsid_yes_t can get the session ID. $result = system "runcon -t test_getsid_yes_t -- $basedir/source $pid 2>&1"; diff --git a/tests/task_setnice/test b/tests/task_setnice/test index 09352ed..ed25884 100755 --- a/tests/task_setnice/test +++ b/tests/task_setnice/test @@ -9,12 +9,12 @@ $basedir =~ s|(.*)/[^/]*|$1|; # Start the process that will have its priority changed. system("mkfifo $basedir/flag"); if ( ( $pid = fork() ) == 0 ) { - exec -"runcon -t test_setsched_target_t sh -c 'echo >$basedir/flag; while :; do :; done'"; + exec 'runcon', '-t', 'test_setsched_target_t', 'sh', '-c', + "echo >$basedir/flag; while :; do :; done"; } # Wait for it to start. -system("read -t 5 <>$basedir/flag"); +`bash -c 'read -t 5 <>$basedir/flag'`; # Verify that test_setsched_yes_t can change the priority up and down. $result = system "runcon -t test_setsched_yes_t -- renice +10 -p $pid 2>&1"; diff --git a/tests/task_setscheduler/test b/tests/task_setscheduler/test index fa7d9cb..42a161a 100755 --- a/tests/task_setscheduler/test +++ b/tests/task_setscheduler/test @@ -9,12 +9,12 @@ $basedir =~ s|(.*)/[^/]*|$1|; # Start the process that will have its priority and scheduling changed. system("mkfifo $basedir/flag"); if ( ( $pid = fork() ) == 0 ) { - exec -"runcon -t test_setsched_target_t sh -c 'echo >$basedir/flag; while :; do sleep 1; done'"; + exec 'runcon', '-t', 'test_setsched_target_t', 'sh', '-c', + "echo >$basedir/flag; while :; do sleep 1; done"; } # Wait for it to start. -system("read -t 5 <>$basedir/flag"); +`bash -c 'read -t 5 <>$basedir/flag'`; $cgroup_cpu = "/sys/fs/cgroup/cpu/tasks"; if ( -w $cgroup_cpu ) { diff --git a/tests/unix_socket/test b/tests/unix_socket/test index c48d1ad..600fc99 100755 --- a/tests/unix_socket/test +++ b/tests/unix_socket/test @@ -38,7 +38,7 @@ sub server_start { } # Wait for it to initialize. - system("read -t 5 <>$basedir/flag"); + `bash -c 'read -t 5 <>$basedir/flag'`; return $pid; } diff --git a/tests/vsock_socket/test b/tests/vsock_socket/test index 41d9bc8..70fde70 100755 --- a/tests/vsock_socket/test +++ b/tests/vsock_socket/test @@ -34,7 +34,7 @@ sub server_start { } # Wait for it to initialize, read port number. - my $port = `read -t 5 <>$basedir/flag; echo \$REPLY`; + my $port = `bash -c 'read -t 5 <>$basedir/flag; echo \$REPLY'`; return ( $pid, $port ); }
Debian uses Dash as default shell and switching via dpkg-reconfigure dash has become deprecated. * Use POSIX compliant `> target 2>&1` instead of `>& target`. * Call runcon directly to avoid a fork within Dash, which breaks tests requiring to not change the PID of executing commands * Use bash explicitly for non POSIX read option -t Signed-off-by: Christian Göttsche <cgzones@googlemail.com> --- README.md | 7 ------- tests/binder/test | 2 +- tests/bpf/test | 4 ++-- tests/fdreceive/test | 2 +- tests/filesystem/Filesystem.pm | 14 +++++++------- tests/inet_socket/test | 2 +- tests/ptrace/test | 6 +++--- tests/sctp/test | 2 +- tests/sigkill/test | 2 +- tests/task_getpgid/test | 6 +++--- tests/task_getscheduler/test | 6 +++--- tests/task_getsid/test | 6 +++--- tests/task_setnice/test | 6 +++--- tests/task_setscheduler/test | 6 +++--- tests/unix_socket/test | 2 +- tests/vsock_socket/test | 2 +- 16 files changed, 34 insertions(+), 41 deletions(-)