[PATCH] reset signal handlers before running scripts/hooks

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] reset signal handlers before running scripts/hooks

Andrew Gregory
Front-ends or libraries may set signals to be ignored, which gets
inherited across fork and exec.  This can cause scripts to malfunction
if they expect the signal.  To make matters worse, scripts written in
bash can't reset signals that were ignored when bash was started.

Fixes FS#56756

Signed-off-by: Andrew Gregory <[hidden email]>
---

Hopefully nobody ignores or depends on signals outside of the POSIX
list.  As far as I can tell, the only way to reset all signal handlers
is to iterate over the entire range of positive integers.

 lib/libalpm/util.c                          | 20 ++++++++++++++++++++
 test/pacman/tests/TESTS                     |  1 +
 test/pacman/tests/scriptlet-signal-reset.py | 11 +++++++++++
 3 files changed, 32 insertions(+)
 create mode 100644 test/pacman/tests/scriptlet-signal-reset.py

diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index a06f5bfd..eaf85e93 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -548,6 +548,25 @@ static int _alpm_chroot_read_from_child(alpm_handle_t *handle, int fd,
  return 0;
 }
 
+static void _alpm_reset_signals(void)
+{
+ /* reset POSIX defined signals (see signal.h) */
+ /* there are likely more but there is no easy way
+ * to get the full list of valid signals */
+ int *i, signals[] = {
+ SIGABRT, SIGALRM, SIGBUS, SIGCHLD, SIGCONT, SIGFPE, SIGHUP, SIGILL,
+ SIGINT, SIGKILL, SIGPIPE, SIGQUIT, SIGSEGV, SIGSTOP, SIGTERM, SIGTSTP,
+ SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPOLL, SIGPROF, SIGSYS, SIGTRAP,
+ SIGURG, SIGVTALRM, SIGXCPU, SIGXFSZ,
+ 0
+ };
+ struct sigaction def;
+ def.sa_handler = SIG_DFL;
+ for(i = signals; *i; i++) {
+ sigaction(*i, &def, NULL);
+ }
+}
+
 /** Execute a command with arguments in a chroot.
  * @param handle the context handle
  * @param cmd command to execute
@@ -633,6 +652,7 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[],
  exit(1);
  }
  umask(0022);
+ _alpm_reset_signals();
  execv(cmd, argv);
  /* execv only returns if there was an error */
  fprintf(stderr, _("call to execv failed (%s)\n"), strerror(errno));
diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS
index b11cb511..5deb93c4 100644
--- a/test/pacman/tests/TESTS
+++ b/test/pacman/tests/TESTS
@@ -150,6 +150,7 @@ TESTS += test/pacman/tests/replace102.py
 TESTS += test/pacman/tests/replace103.py
 TESTS += test/pacman/tests/replace104.py
 TESTS += test/pacman/tests/replace110.py
+TESTS += test/pacman/tests/scriptlet-signal-reset.py
 TESTS += test/pacman/tests/scriptlet001.py
 TESTS += test/pacman/tests/scriptlet002.py
 TESTS += test/pacman/tests/sign001.py
diff --git a/test/pacman/tests/scriptlet-signal-reset.py b/test/pacman/tests/scriptlet-signal-reset.py
new file mode 100644
index 00000000..27246d12
--- /dev/null
+++ b/test/pacman/tests/scriptlet-signal-reset.py
@@ -0,0 +1,11 @@
+self.description = "Reset signals before running scriptlets/hooks"
+
+p1 = pmpkg("dummy")
+# check if SIGPIPE is ignored, it should be fatal, but GPGME ignores it
+p1.install['post_install'] = "kill -PIPE $$; echo fail > sigpipe_was_ignored"
+self.addpkg(p1)
+
+self.args = "-U %s" % p1.filename()
+
+self.addrule("PACMAN_RETCODE=0")
+self.addrule("!FILE_EXIST=sigpipe_was_ignored")
--
2.19.0
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] reset signal handlers before running scripts/hooks

Allan McRae
On 3/10/18 5:42 pm, Andrew Gregory wrote:

