[committed 2.32 1/2] librt: fix NULL pointer dereference (bug 28213)

[committed 2.32 1/2] librt: fix NULL pointer dereference (bug 28213)

[Siddhesh Poyarekar]

From: Nikita Popov <npv1310@gmail.com>

Helper thread frees copied attribute on NOTIFY_REMOVED message
received from the OS kernel.  Unfortunately, it fails to check whether
copied attribute actually exists (data.attr != NULL).  This worked
earlier because free() checks passed pointer before actually
attempting to release corresponding memory.  But
__pthread_attr_destroy assumes pointer is not NULL.

So passing NULL pointer to __pthread_attr_destroy will result in
segmentation fault.  This scenario is possible if
notification->sigev_notify_attributes == NULL (which means default
thread attributes should be used).

Signed-off-by: Nikita Popov <npv1310@gmail.com>
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
(cherry picked from commit b805aebd42364fe696e417808a700fdb9800c9e8)
---
 sysdeps/unix/sysv/linux/mq_notify.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/unix/sysv/linux/mq_notify.c b/sysdeps/unix/sysv/linux/mq_notify.c
index b5a903c3a2..2bb98172c8 100644
--- a/sysdeps/unix/sysv/linux/mq_notify.c
+++ b/sysdeps/unix/sysv/linux/mq_notify.c
@@ -132,7 +132,7 @@ helper_thread (void *arg)
 	       to wait until it is done with it.  */
 	    (void) __pthread_barrier_wait (&notify_barrier);
 	}
-      else if (data.raw[NOTIFY_COOKIE_LEN - 1] == NOTIFY_REMOVED)
+      else if (data.raw[NOTIFY_COOKIE_LEN - 1] == NOTIFY_REMOVED && data.attr != NULL)
 	{
 	  /* The only state we keep is the copy of the thread attributes.  */
 	  pthread_attr_destroy (data.attr);
-- 
2.31.1

[committed 2.33 1/2] librt: fix NULL pointer dereference (bug 28213)

[Siddhesh Poyarekar]

From: Nikita Popov <npv1310@gmail.com>

Helper thread frees copied attribute on NOTIFY_REMOVED message
received from the OS kernel.  Unfortunately, it fails to check whether
copied attribute actually exists (data.attr != NULL).  This worked
earlier because free() checks passed pointer before actually
attempting to release corresponding memory.  But
__pthread_attr_destroy assumes pointer is not NULL.

So passing NULL pointer to __pthread_attr_destroy will result in
segmentation fault.  This scenario is possible if
notification->sigev_notify_attributes == NULL (which means default
thread attributes should be used).

Signed-off-by: Nikita Popov <npv1310@gmail.com>
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
(cherry picked from commit b805aebd42364fe696e417808a700fdb9800c9e8)
---
 sysdeps/unix/sysv/linux/mq_notify.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/unix/sysv/linux/mq_notify.c b/sysdeps/unix/sysv/linux/mq_notify.c
index 6f46d29d1d..1714e1cc5f 100644
--- a/sysdeps/unix/sysv/linux/mq_notify.c
+++ b/sysdeps/unix/sysv/linux/mq_notify.c
@@ -132,7 +132,7 @@ helper_thread (void *arg)
 	       to wait until it is done with it.  */
 	    (void) __pthread_barrier_wait (&notify_barrier);
 	}
-      else if (data.raw[NOTIFY_COOKIE_LEN - 1] == NOTIFY_REMOVED)
+      else if (data.raw[NOTIFY_COOKIE_LEN - 1] == NOTIFY_REMOVED && data.attr != NULL)
 	{
 	  /* The only state we keep is the copy of the thread attributes.  */
 	  pthread_attr_destroy (data.attr);
-- 
2.31.1

[committed 2.32 2/2] librt: add test (bug 28213)

[Siddhesh Poyarekar]

From: Nikita Popov <npv1310@gmail.com>

This test implements following logic:
1) Create POSIX message queue.
   Register a notification with mq_notify (using NULL attributes).
   Then immediately unregister the notification with mq_notify.
   Helper thread in a vulnerable version of glibc
   should cause NULL pointer dereference after these steps.
