Add argument checking to sigemptyset(3) and friends.

You could argue that this is hurting people smart enough to have manually
allocated a large-enough sigset_t, but those people are smart enough to
implement their own sigset functions too.

I wonder whether our least unpleasant way out of our self-inflicted 32-bit
cesspool is to have equivalents of _FILE_OFFSET_BITS such as _SIGSET_T_BITS,
so calling code could opt in? You'd have to be careful passing sigset_t
arguments between code compiled with different options.

Bug: 5828899
Change-Id: I0ae60ee8544835b069a2b20568f38ec142e0737b

libc/include/signal.h

@@ -28,6 +28,7 @@
 #ifndef _SIGNAL_H_
 #define _SIGNAL_H_
 
+#include <errno.h>
 #include <sys/cdefs.h>
 #include <limits.h>		/* For LONG_BIT */
 #include <string.h>		/* For memset() */
@@ -53,45 +54,57 @@ typedef int sig_atomic_t;
 #  define _NSIG  64
 #endif
 
-extern const char * const sys_siglist[];
-extern const char * const sys_signame[];
+extern const char* const sys_siglist[];
+extern const char* const sys_signame[];
 
-static __inline__ int sigismember(sigset_t *set, int signum)
-{
-    unsigned long *local_set = (unsigned long *)set;
-    signum--;
-    return (int)((local_set[signum/LONG_BIT] >> (signum%LONG_BIT)) & 1);
+static __inline__ int sigismember(sigset_t* set, int signum) {
+  if (set == NULL || signum < 1 || signum >= 8*sizeof(sigset_t)) {
+    errno = EINVAL;
+    return -1;
+  }
+  unsigned long* local_set = (unsigned long*) set;
+  signum--;
+  return (int) ((local_set[signum/LONG_BIT] >> (signum%LONG_BIT)) & 1);
 }
 
-
-static __inline__ int sigaddset(sigset_t *set, int signum)
-{
-    unsigned long *local_set = (unsigned long *)set;
-    signum--;
-    local_set[signum/LONG_BIT] |= 1UL << (signum%LONG_BIT);
-    return 0;
+static __inline__ int sigaddset(sigset_t* set, int signum) {
+  if (set == NULL || signum < 1 || signum >= 8*sizeof(sigset_t)) {
+    errno = EINVAL;
+    return -1;
+  }
+  unsigned long* local_set = (unsigned long*) set;
+  signum--;
+  local_set[signum/LONG_BIT] |= 1UL << (signum%LONG_BIT);
+  return 0;
 }
 
-
-static __inline__ int sigdelset(sigset_t *set, int signum)
-{
-    unsigned long *local_set = (unsigned long *)set;
-    signum--;
-    local_set[signum/LONG_BIT] &= ~(1UL << (signum%LONG_BIT));
-    return 0;
+static __inline__ int sigdelset(sigset_t* set, int signum) {
+  if (set == NULL || signum < 1 || signum >= 8*sizeof(sigset_t)) {
+    errno = EINVAL;
+    return -1;
+  }
+  unsigned long* local_set = (unsigned long*) set;
+  signum--;
+  local_set[signum/LONG_BIT] &= ~(1UL << (signum%LONG_BIT));
+  return 0;
 }
 
-
-static __inline__ int sigemptyset(sigset_t *set)
-{
-    memset(set, 0, sizeof *set);
-    return 0;
+static __inline__ int sigemptyset(sigset_t* set) {
+  if (set == NULL) {
+    errno = EINVAL;
+    return -1;
+  }
+  memset(set, 0, sizeof *set);
+  return 0;
 }
 
-static __inline__ int sigfillset(sigset_t *set)
-{
-    memset(set, ~0, sizeof *set);
-    return 0;
+static __inline__ int sigfillset(sigset_t* set) {
+  if (set == NULL) {
+    errno = EINVAL;
+    return -1;
+  }
+  memset(set, ~0, sizeof *set);
+  return 0;
 }
 
 

tests/Android.mk

@@ -59,6 +59,7 @@ test_src_files = \
     libgen_test.cpp \
     pthread_test.cpp \
     regex_test.cpp \
+    signal_test.cpp \
     stack_protector_test.cpp \
     stdio_test.cpp \
     stdlib_test.cpp \

tests/signal_test.cpp

@@ -0,0 +1,97 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <gtest/gtest.h>
+
+#include <errno.h>
+#include <signal.h>
+
+template <typename Fn>
+static void TestSigSet1(Fn fn) {
+  // NULL sigset_t*.
+  sigset_t* set_ptr = NULL;
+  errno = 0;
+  ASSERT_EQ(-1, fn(set_ptr));
+  ASSERT_EQ(EINVAL, errno);
+
+  // Non-NULL.
+  sigset_t set;
+  errno = 0;
+  ASSERT_EQ(0, fn(&set));
+  ASSERT_EQ(0, errno);
+}
+
+template <typename Fn>
+static void TestSigSet2(Fn fn) {
+  // NULL sigset_t*.
+  sigset_t* set_ptr = NULL;
+  errno = 0;
+  ASSERT_EQ(-1, fn(set_ptr, SIGSEGV));
+  ASSERT_EQ(EINVAL, errno);
+
+  sigset_t set;
+  sigemptyset(&set);
+
+  int min_signal = SIGHUP;
+  int max_signal = SIGRTMAX;
+
+#if __BIONIC__
+  // bionic's sigset_t is too small: 32 bits instead of 64.
+  // This means you can't refer to any of the real-time signals.
+  // See http://b/3038348 and http://b/5828899.
+  max_signal = 31;
+#else
+  // Other C libraries are perfectly capable of using their largest signal.
+  ASSERT_GE(sizeof(sigset_t) * 8, static_cast<size_t>(SIGRTMAX));
+#endif
+
+  // Bad signal number: too small.
+  errno = 0;
+  ASSERT_EQ(-1, fn(&set, 0));
+  ASSERT_EQ(EINVAL, errno);
+
+  // Bad signal number: too high.
+  errno = 0;
+  ASSERT_EQ(-1, fn(&set, max_signal + 1));
+  ASSERT_EQ(EINVAL, errno);
+
+  // Good signal numbers, low and high ends of range.
+  errno = 0;
+  ASSERT_EQ(0, fn(&set, min_signal));
+  ASSERT_EQ(0, errno);
+  ASSERT_EQ(0, fn(&set, max_signal));
+  ASSERT_EQ(0, errno);
+}
+
+TEST(signal, sigismember_invalid) {
+  TestSigSet2(sigismember);
+}
+
+TEST(signal, sigaddset_invalid) {
+  TestSigSet2(sigaddset);
+}
+
+TEST(signal, sigdelset_invalid) {
+  TestSigSet2(sigdelset);
+}
+
+TEST(signal, sigemptyset_invalid) {
+  TestSigSet1(sigemptyset);
+}
+
+TEST(signal, sigfillset_invalid) {
+  TestSigSet1(sigfillset);
+}