bug#68707: [PATCH] env,kill: Handle unnamed signals

[Grisha Levit]

Android reserves [1] some realtime signals and redefines [2] SIGRTMIN,
 leaving a gap between the signals that have SIG* constants defined in
 signal.h and SIGRTMIN.
 
 When passed such a signal number, gnulib sig2str returns -1 and leaves
 its signame argument unchanged.
 
 The signal listing in env ends up reusing the name of the last printed
 valid signal:
 
     $ env --list-signal-handling true
     HUP        ( 1): IGNORE
     HUP        (32): BLOCK
     HUP        (38): IGNORE
 
 ..and the corresponding signal numbers are rejected as operands for the
 env, kill, and timeout commands.
 
 This patch removes the requirement that sig2str returns 0 for a signal
 number associated with an operand, and allows unnamed signals to be in
 the sets `env' attempts to manipulate when a --*-signal option is used
 with no argument.
 
 This does leave the possibility of numbers lower than SIGNUM_BOUND that
 are not valid signals, but I'm not sure that's really a problem for the
 existing code paths (if it is, adding checks for sigset_t manipulations
 would probably be enough).
 
 To be on the safe side, added a check to report kill(3) EINVAL as a bad
 signo (rather than always blaming the pid).
 
 This does not change the default list printed with `kill -l'.  When a
 name is to be printed, the signal number associated with an unnamed
 signal is used.
 
 [1]: 
https://cs.android.com/android/platform/superproject/main/+/main:bionic/libc/platform/bionic/reserved_signals.h
 [2]: 
https://cs.android.com/android/platform/superproject/main/+/main:bionic/libc/include/signal.h;l=51


* src/operand2sig.c (operand2sig): Drop signame argument, accept all
signal numbers <= SIGNUM_BOUND.  All callers updated.
* src/env.c (parse_signal_action_params, reset_signal_handlers)
(parse_block_signal_params, set_signal_proc_mask)
(list_signal_handling): Accept all signal numbers <= SIGNUM_BOUND,
use signal number for printing if necessary.
* src/kill.c (list_signals, main): Likewise.
(send_signals): Check errno from kill(3) for bad signo.
* src/timeout.c (main): Update operand2sig call.
* tests/misc/kill.sh: Test listing all signal numbers.
---
 src/env.c          | 18 +++++++++---------
 src/kill.c         | 29 ++++++++++++++++-------------
 src/operand2sig.c  |  8 ++++----
 src/operand2sig.h  |  2 +-
 src/timeout.c      |  3 +--
 tests/misc/kill.sh |  8 ++++++++
 6 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/src/env.c b/src/env.c
