[PATCH] librt: add test (bug 28213)
Никита Попов
npv1310@gmail.com
Fri Aug 13 03:28:07 GMT 2021
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
More information about the Libc-alpha
mailing list