> Front-ends or libraries may set signals to be ignored, which gets
> inherited across fork and exec.  This can cause scripts to malfunction
> if they expect the signal.  To make matters worse, scripts written in
> bash can't reset signals that were ignored when bash was started.
>
> Fixes FS#56756
>
> Signed-off-by: Andrew Gregory <[hidden email]>
> ---
>
> Hopefully nobody ignores or depends on signals outside of the POSIX
> list.  As far as I can tell, the only way to reset all signal handlers
> is to iterate over the entire range of positive integers.

I know of no other way either.  Patch looks fine.

A

>
>  lib/libalpm/util.c                          | 20 ++++++++++++++++++++
>  test/pacman/tests/TESTS                     |  1 +
>  test/pacman/tests/scriptlet-signal-reset.py | 11 +++++++++++
>  3 files changed, 32 insertions(+)
>  create mode 100644 test/pacman/tests/scriptlet-signal-reset.py
>
> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index a06f5bfd..eaf85e93 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -548,6 +548,25 @@ static int _alpm_chroot_read_from_child(alpm_handle_t *handle, int fd,
>   return 0;
>  }
>  
> +static void _alpm_reset_signals(void)
> +{
> + /* reset POSIX defined signals (see signal.h) */
> + /* there are likely more but there is no easy way
> + * to get the full list of valid signals */
> + int *i, signals[] = {
> + SIGABRT, SIGALRM, SIGBUS, SIGCHLD, SIGCONT, SIGFPE, SIGHUP, SIGILL,
> + SIGINT, SIGKILL, SIGPIPE, SIGQUIT, SIGSEGV, SIGSTOP, SIGTERM, SIGTSTP,
> + SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPOLL, SIGPROF, SIGSYS, SIGTRAP,
> + SIGURG, SIGVTALRM, SIGXCPU, SIGXFSZ,
> + 0
> + };
> + struct sigaction def;
> + def.sa_handler = SIG_DFL;
> + for(i = signals; *i; i++) {
> + sigaction(*i, &def, NULL);
> + }
> +}
> +
>  /** Execute a command with arguments in a chroot.
>   * @param handle the context handle
>   * @param cmd command to execute
> @@ -633,6 +652,7 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[],
>   exit(1);
>   }
>   umask(0022);
> + _alpm_reset_signals();
>   execv(cmd, argv);
>   /* execv only returns if there was an error */
>   fprintf(stderr, _("call to execv failed (%s)\n"), strerror(errno));
> diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS
> index b11cb511..5deb93c4 100644
> --- a/test/pacman/tests/TESTS
> +++ b/test/pacman/tests/TESTS
> @@ -150,6 +150,7 @@ TESTS += test/pacman/tests/replace102.py
>  TESTS += test/pacman/tests/replace103.py
>  TESTS += test/pacman/tests/replace104.py
>  TESTS += test/pacman/tests/replace110.py
> +TESTS += test/pacman/tests/scriptlet-signal-reset.py
>  TESTS += test/pacman/tests/scriptlet001.py
>  TESTS += test/pacman/tests/scriptlet002.py
>  TESTS += test/pacman/tests/sign001.py
> diff --git a/test/pacman/tests/scriptlet-signal-reset.py b/test/pacman/tests/scriptlet-signal-reset.py
> new file mode 100644
> index 00000000..27246d12
> --- /dev/null
> +++ b/test/pacman/tests/scriptlet-signal-reset.py
> @@ -0,0 +1,11 @@
> +self.description = "Reset signals before running scriptlets/hooks"
> +
> +p1 = pmpkg("dummy")
> +# check if SIGPIPE is ignored, it should be fatal, but GPGME ignores it
> +p1.install['post_install'] = "kill -PIPE $$; echo fail > sigpipe_was_ignored"
> +self.addpkg(p1)
> +
> +self.args = "-U %s" % p1.filename()
> +
> +self.addrule("PACMAN_RETCODE=0")
> +self.addrule("!FILE_EXIST=sigpipe_was_ignored")
>