From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v3] Add regression test for CVE-2017-17052
Date: Fri, 19 Jan 2018 17:03:48 +0100 [thread overview]
Message-ID: <20180119160347.GB7954@rei> (raw)
In-Reply-To: <20180112115952.9287-1-mmoese@suse.de>
Hi!
> +#include <unistd.h>
> +#include <pthread.h>
> +#include <sys/wait.h>
> +#include <sys/syscall.h>
> +#include <sys/types.h>
> +
> +#include "tst_test.h"
> +#include "tst_safe_pthread.h"
> +#include "lapi/syscalls.h"
> +
> +#define RUNS 4
> +#define EXEC_USEC 400000
> +
> +struct my_shm_data {
> + int exit;
> +};
> +static struct my_shm_data *shm;
There is no need to pack the the exit into a structure like that, we can
simply do:
static volatile int *do_exit;
...
do_exit = SAFE_MMAP(...);
And it should be volatile as well, so that it's not optimized-out of the
loops by the compiler.
> +static void setup(void)
> +{
> + shm = SAFE_MMAP(NULL, sizeof(struct my_shm_data), PROT_READ|PROT_WRITE,
^
The system aligns the length to be a
multiple of pagesize, so we may as well
pass result of getpagesize() here.
> + MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> +
> + shm->exit = 0;
> +}
> +
> +static void cleanup(void)
> +{
> + SAFE_MUNMAP(shm, sizeof(struct my_shm_data));
^
Here we must pass length that is multiple of
pagesize, at least manual pages says so.
> +}
> +
> +static void *mmap_thread(void *_arg)
Identifiers starting with underscore are reserved for system i.e. libc
we should avoid using these here.
> +{
> + for (;;) {
> + SAFE_MMAP(NULL, 0x1000000, PROT_READ,
> + MAP_POPULATE|MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> + if (shm->exit)
> + exit(0);
> + }
We may as well do:
return arg;
Which is a nice trick to avoid unused warnings.
Also you are supposed to include stdlib.h for exit(3).
> +}
> +
> +static void *fork_thread(void *_arg)
> +{
> + if (shm->exit)
> + exit(0);
> +
> + usleep(rand() % 10000);
> + SAFE_FORK();
> +}
Here as well, the arg should not start with underscore and we should add
return to avoid the warnings as well.
Sorry for not pointing these in the previous review, also no need to
respin the patch, I can fix the minor problems before commiting.
--
Cyril Hrubis
chrubis@suse.cz
next prev parent reply other threads:[~2018-01-19 16:03 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-12 11:59 [LTP] [PATCH v3] Add regression test for CVE-2017-17052 Michael Moese
2018-01-19 16:03 ` Cyril Hrubis [this message]
2018-01-19 17:54 ` Cyril Hrubis
2018-01-20 11:32 ` Michael Moese
2018-01-22 15:48 ` Cyril Hrubis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180119160347.GB7954@rei \
--to=chrubis@suse.cz \
--cc=ltp@lists.linux.it \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.