2) Once again, register the same notification.
   Try to send a dummy message.
   Test is considered successfulif the dummy message
   is successfully received by the callback function.

Signed-off-by: Nikita Popov <npv1310@gmail.com>
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
(cherry picked from commit 4cc79c217744743077bf7a0ec5e0a4318f1e6641)
---
 rt/Makefile      |   1 +
 rt/tst-bz28213.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)
 create mode 100644 rt/tst-bz28213.c

diff --git a/rt/Makefile b/rt/Makefile
index dab5d62a57..93502cfaa7 100644
--- a/rt/Makefile
+++ b/rt/Makefile
@@ -44,6 +44,7 @@ tests := tst-shm tst-timer tst-timer2 \
 	 tst-aio7 tst-aio8 tst-aio9 tst-aio10 \
 	 tst-mqueue1 tst-mqueue2 tst-mqueue3 tst-mqueue4 \
 	 tst-mqueue5 tst-mqueue6 tst-mqueue7 tst-mqueue8 tst-mqueue9 \
+	 tst-bz28213 \
 	 tst-timer3 tst-timer4 tst-timer5 \
 	 tst-cpuclock2 tst-cputimer1 tst-cputimer2 tst-cputimer3 \
 	 tst-shm-cancel
diff --git a/rt/tst-bz28213.c b/rt/tst-bz28213.c
new file mode 100644
index 0000000000..0c096b5a0a
--- /dev/null
+++ b/rt/tst-bz28213.c
@@ -0,0 +1,101 @@
+/* Bug 28213: test for NULL pointer dereference in mq_notify.
+   Copyright (C) The GNU Toolchain Authors.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <mqueue.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+
+static mqd_t m = -1;
+static const char msg[] = "hello";
+
+static void
+check_bz28213_cb (union sigval sv)
+{
+  char buf[sizeof (msg)];
+
+  (void) sv;
+
+  TEST_VERIFY_EXIT ((size_t) mq_receive (m, buf, sizeof (buf), NULL)
+		    == sizeof (buf));
+  TEST_VERIFY_EXIT (memcmp (buf, msg, sizeof (buf)) == 0);
+
+  exit (0);
+}
+
+static void
+check_bz28213 (void)
+{
+  struct sigevent sev;
+
+  memset (&sev, '\0', sizeof (sev));
+  sev.sigev_notify = SIGEV_THREAD;
+  sev.sigev_notify_function = check_bz28213_cb;
+
+  /* Step 1: Register & unregister notifier.
+     Helper thread should receive NOTIFY_REMOVED notification.
+     In a vulnerable version of glibc, NULL pointer dereference follows. */
+  TEST_VERIFY_EXIT (mq_notify (m, &sev) == 0);
+  TEST_VERIFY_EXIT (mq_notify (m, NULL) == 0);
+
+  /* Step 2: Once again, register notification.
+     Try to send one message.
+     Test is considered successful, if the callback does exit (0). */
+  TEST_VERIFY_EXIT (mq_notify (m, &sev) == 0);
+  TEST_VERIFY_EXIT (mq_send (m, msg, sizeof (msg), 1) == 0);
+
+  /* Wait... */
+  pause ();
+}
+
+static int
+do_test (void)
+{
+  static const char m_name[] = "/bz28213_queue";
+  struct mq_attr m_attr;
+
+  memset (&m_attr, '\0', sizeof (m_attr));
+  m_attr.mq_maxmsg = 1;
+  m_attr.mq_msgsize = sizeof (msg);
+
+  m = mq_open (m_name,
+               O_RDWR | O_CREAT | O_EXCL,
+               0600,
+               &m_attr);
+
+  if (m < 0)
+    {
+      if (errno == ENOSYS)
+        FAIL_UNSUPPORTED ("POSIX message queues are not implemented\n");
+      FAIL_EXIT1 ("Failed to create POSIX message queue: %m\n");
+    }
+
+  TEST_VERIFY_EXIT (mq_unlink (m_name) == 0);
+
+  check_bz28213 ();
+
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.31.1

[committed 2.33 2/2] librt: add test (bug 28213)

[Siddhesh Poyarekar]

From: Nikita Popov <npv1310@gmail.com>

This test implements following logic:
1) Create POSIX message queue.
   Register a notification with mq_notify (using NULL attributes).
   Then immediately unregister the notification with mq_notify.
   Helper thread in a vulnerable version of glibc
   should cause NULL pointer dereference after these steps.
