* [PATCH] librt: add test (bug 28213) @ 2021-08-11 11:26 Никита Попов 2021-08-12 3:10 ` Никита Попов 2021-08-12 4:18 ` Siddhesh Poyarekar 0 siblings, 2 replies; 8+ messages in thread From: Никита Попов @ 2021-08-11 11:26 UTC (permalink / raw) To: libc-alpha [-- Attachment #1: Type: text/plain, Size: 478 bytes --] Hello. I'm submitting a test case for recent bug 28213. It shows the following results. Before fix of bug 28213: # cat ./rt/tst-bz28213.out Child caused segmentation fault # cat ./rt/tst-bz28213.test-result FAIL: rt/tst-bz28213 original exit status 1 After fix of bug 28213: # cat ./rt/tst-bz28213.out # cat ./rt/tst-bz28213.test-result PASS: rt/tst-bz28213 original exit status 0 I managed to get libc cause segmentation fault. Looking forward to receiving feedback from you. [-- Attachment #2: 0001-librt-add-test-bug-28213.patch --] [-- Type: text/x-patch, Size: 5754 bytes --] From eb51dcf05befe6e839102e0cb622b1afdbf72a7a Mon Sep 17 00:00:00 2001 From: Nikita Popov <npv1310@gmail.com> Date: Wed, 11 Aug 2021 14:36:50 +0500 Subject: [PATCH] librt: add test (bug 28213) To: libc-alpha@sourceware.org This test implements following logic: 1) Create dummy message queue, register it with mq_notify (using NULL attributes), immediately close the queue. Helper thread should cause NULL pointer dereference by this moment. 2) Create another queue and try to send a message via the helper thread. Test is considered successful if the callback function receives the same message as was sent. Signed-off-by: Nikita Popov <npv1310@gmail.com> --- rt/Makefile | 1 + rt/tst-bz28213.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+) create mode 100644 rt/tst-bz28213.c diff --git a/rt/Makefile b/rt/Makefile index 113cea03a5..910e775995 100644 --- a/rt/Makefile +++ b/rt/Makefile @@ -74,6 +74,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..6c043a9017 --- /dev/null +++ b/rt/tst-bz28213.c @@ -0,0 +1,174 @@ +/* Bug 28213: test for NULL pointer dereference in mq_notify. + Copyright (C) 2018-2021 Free Software Foundation, Inc. + 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 <sys/wait.h> +#include <fcntl.h> +#include <unistd.h> +#include <mqueue.h> +#include <signal.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <support/xunistd.h> + +static const char check_bz28213_name[] = "/bz28213_queue"; +static const char check_bz28213_msg[] = "dummy"; + +static void +check_bz28213_cb (union sigval sv) +{ + char buf[sizeof (check_bz28213_msg)]; + mqd_t m = sv.sival_int; + ssize_t n; + + if ((n = mq_receive (m, buf, sizeof (buf), NULL)) < 0L || + (size_t) n != sizeof (buf)) + exit (1); + + if (memcmp (buf, check_bz28213_msg, sizeof (buf)) != 0) + exit (1); + + exit (0); +} + +static void +check_bz28213 (void) +{ + mqd_t m; + struct sigevent sev; + struct mq_attr attr; + unsigned int i; + + /* First iteration should lead to undefined behavior due to NULL pointer dereference. + Second iteration tests whether helper thread still works. */ + for (i = 0U; i < 2U; i++) + { + memset (&attr, '\0', sizeof (attr)); + attr.mq_maxmsg = 1; + attr.mq_msgsize = sizeof (check_bz28213_msg); + + if ((m = mq_open (check_bz28213_name, + O_RDWR | O_CREAT | O_EXCL, + 0600, + &attr)) < 0) + exit (1); + + if (mq_unlink (check_bz28213_name) < 0) + exit (1); + + memset (&sev, '\0', sizeof (sev)); + sev.sigev_notify = SIGEV_THREAD; + sev.sigev_value.sival_int = m; + sev.sigev_notify_function = check_bz28213_cb; + + if (mq_notify (m, &sev) < 0) + exit (1); + + if (i && mq_send (m, check_bz28213_msg, sizeof (check_bz28213_msg), 1) < 0) + exit (1); + + if (!i) + mq_close (m); + } + + alarm (10); + + while (1) + pause (); +} + +/* Skip entire testing if queues are not implemented */ +static int +check_mq_api (void) +{ + struct mq_attr attr; + mqd_t m; + int rc = 0; + + memset (&attr, '\0', sizeof (attr)); + attr.mq_maxmsg = 1; + attr.mq_msgsize = 1; + + if ((m = mq_open (check_bz28213_name, + O_RDWR | O_CREAT | O_EXCL, + 0600, + &attr)) < 0) + { + if (errno == ENOSYS) + { + rc = 1; + printf ("SKIP: not implemented\n"); + } + + return rc; + } + + mq_unlink (check_bz28213_name); + mq_close (m); + + return rc; +} + +static int +do_test (void) +{ + pid_t pid; + int status, rc; + + if (check_mq_api () == 1) + return 0; + + if ((pid = xfork ()) == 0) + check_bz28213 (); + + if (TEMP_FAILURE_RETRY (waitpid (pid, &status, 0)) != pid) + { + kill (pid, SIGKILL); + printf ("waitpid failed\n"); + return 1; + } + + rc = 1; + if (WIFEXITED (status)) + { + int child_rc; + + if ((child_rc = WEXITSTATUS (status)) == 0) + rc = 0; + else + printf ("Child returned non-zero exit code [%d]\n", child_rc); + } + else if (WIFSIGNALED (status)) + { + int child_signo; + + if ((child_signo = WTERMSIG (status)) == SIGABRT) + printf ("Child timed out\n"); + else if (child_signo == SIGSEGV) + printf ("Child caused segmentation fault\n"); + else + printf ("Child terminated with signal %d\n", child_signo); + } + + return rc; +} + +#include <support/test-driver.c> -- 2.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] librt: add test (bug 28213) 2021-08-11 11:26 [PATCH] librt: add test (bug 28213) Никита Попов @ 2021-08-12 3:10 ` Никита Попов 2021-08-12 4:18 ` Siddhesh Poyarekar 1 sibling, 0 replies; 8+ messages in thread From: Никита Попов @ 2021-08-12 3:10 UTC (permalink / raw) To: libc-alpha Hello, any update? On Wed, Aug 11, 2021, 16:26 Никита Попов <npv1310@gmail.com> wrote: > Hello. I'm submitting a test case for recent bug 28213. It shows the > following results. > Before fix of bug 28213: > # cat ./rt/tst-bz28213.out > Child caused segmentation fault > # cat ./rt/tst-bz28213.test-result > FAIL: rt/tst-bz28213 > original exit status 1 > > After fix of bug 28213: > # cat ./rt/tst-bz28213.out > # cat ./rt/tst-bz28213.test-result > PASS: rt/tst-bz28213 > original exit status 0 > I managed to get libc cause segmentation fault. > Looking forward to receiving feedback from you. > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] librt: add test (bug 28213) 2021-08-11 11:26 [PATCH] librt: add test (bug 28213) Никита Попов 2021-08-12 3:10 ` Никита Попов @ 2021-08-12 4:18 ` Siddhesh Poyarekar 2021-08-12 9:00 ` Никита Попов 1 sibling, 1 reply; 8+ messages in thread From: Siddhesh Poyarekar @ 2021-08-12 4:18 UTC (permalink / raw) To: Никита Попов, libc-alpha On 8/11/21 4:56 PM, Никита Попов via Libc-alpha wrote: > Hello. I'm submitting a test case for recent bug 28213. It shows the > following results. > Before fix of bug 28213: > # cat ./rt/tst-bz28213.out > Child caused segmentation fault > # cat ./rt/tst-bz28213.test-result > FAIL: rt/tst-bz28213 > original exit status 1 > > After fix of bug 28213: > # cat ./rt/tst-bz28213.out > # cat ./rt/tst-bz28213.test-result > PASS: rt/tst-bz28213 > original exit status 0 > I managed to get libc cause segmentation fault. > Looking forward to receiving feedback from you. Thank you for sharing this. My detailed review comments are inline. Overall I think this can be done without forking a new process since the test infrastructure already does that and is capable of reporting unexpected signals. All you need to test is that the program runs to completion and ensure that the helper thread is called, perhaps by simply sleeping for a while. > From eb51dcf05befe6e839102e0cb622b1afdbf72a7a Mon Sep 17 00:00:00 2001 > From: Nikita Popov <npv1310@gmail.com> > Date: Wed, 11 Aug 2021 14:36:50 +0500 > Subject: [PATCH] librt: add test (bug 28213) > To: libc-alpha@sourceware.org > > This test implements following logic: > 1) Create dummy message queue, register it with mq_notify (using NULL attributes), > immediately close the queue. > Helper thread should cause NULL pointer dereference by this moment. > 2) Create another queue and try to send a message via the helper thread. > Test is considered successful if the callback function receives the same message as was sent. > > Signed-off-by: Nikita Popov <npv1310@gmail.com> > --- > rt/Makefile | 1 + > rt/tst-bz28213.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 175 insertions(+) > create mode 100644 rt/tst-bz28213.c > > diff --git a/rt/Makefile b/rt/Makefile > index 113cea03a5..910e775995 100644 > --- a/rt/Makefile > +++ b/rt/Makefile > @@ -74,6 +74,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..6c043a9017 > --- /dev/null > +++ b/rt/tst-bz28213.c > @@ -0,0 +1,174 @@ > +/* Bug 28213: test for NULL pointer dereference in mq_notify. > + Copyright (C) 2018-2021 Free Software Foundation, Inc. Please make this copyright "The GNU Toolchain Authors" since you don't have a copyright assignment with the FSF. > + 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 <sys/wait.h> > +#include <fcntl.h> > +#include <unistd.h> > +#include <mqueue.h> > +#include <signal.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <support/xunistd.h> > + > +static const char check_bz28213_name[] = "/bz28213_queue"; > +static const char check_bz28213_msg[] = "dummy"; > + > +static void > +check_bz28213_cb (union sigval sv) > +{ > + char buf[sizeof (check_bz28213_msg)]; > + mqd_t m = sv.sival_int; > + ssize_t n; > + > + if ((n = mq_receive (m, buf, sizeof (buf), NULL)) < 0L || > + (size_t) n != sizeof (buf)) > + exit (1); > + > + if (memcmp (buf, check_bz28213_msg, sizeof (buf)) != 0) > + exit (1); > + > + exit (0); > +} Instead of exiting with error, please use the TEST_VERIFY_EXIT macro in support/check.h. > + > +static void > +check_bz28213 (void) > +{ > + mqd_t m; > + struct sigevent sev; > + struct mq_attr attr; > + unsigned int i; > + > + /* First iteration should lead to undefined behavior due to NULL pointer dereference. > + Second iteration tests whether helper thread still works. */ > + for (i = 0U; i < 2U; i++) > + { > + memset (&attr, '\0', sizeof (attr)); > + attr.mq_maxmsg = 1; > + attr.mq_msgsize = sizeof (check_bz28213_msg); > + > + if ((m = mq_open (check_bz28213_name, > + O_RDWR | O_CREAT | O_EXCL, > + 0600, > + &attr)) < 0) > + exit (1); > + > + if (mq_unlink (check_bz28213_name) < 0) > + exit (1); > + > + memset (&sev, '\0', sizeof (sev)); > + sev.sigev_notify = SIGEV_THREAD; > + sev.sigev_value.sival_int = m; > + sev.sigev_notify_function = check_bz28213_cb; > + > + if (mq_notify (m, &sev) < 0) > + exit (1); > + > + if (i && mq_send (m, check_bz28213_msg, sizeof (check_bz28213_msg), 1) < 0) > + exit (1); Likewise for these exits, please use TEST_VERIFY_EXIT instead. > + > + if (!i) > + mq_close (m); > + } > + > + alarm (10); > + > + while (1) > + pause (); Instead you could define TIMEOUT to DEFAULT_TIMEOUT and then use a sleep(DEFAULT_TIMEOUT / 2) to wait long enough (10 seconds ought to be enough for anybody?) for the helper thread to run. Architecture maintainers can later bump up TIMEOUT if they find that the test times out on their architecture. > +} > + > +/* Skip entire testing if queues are not implemented */ > +static int > +check_mq_api (void) > +{ > + struct mq_attr attr; > + mqd_t m; > + int rc = 0; > + > + memset (&attr, '\0', sizeof (attr)); > + attr.mq_maxmsg = 1; > + attr.mq_msgsize = 1; > + > + if ((m = mq_open (check_bz28213_name, > + O_RDWR | O_CREAT | O_EXCL, > + 0600, > + &attr)) < 0) > + { > + if (errno == ENOSYS) > + { > + rc = 1; > + printf ("SKIP: not implemented\n"); > + } > + > + return rc; > + } > + > + mq_unlink (check_bz28213_name); > + mq_close (m); > + > + return rc; > +} > + > +static int > +do_test (void) > +{ > + pid_t pid; > + int status, rc; > + > + if (check_mq_api () == 1) > + return 0; Please use FAIL_UNSUPPORTED() here. > + > + if ((pid = xfork ()) == 0) > + check_bz28213 (); > + > + if (TEMP_FAILURE_RETRY (waitpid (pid, &status, 0)) != pid) > + { > + kill (pid, SIGKILL); > + printf ("waitpid failed\n"); > + return 1; > + } > + > + rc = 1; > + if (WIFEXITED (status)) > + { > + int child_rc; > + > + if ((child_rc = WEXITSTATUS (status)) == 0) > + rc = 0; > + else > + printf ("Child returned non-zero exit code [%d]\n", child_rc); > + } > + else if (WIFSIGNALED (status)) > + { > + int child_signo; > + > + if ((child_signo = WTERMSIG (status)) == SIGABRT) > + printf ("Child timed out\n"); > + else if (child_signo == SIGSEGV) > + printf ("Child caused segmentation fault\n"); > + else > + printf ("Child terminated with signal %d\n", child_signo); > + } Forking should not be necessary since the test driver already reports unexpected signals. > + > + return rc; Return 0 in the end. > +} > + > +#include <support/test-driver.c> > -- > 2.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] librt: add test (bug 28213) 2021-08-12 4:18 ` Siddhesh Poyarekar @ 2021-08-12 9:00 ` Никита Попов 2021-08-12 9:34 ` Siddhesh Poyarekar 0 siblings, 1 reply; 8+ messages in thread From: Никита Попов @ 2021-08-12 9:00 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: libc-alpha [-- Attachment #1: Type: text/plain, Size: 9164 bytes --] Thank you for your feedback! I'm submitting a fixed version. It has slightly changed logic though. The results are: (before) # cat rt/tst-bz28213.test-result FAIL: rt/tst-bz28213 original exit status 1 # cat rt/tst-bz28213.out Didn't expect signal from child: got `Segmentation fault' (after) # cat rt/tst-bz28213.test-result PASS: rt/tst-bz28213 original exit status 0 # cat rt/tst-bz28213.out чт, 12 авг. 2021 г. в 09:19, Siddhesh Poyarekar <siddhesh@gotplt.org>: > > On 8/11/21 4:56 PM, Никита Попов via Libc-alpha wrote: > > Hello. I'm submitting a test case for recent bug 28213. It shows the > > following results. > > Before fix of bug 28213: > > # cat ./rt/tst-bz28213.out > > Child caused segmentation fault > > # cat ./rt/tst-bz28213.test-result > > FAIL: rt/tst-bz28213 > > original exit status 1 > > > > After fix of bug 28213: > > # cat ./rt/tst-bz28213.out > > # cat ./rt/tst-bz28213.test-result > > PASS: rt/tst-bz28213 > > original exit status 0 > > I managed to get libc cause segmentation fault. > > Looking forward to receiving feedback from you. > > Thank you for sharing this. My detailed review comments are inline. > Overall I think this can be done without forking a new process since the > test infrastructure already does that and is capable of reporting > unexpected signals. All you need to test is that the program runs to > completion and ensure that the helper thread is called, perhaps by > simply sleeping for a while. > > > From eb51dcf05befe6e839102e0cb622b1afdbf72a7a Mon Sep 17 00:00:00 2001 > > From: Nikita Popov <npv1310@gmail.com> > > Date: Wed, 11 Aug 2021 14:36:50 +0500 > > Subject: [PATCH] librt: add test (bug 28213) > > To: libc-alpha@sourceware.org > > > > This test implements following logic: > > 1) Create dummy message queue, register it with mq_notify (using NULL attributes), > > immediately close the queue. > > Helper thread should cause NULL pointer dereference by this moment. > > 2) Create another queue and try to send a message via the helper thread. > > Test is considered successful if the callback function receives the same message as was sent. > > > > Signed-off-by: Nikita Popov <npv1310@gmail.com> > > --- > > rt/Makefile | 1 + > > rt/tst-bz28213.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 175 insertions(+) > > create mode 100644 rt/tst-bz28213.c > > > > diff --git a/rt/Makefile b/rt/Makefile > > index 113cea03a5..910e775995 100644 > > --- a/rt/Makefile > > +++ b/rt/Makefile > > @@ -74,6 +74,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..6c043a9017 > > --- /dev/null > > +++ b/rt/tst-bz28213.c > > @@ -0,0 +1,174 @@ > > +/* Bug 28213: test for NULL pointer dereference in mq_notify. > > + Copyright (C) 2018-2021 Free Software Foundation, Inc. > > Please make this copyright "The GNU Toolchain Authors" since you don't > have a copyright assignment with the FSF. > > > + 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 <sys/wait.h> > > +#include <fcntl.h> > > +#include <unistd.h> > > +#include <mqueue.h> > > +#include <signal.h> > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <string.h> > > +#include <support/xunistd.h> > > + > > +static const char check_bz28213_name[] = "/bz28213_queue"; > > +static const char check_bz28213_msg[] = "dummy"; > > + > > +static void > > +check_bz28213_cb (union sigval sv) > > +{ > > + char buf[sizeof (check_bz28213_msg)]; > > + mqd_t m = sv.sival_int; > > + ssize_t n; > > + > > + if ((n = mq_receive (m, buf, sizeof (buf), NULL)) < 0L || > > + (size_t) n != sizeof (buf)) > > + exit (1); > > + > > + if (memcmp (buf, check_bz28213_msg, sizeof (buf)) != 0) > > + exit (1); > > + > > + exit (0); > > +} > > Instead of exiting with error, please use the TEST_VERIFY_EXIT macro in > support/check.h. > > > + > > +static void > > +check_bz28213 (void) > > +{ > > + mqd_t m; > > + struct sigevent sev; > > + struct mq_attr attr; > > + unsigned int i; > > + > > + /* First iteration should lead to undefined behavior due to NULL pointer dereference. > > + Second iteration tests whether helper thread still works. */ > > + for (i = 0U; i < 2U; i++) > > + { > > + memset (&attr, '\0', sizeof (attr)); > > + attr.mq_maxmsg = 1; > > + attr.mq_msgsize = sizeof (check_bz28213_msg); > > + > > + if ((m = mq_open (check_bz28213_name, > > + O_RDWR | O_CREAT | O_EXCL, > > + 0600, > > + &attr)) < 0) > > + exit (1); > > + > > + if (mq_unlink (check_bz28213_name) < 0) > > + exit (1); > > + > > + memset (&sev, '\0', sizeof (sev)); > > + sev.sigev_notify = SIGEV_THREAD; > > + sev.sigev_value.sival_int = m; > > + sev.sigev_notify_function = check_bz28213_cb; > > + > > + if (mq_notify (m, &sev) < 0) > > + exit (1); > > + > > + if (i && mq_send (m, check_bz28213_msg, sizeof (check_bz28213_msg), 1) < 0) > > + exit (1); > > Likewise for these exits, please use TEST_VERIFY_EXIT instead. > > > + > > + if (!i) > > + mq_close (m); > > + } > > + > > + alarm (10); > > + > > + while (1) > > + pause (); > > Instead you could define TIMEOUT to DEFAULT_TIMEOUT and then use a > sleep(DEFAULT_TIMEOUT / 2) to wait long enough (10 seconds ought to be > enough for anybody?) for the helper thread to run. Architecture > maintainers can later bump up TIMEOUT if they find that the test times > out on their architecture. > > > +} > > + > > +/* Skip entire testing if queues are not implemented */ > > +static int > > +check_mq_api (void) > > +{ > > + struct mq_attr attr; > > + mqd_t m; > > + int rc = 0; > > + > > + memset (&attr, '\0', sizeof (attr)); > > + attr.mq_maxmsg = 1; > > + attr.mq_msgsize = 1; > > + > > + if ((m = mq_open (check_bz28213_name, > > + O_RDWR | O_CREAT | O_EXCL, > > + 0600, > > + &attr)) < 0) > > + { > > + if (errno == ENOSYS) > > + { > > + rc = 1; > > + printf ("SKIP: not implemented\n"); > > + } > > + > > + return rc; > > + } > > + > > + mq_unlink (check_bz28213_name); > > + mq_close (m); > > + > > + return rc; > > +} > > + > > +static int > > +do_test (void) > > +{ > > + pid_t pid; > > + int status, rc; > > + > > + if (check_mq_api () == 1) > > + return 0; > > Please use FAIL_UNSUPPORTED() here. > > > + > > + if ((pid = xfork ()) == 0) > > + check_bz28213 (); > > + > > + if (TEMP_FAILURE_RETRY (waitpid (pid, &status, 0)) != pid) > > + { > > + kill (pid, SIGKILL); > > + printf ("waitpid failed\n"); > > + return 1; > > + } > > + > > + rc = 1; > > + if (WIFEXITED (status)) > > + { > > + int child_rc; > > + > > + if ((child_rc = WEXITSTATUS (status)) == 0) > > + rc = 0; > > + else > > + printf ("Child returned non-zero exit code [%d]\n", child_rc); > > + } > > + else if (WIFSIGNALED (status)) > > + { > > + int child_signo; > > + > > + if ((child_signo = WTERMSIG (status)) == SIGABRT) > > + printf ("Child timed out\n"); > > + else if (child_signo == SIGSEGV) > > + printf ("Child caused segmentation fault\n"); > > + else > > + printf ("Child terminated with signal %d\n", child_signo); > > + } > > Forking should not be necessary since the test driver already reports > unexpected signals. > > > + > > + return rc; > > Return 0 in the end. > > > +} > > + > > +#include <support/test-driver.c> > > -- > > 2.17.1 > [-- Attachment #2: 0001-librt-add-test-bug-28213.patch --] [-- Type: text/x-patch, Size: 4508 bytes --] From 35600a79df9f3dedaca35ad8ec050be80e13fb12 Mon Sep 17 00:00:00 2001 From: Nikita Popov <npv1310@gmail.com> Date: Thu, 12 Aug 2021 12:07:00 +0500 Subject: [PATCH] librt: add test (bug 28213) To: libc-alpha@sourceware.org This test implements following logic: 1) Create POSIX message queue. Register a notification with mq_notify (using NULL thread 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 successful if the dummy message is successfully received by the callback function. Signed-off-by: Nikita Popov <npv1310@gmail.com> --- rt/Makefile | 1 + rt/tst-bz28213.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 rt/tst-bz28213.c diff --git a/rt/Makefile b/rt/Makefile index 113cea03a5..910e775995 100644 --- a/rt/Makefile +++ b/rt/Makefile @@ -74,6 +74,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..b020e3d7dc --- /dev/null +++ b/rt/tst-bz28213.c @@ -0,0 +1,102 @@ +/* Bug 28213: test for NULL pointer dereference in mq_notify. + Copyright (C) 2021-2021 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/test-driver.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... */ + sleep (DEFAULT_TIMEOUT); + FAIL_EXIT1 ("Operation timed out\n"); +} + +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.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] librt: add test (bug 28213) 2021-08-12 9:00 ` Никита Попов @ 2021-08-12 9:34 ` Siddhesh Poyarekar 2021-08-12 10:20 ` Никита Попов 0 siblings, 1 reply; 8+ messages in thread From: Siddhesh Poyarekar @ 2021-08-12 9:34 UTC (permalink / raw) To: Никита Попов Cc: libc-alpha On 8/12/21 2:30 PM, Никита Попов wrote: > Thank you for your feedback! > I'm submitting a fixed version. It has slightly changed logic though. > The results are: > (before) > # cat rt/tst-bz28213.test-result > FAIL: rt/tst-bz28213 > original exit status 1 > # cat rt/tst-bz28213.out > Didn't expect signal from child: got `Segmentation fault' > (after) > # cat rt/tst-bz28213.test-result > PASS: rt/tst-bz28213 > original exit status 0 > # cat rt/tst-bz28213.out Thanks for the update, this logic is certainly more robust. We're very close now, just some more nits: > From 35600a79df9f3dedaca35ad8ec050be80e13fb12 Mon Sep 17 00:00:00 2001 > From: Nikita Popov <npv1310@gmail.com> > Date: Thu, 12 Aug 2021 12:07:00 +0500 > Subject: [PATCH] librt: add test (bug 28213) > To: libc-alpha@sourceware.org > > This test implements following logic: > 1) Create POSIX message queue. > Register a notification with mq_notify (using NULL thread 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 successful if the dummy message is successfully received by the callback function. Please keep line lengths in git commit logs at 74 chars or less. > > Signed-off-by: Nikita Popov <npv1310@gmail.com> > --- > rt/Makefile | 1 + > rt/tst-bz28213.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 103 insertions(+) > create mode 100644 rt/tst-bz28213.c > > diff --git a/rt/Makefile b/rt/Makefile > index 113cea03a5..910e775995 100644 > --- a/rt/Makefile > +++ b/rt/Makefile > @@ -74,6 +74,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..b020e3d7dc > --- /dev/null > +++ b/rt/tst-bz28213.c > @@ -0,0 +1,102 @@ > +/* Bug 28213: test for NULL pointer dereference in mq_notify. > + Copyright (C) 2021-2021 The GNU Toolchain Authors. No copyright years needed. Oh and congratulations, you're the first contributor to add this :) > + 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/test-driver.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)); Please wrap at 79 chars. Please review the style and conventions guide here: https://sourceware.org/glibc/wiki/Style_and_Conventions > + 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); OK. > + > + /* 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); OK. > + > + /* Wait... */ > + sleep (DEFAULT_TIMEOUT); > + FAIL_EXIT1 ("Operation timed out\n"); With this new logic, you only need to pause() here. The test driver has timeout functionality that will automatically fail the test if it stays stuck here for DEFAULT_TIMEOUT seconds. > +} > + > +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"); OK. > + FAIL_EXIT1 ("Failed to create POSIX message queue: %m\n"); > + } > + > + TEST_VERIFY_EXIT (mq_unlink (m_name) == 0); > + > + check_bz28213 (); > + > + return 0; > +} OK. > + > +#include <support/test-driver.c> > -- > 2.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] librt: add test (bug 28213) 2021-08-12 9:34 ` Siddhesh Poyarekar @ 2021-08-12 10:20 ` Никита Попов 2021-08-13 3:28 ` Никита Попов 0 siblings, 1 reply; 8+ messages in thread From: Никита Попов @ 2021-08-12 10:20 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: libc-alpha [-- Attachment #1: Type: text/plain, Size: 6546 bytes --] чт, 12 авг. 2021 г. в 14:34, Siddhesh Poyarekar <siddhesh@gotplt.org>: > > On 8/12/21 2:30 PM, Никита Попов wrote: > > Thank you for your feedback! > > I'm submitting a fixed version. It has slightly changed logic though. > > The results are: > > (before) > > # cat rt/tst-bz28213.test-result > > FAIL: rt/tst-bz28213 > > original exit status 1 > > # cat rt/tst-bz28213.out > > Didn't expect signal from child: got `Segmentation fault' > > (after) > > # cat rt/tst-bz28213.test-result > > PASS: rt/tst-bz28213 > > original exit status 0 > > # cat rt/tst-bz28213.out > > Thanks for the update, this logic is certainly more robust. We're very > close now, just some more nits: > > > From 35600a79df9f3dedaca35ad8ec050be80e13fb12 Mon Sep 17 00:00:00 2001 > > From: Nikita Popov <npv1310@gmail.com> > > Date: Thu, 12 Aug 2021 12:07:00 +0500 > > Subject: [PATCH] librt: add test (bug 28213) > > To: libc-alpha@sourceware.org > > > > This test implements following logic: > > 1) Create POSIX message queue. > > Register a notification with mq_notify (using NULL thread 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 successful if the dummy message is successfully received by the callback function. > > Please keep line lengths in git commit logs at 74 chars or less. > > > > > Signed-off-by: Nikita Popov <npv1310@gmail.com> > > --- > > rt/Makefile | 1 + > > rt/tst-bz28213.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 103 insertions(+) > > create mode 100644 rt/tst-bz28213.c > > > > diff --git a/rt/Makefile b/rt/Makefile > > index 113cea03a5..910e775995 100644 > > --- a/rt/Makefile > > +++ b/rt/Makefile > > @@ -74,6 +74,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..b020e3d7dc > > --- /dev/null > > +++ b/rt/tst-bz28213.c > > @@ -0,0 +1,102 @@ > > +/* Bug 28213: test for NULL pointer dereference in mq_notify. > > + Copyright (C) 2021-2021 The GNU Toolchain Authors. > > No copyright years needed. Oh and congratulations, you're the first > contributor to add this :) > > > + 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/test-driver.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)); > > Please wrap at 79 chars. Please review the style and conventions guide > here: > > https://sourceware.org/glibc/wiki/Style_and_Conventions > > > + 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); > > OK. > > > + > > + /* 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); > > OK. > > > + > > + /* Wait... */ > > + sleep (DEFAULT_TIMEOUT); > > + FAIL_EXIT1 ("Operation timed out\n"); > > With this new logic, you only need to pause() here. The test driver has > timeout functionality that will automatically fail the test if it stays > stuck here for DEFAULT_TIMEOUT seconds. > > > +} > > + > > +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"); > > OK. > > > + FAIL_EXIT1 ("Failed to create POSIX message queue: %m\n"); > > + } > > + > > + TEST_VERIFY_EXIT (mq_unlink (m_name) == 0); > > + > > + check_bz28213 (); > > + > > + return 0; > > +} > > OK. > > > + > > +#include <support/test-driver.c> > > -- > > 2.17.1 [-- Attachment #2: 0001-librt-add-test-bug-28213.patch --] [-- Type: text/x-patch, Size: 4443 bytes --] From f623213cee4e1a140dc90eaf60832119f7e28589 Mon Sep 17 00:00:00 2001 From: Nikita Popov <npv1310@gmail.com> Date: Thu, 12 Aug 2021 12:07:00 +0500 Subject: [PATCH] librt: add test (bug 28213) To: libc-alpha@sourceware.org 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> --- rt/Makefile | 1 + rt/tst-bz28213.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 rt/tst-bz28213.c diff --git a/rt/Makefile b/rt/Makefile index 113cea03a5..910e775995 100644 --- a/rt/Makefile +++ b/rt/Makefile @@ -74,6 +74,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..4aacd293be --- /dev/null +++ b/rt/tst-bz28213.c @@ -0,0 +1,102 @@ +/* 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... */ + while (1) + 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.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] librt: add test (bug 28213) 2021-08-12 10:20 ` Никита Попов @ 2021-08-13 3:28 ` Никита Попов 2021-08-13 3:47 ` Siddhesh Poyarekar 0 siblings, 1 reply; 8+ messages in thread From: Никита Попов @ 2021-08-13 3:28 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: libc-alpha Hello. I have some concerns about the recent commit 4cc79c217744743077bf7a0ec5e0a4318f1e6641. In rt/tst-bz28213.c we use one call to pause (). Should it return for some reason 'return 0' from do_test () follows. Maybe it is better to wrap pause () call here into an infinite loop like this? [while (1) pause ();] Then we can be sure that the only success in this test can be reached via exit (0) from the callback function. чт, 12 авг. 2021 г. в 15:20, Никита Попов <npv1310@gmail.com>: > > чт, 12 авг. 2021 г. в 14:34, Siddhesh Poyarekar <siddhesh@gotplt.org>: > > > > On 8/12/21 2:30 PM, Никита Попов wrote: > > > Thank you for your feedback! > > > I'm submitting a fixed version. It has slightly changed logic though. > > > The results are: > > > (before) > > > # cat rt/tst-bz28213.test-result > > > FAIL: rt/tst-bz28213 > > > original exit status 1 > > > # cat rt/tst-bz28213.out > > > Didn't expect signal from child: got `Segmentation fault' > > > (after) > > > # cat rt/tst-bz28213.test-result > > > PASS: rt/tst-bz28213 > > > original exit status 0 > > > # cat rt/tst-bz28213.out > > > > Thanks for the update, this logic is certainly more robust. We're very > > close now, just some more nits: > > > > > From 35600a79df9f3dedaca35ad8ec050be80e13fb12 Mon Sep 17 00:00:00 2001 > > > From: Nikita Popov <npv1310@gmail.com> > > > Date: Thu, 12 Aug 2021 12:07:00 +0500 > > > Subject: [PATCH] librt: add test (bug 28213) > > > To: libc-alpha@sourceware.org > > > > > > This test implements following logic: > > > 1) Create POSIX message queue. > > > Register a notification with mq_notify (using NULL thread 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 successful if the dummy message is successfully received by the callback function. > > > > Please keep line lengths in git commit logs at 74 chars or less. > > > > > > > > Signed-off-by: Nikita Popov <npv1310@gmail.com> > > > --- > > > rt/Makefile | 1 + > > > rt/tst-bz28213.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 103 insertions(+) > > > create mode 100644 rt/tst-bz28213.c > > > > > > diff --git a/rt/Makefile b/rt/Makefile > > > index 113cea03a5..910e775995 100644 > > > --- a/rt/Makefile > > > +++ b/rt/Makefile > > > @@ -74,6 +74,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..b020e3d7dc > > > --- /dev/null > > > +++ b/rt/tst-bz28213.c > > > @@ -0,0 +1,102 @@ > > > +/* Bug 28213: test for NULL pointer dereference in mq_notify. > > > + Copyright (C) 2021-2021 The GNU Toolchain Authors. > > > > No copyright years needed. Oh and congratulations, you're the first > > contributor to add this :) > > > > > + 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/test-driver.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)); > > > > Please wrap at 79 chars. Please review the style and conventions guide > > here: > > > > https://sourceware.org/glibc/wiki/Style_and_Conventions > > > > > + 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); > > > > OK. > > > > > + > > > + /* 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); > > > > OK. > > > > > + > > > + /* Wait... */ > > > + sleep (DEFAULT_TIMEOUT); > > > + FAIL_EXIT1 ("Operation timed out\n"); > > > > With this new logic, you only need to pause() here. The test driver has > > timeout functionality that will automatically fail the test if it stays > > stuck here for DEFAULT_TIMEOUT seconds. > > > > > +} > > > + > > > +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"); > > > > OK. > > > > > + FAIL_EXIT1 ("Failed to create POSIX message queue: %m\n"); > > > + } > > > + > > > + TEST_VERIFY_EXIT (mq_unlink (m_name) == 0); > > > + > > > + check_bz28213 (); > > > + > > > + return 0; > > > +} > > > > OK. > > > > > + > > > +#include <support/test-driver.c> > > > -- > > > 2.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] librt: add test (bug 28213) 2021-08-13 3:28 ` Никита Попов @ 2021-08-13 3:47 ` Siddhesh Poyarekar 0 siblings, 0 replies; 8+ messages in thread From: Siddhesh Poyarekar @ 2021-08-13 3:47 UTC (permalink / raw) To: Никита Попов Cc: libc-alpha On 8/13/21 8:58 AM, Никита Попов wrote: > Hello. I have some concerns about the recent commit > 4cc79c217744743077bf7a0ec5e0a4318f1e6641. In rt/tst-bz28213.c we use > one call to pause (). Should it return for some reason 'return 0' from > do_test () follows. Maybe it is better to wrap pause () call here into > an infinite loop like this? [while (1) pause ();] Then we can be sure > that the only success in this test can be reached via exit (0) from > the callback function. The only way pause() can return is through a signal handler. The only signal we handle in this test is SIGALRM (in the test driver for test timeout) and it results in a test failure. All other signals will result in immediate process termination, which will also result in a test failure. Siddhesh ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-08-13 3:47 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-11 11:26 [PATCH] librt: add test (bug 28213) Никита Попов 2021-08-12 3:10 ` Никита Попов 2021-08-12 4:18 ` Siddhesh Poyarekar 2021-08-12 9:00 ` Никита Попов 2021-08-12 9:34 ` Siddhesh Poyarekar 2021-08-12 10:20 ` Никита Попов 2021-08-13 3:28 ` Никита Попов 2021-08-13 3:47 ` Siddhesh Poyarekar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).