index ffe896039..e40767610 100644
--- a/src/env.c
+++ b/src/env.c
@@ -538,7 +538,6 @@ parse_split_string (char const *str, int *orig_optind,
 static void
 parse_signal_action_params (char const *optarg, bool set_default)
 {
-  char signame[SIG2STR_MAX];
   char *opt_sig;
   char *optarg_writable;
 
@@ -548,8 +547,7 @@ parse_signal_action_params (char const *optarg, bool 
set_default)
          Some signals cannot be set to ignore or default (e.g., SIGKILL,
          SIGSTOP on most OSes, and SIGCONT on AIX.) - so ignore errors.  */
       for (int i = 1 ; i <= SIGNUM_BOUND; i++)
-        if (sig2str (i, signame) == 0)
-          signals[i] = set_default ? DEFAULT_NOERR : IGNORE_NOERR;
+        signals[i] = set_default ? DEFAULT_NOERR : IGNORE_NOERR;
       return;
     }
 
@@ -558,7 +556,7 @@ parse_signal_action_params (char const *optarg, bool 
set_default)
   opt_sig = strtok (optarg_writable, ",");
   while (opt_sig)
     {
-      int signum = operand2sig (opt_sig, signame);
+      int signum = operand2sig (opt_sig);
       /* operand2sig accepts signal 0 (EXIT) - but we reject it.  */
       if (signum == 0)
         error (0, 0, _("%s: invalid signal"), quote (opt_sig));
@@ -607,7 +605,8 @@ reset_signal_handlers (void)
       if (dev_debug)
         {
           char signame[SIG2STR_MAX];
-          sig2str (i, signame);
+          if (sig2str (i, signame) != 0)
+            snprintf (signame, sizeof signame, "%d", i);
           devmsg ("Reset signal %s (%d) to %s%s\n",
                   signame, i,
                   set_to_default ? "DEFAULT" : "IGNORE",
@@ -620,7 +619,6 @@ reset_signal_handlers (void)
 static void
 parse_block_signal_params (char const *optarg, bool block)
 {
-  char signame[SIG2STR_MAX];
   char *opt_sig;
   char *optarg_writable;
 
@@ -647,7 +645,7 @@ parse_block_signal_params (char const *optarg, bool block)
   opt_sig = strtok (optarg_writable, ",");
   while (opt_sig)
     {
-      int signum = operand2sig (opt_sig, signame);
+      int signum = operand2sig (opt_sig);
       /* operand2sig accepts signal 0 (EXIT) - but we reject it.  */
       if (signum == 0)
         error (0, 0, _("%s: invalid signal"), quote (opt_sig));
@@ -695,7 +693,8 @@ set_signal_proc_mask (void)
       if (dev_debug && debug_act)
         {
           char signame[SIG2STR_MAX];
-          sig2str (i, signame);
+          if (sig2str (i, signame) != 0)
+            snprintf (signame, sizeof signame, "%d", i);
           devmsg ("signal %s (%d) mask set to %s\n",
                   signame, i, debug_act);
         }
@@ -728,7 +727,8 @@ list_signal_handling (void)
       if (! *ignored && ! *blocked)
         continue;
 
-      sig2str (i, signame);
+      if (sig2str (i, signame) != 0)
+        snprintf (signame, sizeof signame, "%d", i);
       fprintf (stderr, "%-10s (%2d): %s%s%s\n", signame, i,
                blocked, connect, ignored);
     }
diff --git a/src/kill.c b/src/kill.c
index 9c8b6c191..f386c9cbc 100644
--- a/src/kill.c
+++ b/src/kill.c
@@ -131,11 +131,15 @@ list_signals (bool table, char *const *argv)
       if (argv)
         for (; *argv; argv++)
           {
-            signum = operand2sig (*argv, signame);
+            signum = operand2sig (*argv);
             if (signum < 0)
               status = EXIT_FAILURE;
             else
-              print_table_row (num_width, signum, name_width, signame);
+              {
+                if (sig2str (signum, signame) != 0)
+                  snprintf (signame, sizeof signame, "%d", signum);
+                print_table_row (num_width, signum, name_width, signame);
+              }
           }
       else
         for (signum = 1; signum <= SIGNUM_BOUND; signum++)
@@ -147,16 +151,13 @@ list_signals (bool table, char *const *argv)
       if (argv)
         for (; *argv; argv++)
           {
-            signum = operand2sig (*argv, signame);
+            signum = operand2sig (*argv);
             if (signum < 0)
               status = EXIT_FAILURE;
+            else if (ISDIGIT (**argv) && sig2str (signum, signame) == 0)
+              puts (signame);
             else
-              {
-                if (ISDIGIT (**argv))
-                  puts (signame);
-                else
-                  printf ("%d\n", signum);
-              }
+              printf ("%d\n", signum);
           }
       else
         for (signum = 1; signum <= SIGNUM_BOUND; signum++)
@@ -188,9 +189,12 @@ send_signals (int signum, char *const *argv)
           error (0, 0, _("%s: invalid process id"), quote (arg));
           status = EXIT_FAILURE;
         }
-      else if (kill (pid, signum) != 0)
+      else if (errno = 0, kill (pid, signum) != 0)
         {
-          error (0, errno, "%s", quote (arg));
+          if (errno == EINVAL)
+            error (0, errno, "%d", signum);
+          else
+            error (0, errno, "%s", quote (arg));
           status = EXIT_FAILURE;
         }
     }
@@ -206,7 +210,6 @@ main (int argc, char **argv)
   bool list = false;
   bool table = false;
   int signum = -1;
-  char signame[SIG2STR_MAX];
 
   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
@@ -251,7 +254,7 @@ main (int argc, char **argv)
             error (0, 0, _("%s: multiple signals specified"), quote (optarg));
             usage (EXIT_FAILURE);
           }
-        signum = operand2sig (optarg, signame);
+        signum = operand2sig (optarg);
         if (signum < 0)
           usage (EXIT_FAILURE);
         break;
diff --git a/src/operand2sig.c b/src/operand2sig.c
index 2a2563c62..b46cb1bed 100644
--- a/src/operand2sig.c
+++ b/src/operand2sig.c
@@ -18,8 +18,8 @@
    FIXME: Move this to gnulib/str2sig.c */
 
 
-/* Convert OPERAND to a signal number with printable representation SIGNAME.
-   Return the signal number, or -1 if unsuccessful.  */
+/* Convert OPERAND to a signal number.  Return the signal number, or -1 if
+   unsuccessful.  */
 
 #include <config.h>
 #include <stdio.h>
@@ -32,7 +32,7 @@
 #include "operand2sig.h"
 
 extern int
-operand2sig (char const *operand, char *signame)
+operand2sig (char const *operand)
 {
   int signum;
 
@@ -82,7 +82,7 @@ operand2sig (char const *operand, char *signame)
       free (upcased);
     }
 
-  if (signum < 0 || sig2str (signum, signame) != 0)
+  if (0 > signum || signum > SIGNUM_BOUND)
     {
       error (0, 0, _("%s: invalid signal"), quote (operand));
       return -1;
diff --git a/src/operand2sig.h b/src/operand2sig.h
index e46689e7b..3bc551051 100644
--- a/src/operand2sig.h
+++ b/src/operand2sig.h
@@ -15,5 +15,5 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <https://www.gnu.org/licenses/>.  */
 
-extern int operand2sig (char const *operand, char *signame)
+extern int operand2sig (char const *operand)
   _GL_ATTRIBUTE_NONNULL ();
diff --git a/src/timeout.c b/src/timeout.c
index 85d97c0b5..7d1ea7da6 100644
--- a/src/timeout.c
+++ b/src/timeout.c
@@ -462,7 +462,6 @@ int
 main (int argc, char **argv)
 {
   double timeout;
-  char signame[SIG2STR_MAX];
   int c;
 
   initialize_main (&argc, &argv);
@@ -483,7 +482,7 @@ main (int argc, char **argv)
           break;
 
         case 's':
-          term_signal = operand2sig (optarg, signame);
+          term_signal = operand2sig (optarg);
           if (term_signal == -1)
             usage (EXIT_CANCELED);
           break;
diff --git a/tests/misc/kill.sh b/tests/misc/kill.sh
index 69679e5a6..79a93de5f 100755
--- a/tests/misc/kill.sh
+++ b/tests/misc/kill.sh
@@ -60,4 +60,12 @@ returns_ 1 env kill -l -1 || fail=1
 returns_ 1 env kill -l -1 0 || fail=1
 returns_ 1 env kill -l INVALID TERM || fail=1
 
+# Verify all signal numbers can be listed
+SIG_LAST_STR=$(env kill -l | tail -n1) || framework_failure_
+SIG_LAST_NUM=$(env kill -l -- "$SIG_LAST_STR") || framework_failure_
+SIG_SEQ=$(env seq -- 0 "$SIG_LAST_NUM") || framework_failure_
+test -n "$SIG_SEQ" || framework_failure_
+env kill -l -- $SIG_SEQ || fail=1
+env kill -t -- $SIG_SEQ || fail=1
+
 Exit $fail
-- 
2.43.0