2) Once again, register the same notification.
   Try to send a dummy message.
   Test is considered successfulif the dummy message
   is successfully received by the callback function.

Signed-off-by: Nikita Popov <npv1310@gmail.com>
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
(cherry picked from commit 4cc79c217744743077bf7a0ec5e0a4318f1e6641)
---
 rt/Makefile      |   1 +
 rt/tst-bz28213.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)
 create mode 100644 rt/tst-bz28213.c

diff --git a/rt/Makefile b/rt/Makefile
index 7b374f2073..c87d95793a 100644
--- a/rt/Makefile
+++ b/rt/Makefile
@@ -44,6 +44,7 @@ tests := tst-shm tst-timer tst-timer2 \
 	 tst-aio7 tst-aio8 tst-aio9 tst-aio10 \
 	 tst-mqueue1 tst-mqueue2 tst-mqueue3 tst-mqueue4 \
 	 tst-mqueue5 tst-mqueue6 tst-mqueue7 tst-mqueue8 tst-mqueue9 \
+	 tst-bz28213 \
 	 tst-timer3 tst-timer4 tst-timer5 \
 	 tst-cpuclock2 tst-cputimer1 tst-cputimer2 tst-cputimer3 \
 	 tst-shm-cancel
diff --git a/rt/tst-bz28213.c b/rt/tst-bz28213.c
new file mode 100644
index 0000000000..0c096b5a0a
--- /dev/null
+++ b/rt/tst-bz28213.c
@@ -0,0 +1,101 @@
+/* Bug 28213: test for NULL pointer dereference in mq_notify.
+   Copyright (C) The GNU Toolchain Authors.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <mqueue.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+
+static mqd_t m = -1;
+static const char msg[] = "hello";
+
+static void
+check_bz28213_cb (union sigval sv)
+{
+  char buf[sizeof (msg)];
+
+  (void) sv;
+
+  TEST_VERIFY_EXIT ((size_t) mq_receive (m, buf, sizeof (buf), NULL)
+		    == sizeof (buf));
+  TEST_VERIFY_EXIT (memcmp (buf, msg, sizeof (buf)) == 0);
+
+  exit (0);
+}
+
+static void
+check_bz28213 (void)
+{
+  struct sigevent sev;
+
+  memset (&sev, '\0', sizeof (sev));
+  sev.sigev_notify = SIGEV_THREAD;
+  sev.sigev_notify_function = check_bz28213_cb;
+
+  /* Step 1: Register & unregister notifier.
+     Helper thread should receive NOTIFY_REMOVED notification.
+     In a vulnerable version of glibc, NULL pointer dereference follows. */
+  TEST_VERIFY_EXIT (mq_notify (m, &sev) == 0);
+  TEST_VERIFY_EXIT (mq_notify (m, NULL) == 0);
+
+  /* Step 2: Once again, register notification.
+     Try to send one message.
+     Test is considered successful, if the callback does exit (0). */
+  TEST_VERIFY_EXIT (mq_notify (m, &sev) == 0);
+  TEST_VERIFY_EXIT (mq_send (m, msg, sizeof (msg), 1) == 0);
+
+  /* Wait... */
+  pause ();
+}
+
+static int
+do_test (void)
+{
+  static const char m_name[] = "/bz28213_queue";
+  struct mq_attr m_attr;
+
+  memset (&m_attr, '\0', sizeof (m_attr));
+  m_attr.mq_maxmsg = 1;
+  m_attr.mq_msgsize = sizeof (msg);
+
+  m = mq_open (m_name,
+               O_RDWR | O_CREAT | O_EXCL,
+               0600,
+               &m_attr);
+
+  if (m < 0)
+    {
+      if (errno == ENOSYS)
+        FAIL_UNSUPPORTED ("POSIX message queues are not implemented\n");
+      FAIL_EXIT1 ("Failed to create POSIX message queue: %m\n");
+    }
+
+  TEST_VERIFY_EXIT (mq_unlink (m_name) == 0);
+
+  check_bz28213 ();
+
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.31.1