From: Andreas Schwab Date: Thu, 27 May 2021 10:49:47 +0000 (+0200) Subject: [PATCH] Use __pthread_attr_copy in mq_notify (bug 27896) X-Git-Tag: archive/raspbian/2.28-10+rpi1+deb10u2^2~4 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=cefd02455a064eaaff3ac4e90bc2e5b9ef04681f;p=glibc.git [PATCH] Use __pthread_attr_copy in mq_notify (bug 27896) Make a deep copy of the pthread attribute object to remove a potential use-after-free issue. From 217b6dc298156bdb0d6aea9ea93e7e394a5ff091 Mon Sep 17 00:00:00 2001 From: Florian Weimer Date: Tue, 1 Jun 2021 17:51:41 +0200 Subject: [PATCH] Fix use of __pthread_attr_copy in mq_notify (bug 27896) __pthread_attr_copy can fail and does not initialize the attribute structure in that case. If __pthread_attr_copy is never called and there is no allocated attribute, pthread_attr_destroy should not be called, otherwise there is a null pointer dereference in rt/tst-mqueue6. Fixes commit 42d359350510506b87101cf77202fefcbfc790cb ("Use __pthread_attr_copy in mq_notify (bug 27896)"). Reviewed-by: Siddhesh Poyarekar From b805aebd42364fe696e417808a700fdb9800c9e8 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 9 Aug 2021 20:17:34 +0530 Subject: [PATCH] librt: fix NULL pointer dereference (bug 28213) 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 Reviewed-by: Siddhesh Poyarekar From 4cc79c217744743077bf7a0ec5e0a4318f1e6641 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 12 Aug 2021 16:09:50 +0530 Subject: [PATCH] librt: add test (bug 28213) 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 Reviewed-by: Siddhesh Poyarekar Gbp-Pq: Topic all Gbp-Pq: Name git-CVE-2021-33574-mq_notify-use-after-free.diff --- diff --git a/rt/Makefile b/rt/Makefile index 6d6b896ee..caffd5392 100644 --- a/rt/Makefile +++ b/rt/Makefile @@ -50,6 +50,7 @@ tests := tst-shm tst-clock tst-clock_nanosleep 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-cpuclock1 tst-cpuclock2 \ tst-cputimer1 tst-cputimer2 tst-cputimer3 \ diff --git a/rt/tst-bz28213.c b/rt/tst-bz28213.c new file mode 100644 index 000000000..0c096b5a0 --- /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 + . */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +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 diff --git a/sysdeps/unix/sysv/linux/mq_notify.c b/sysdeps/unix/sysv/linux/mq_notify.c index 3563e82cd..69b8db4d4 100644 --- a/sysdeps/unix/sysv/linux/mq_notify.c +++ b/sysdeps/unix/sysv/linux/mq_notify.c @@ -134,9 +134,12 @@ helper_thread (void *arg) to wait until it is done with it. */ (void) __pthread_barrier_wait (¬ify_barrier); } - else if (data.raw[NOTIFY_COOKIE_LEN - 1] == NOTIFY_REMOVED) - /* The only state we keep is the copy of the thread attributes. */ - free (data.attr); + 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); + free (data.attr); + } } return NULL; } @@ -215,6 +218,39 @@ init_mq_netlink (void) } +static int __pthread_attr_copy(pthread_attr_t *target, const pthread_attr_t *source) +{ + /* Avoid overwriting *TARGET until all allocations have + succeeded. */ + union { + pthread_attr_t external; + struct pthread_attr internal; + } temp; + temp.external = *source; + + /* Force new allocation. This function has full ownership of temp. */ + temp.internal.cpuset = NULL; + temp.internal.cpusetsize = 0; + + struct pthread_attr *isource = (struct pthread_attr *) source; + + /* Propagate affinity mask information. */ + if (isource->cpusetsize > 0) { + /* inline pthread_attr_setaffinity_np, otherwise we break conformance tests */ + temp.internal.cpuset = malloc (isource->cpusetsize); + if (temp.internal.cpuset == NULL) + return ENOMEM; + temp.internal.cpusetsize = isource->cpusetsize; + memcpy(temp.internal.cpuset, isource->cpuset, isource->cpusetsize); + } + + /* Transfer ownership. *target is not assumed to have been + initialized. */ + *target = temp.external; + return 0; +} + + /* Register notification upon message arrival to an empty message queue MQDES. */ int @@ -257,8 +293,14 @@ mq_notify (mqd_t mqdes, const struct sigevent *notification) if (data.attr == NULL) return -1; - memcpy (data.attr, notification->sigev_notify_attributes, - sizeof (pthread_attr_t)); + int ret = __pthread_attr_copy (data.attr, + notification->sigev_notify_attributes); + if (ret != 0) + { + free (data.attr); + __set_errno (ret); + return -1; + } } /* Construct the new request. */ @@ -271,8 +313,11 @@ mq_notify (mqd_t mqdes, const struct sigevent *notification) int retval = INLINE_SYSCALL (mq_notify, 2, mqdes, &se); /* If it failed, free the allocated memory. */ - if (__glibc_unlikely (retval != 0)) - free (data.attr); + if (retval != 0 && data.attr != NULL) + { + pthread_attr_destroy (data.attr); + free (data.attr); + } return retval; }