| Message ID | 20191009142446.6997-1-rpalethorpe@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] LTP Wrapper for Syzkaller reproducers | expand |
On Wed, Oct 9, 2019 at 4:26 PM Richard Palethorpe <rpalethorpe@suse.com> wrote: > > First attempt at wrapping the Syzkaller reproducers in the LTP library. I am > posting this in case anyone wants to experiment with it early or has a > radically different approach in mind. > > This just uses exec to run the reproducer executables as per Metan's > suggestion. There is a simple script which creates a runtest file allowing it > to work with existing LTP test runners, albeit with a bit of extra work for > now. > > This would benefit from the following LTP library patch: > https://patchwork.ozlabs.org/patch/935568/ > > Running it without KASAN and the other kernel debugging options is not a good > idea. We can easily detect when the kernel config is wrong and print a > warning or even refuse to run, but I haven't added it yet. > > Having to download, compile and install the reproducers seperately is annoying > and I bet most users won't do it. We can probably automate that as part of the > install, it is just a question of how much we do as default. > > --- > runtest/.gitignore | 1 + > testcases/kernel/Makefile | 1 + > testcases/kernel/syzkaller-repros/.gitignore | 1 + > testcases/kernel/syzkaller-repros/Makefile | 10 +++ > testcases/kernel/syzkaller-repros/README.md | 39 +++++++++ > .../kernel/syzkaller-repros/gen-runtest.sh | 8 ++ > testcases/kernel/syzkaller-repros/syzwrap.c | 85 +++++++++++++++++++ > 7 files changed, 145 insertions(+) > create mode 100644 runtest/.gitignore > create mode 100644 testcases/kernel/syzkaller-repros/.gitignore > create mode 100644 testcases/kernel/syzkaller-repros/Makefile > create mode 100644 testcases/kernel/syzkaller-repros/README.md > create mode 100755 testcases/kernel/syzkaller-repros/gen-runtest.sh > create mode 100644 testcases/kernel/syzkaller-repros/syzwrap.c > > diff --git a/runtest/.gitignore b/runtest/.gitignore > new file mode 100644 > index 000000000..e3725dd42 > --- /dev/null > +++ b/runtest/.gitignore > @@ -0,0 +1 @@ > +syzkaller-repros > diff --git a/testcases/kernel/Makefile b/testcases/kernel/Makefile > index 3319b3163..0150cfb4f 100644 > --- a/testcases/kernel/Makefile > +++ b/testcases/kernel/Makefile > @@ -53,6 +53,7 @@ SUBDIRS += connectors \ > sched \ > security \ > sound \ > + syzkaller-repros \ > tracing \ > uevents \ > > diff --git a/testcases/kernel/syzkaller-repros/.gitignore b/testcases/kernel/syzkaller-repros/.gitignore > new file mode 100644 > index 000000000..dbda1c71f > --- /dev/null > +++ b/testcases/kernel/syzkaller-repros/.gitignore > @@ -0,0 +1 @@ > +syzwrap > diff --git a/testcases/kernel/syzkaller-repros/Makefile b/testcases/kernel/syzkaller-repros/Makefile > new file mode 100644 > index 000000000..8e74805c2 > --- /dev/null > +++ b/testcases/kernel/syzkaller-repros/Makefile > @@ -0,0 +1,10 @@ > +# SPDX-License-Identifier: GPL-2.0-or-later > +# Copyright (c) 2019 Linux Test Project > + > +top_srcdir ?= ../../.. > + > +include $(top_srcdir)/include/mk/testcases.mk > + > +CFLAGS += -D_GNU_SOURCE > + > +include $(top_srcdir)/include/mk/generic_leaf_target.mk > diff --git a/testcases/kernel/syzkaller-repros/README.md b/testcases/kernel/syzkaller-repros/README.md > new file mode 100644 > index 000000000..e95ae19e2 > --- /dev/null > +++ b/testcases/kernel/syzkaller-repros/README.md > @@ -0,0 +1,39 @@ > +LTP wrapper for Syzkaller reproducers > +===================================== > + > +This allows you to run the autogenerated bug reproducers from the Syzkaller > +fuzzer within the LTP framework. Meaning that you may use an existing test > +runner compatible with the LTP. > + > +However some extra setup is currently required. > + > +Instructions > +------------ > + > +1. Download and compile the reproducers. > +2. Build the LTP as normal > +3. Use the gen-runtest.sh script to create a runtest file > +4. Install the LTP and the reproducers to the SUT > +5. Execute the tests in the syzkaller-repros runtest file > + > +For now you can download the reproducers from here: > +https://github.com/dvyukov/syzkaller-repros. Soon they will be available on > +kernel.org. > + > +The gen-runtest takes two arguments: > + > +1. The directory where the reproducer executables are currently accessible > +2. The *absolute* path to the directory where they will be on the SUT (If > + different, can be omitted) > + > +For example: > +``` > +./gen-runtest.sh ~/qa/syzkaller-repros/bin /mnt/syzkaller-repros/bin > > +~/qa/ltp-build/runtest/syzkaller-repros > +``` > + > +For the LTP, just doing `make install` will copy all the relevant files > +(assuming you put the runtest file in the runtest folder). However you will > +need to copy the reproducers yourself. > + > + > diff --git a/testcases/kernel/syzkaller-repros/gen-runtest.sh b/testcases/kernel/syzkaller-repros/gen-runtest.sh > new file mode 100755 > index 000000000..091818fb2 > --- /dev/null > +++ b/testcases/kernel/syzkaller-repros/gen-runtest.sh > @@ -0,0 +1,8 @@ > +#!/usr/bin/sh > + > +BUILD_DIR=$1 > +SUT_DIR=$2 > + > +for f in $(ls $BUILD_DIR); do > + echo $f syzwrap -d ${SUT_DIR:-$BUILD_DIR} -n $f > +done > diff --git a/testcases/kernel/syzkaller-repros/syzwrap.c b/testcases/kernel/syzkaller-repros/syzwrap.c > new file mode 100644 > index 000000000..7951d1819 > --- /dev/null > +++ b/testcases/kernel/syzkaller-repros/syzwrap.c > @@ -0,0 +1,85 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2019 Richard Palethorpe <rpalethorpe@suse.com> > + * > + * Run a single reproducer generated by the Syzkaller fuzzer. > + */ > + > +#include <sys/types.h> > +#include <sys/wait.h> > +#include <signal.h> > +#include <stdio.h> > + > +#include "tst_test.h" > +#include "tst_taint.h" > +#include "tst_safe_stdio.h" > + > +static char *dir; > +static char *name; > +static char *path; > + > +static struct tst_option options[] = { > + {"d:", &dir, "Mandatory directory containing reproducers"}, > + {"n:", &name, "Mandatory executable name of reproducer"}, > + {NULL, NULL, NULL} > +}; > + > +static void setup(void) > +{ > + tst_taint_init(TST_TAINT_W | TST_TAINT_D); > + > + if (!dir) > + tst_brk(TBROK, "No reproducer directory specified"); > + > + if (!name) > + tst_brk(TBROK, "No reproducer name specified"); > + > + tst_res(TINFO, "https://syzkaller.appspot.com/bug?id=%s", name); > + > + SAFE_ASPRINTF(&path, "%s/%s", dir, name); > + tst_res(TINFO, "%s", path); > +} > + > +static void run(void) > +{ > + unsigned int backoff = 100; > + int rem, status, sent_kill = 0; > + float exec_time_start = (float)tst_timeout_remaining(); > + int pid = SAFE_FORK(); > + > + if (!pid) { > + execle(path, name, environ); > + tst_brk(TBROK | TERRNO, "Failed to exec reproducer"); > + } > + > + while (!waitpid(pid, &status, WNOHANG)) { > + rem = tst_timeout_remaining(); > + > + if (!sent_kill && rem / exec_time_start < 0.98) { > + tst_res(TINFO, "Timeout; killing reproducer"); > + > + TEST(kill(pid, SIGKILL)); > + if (TST_RET == -1) > + tst_res(TWARN | TTERRNO, "kill() failed"); > + else > + sent_kill = 1; > + } > + > + usleep(backoff); > + backoff = MIN(2 * backoff, 1000000); > + } > + > + if (tst_taint_check()) { > + tst_res(TFAIL, "Kernel is tainted"); > + } else { > + tst_res(TPASS, "Kernel is not tainted"); > + } > +} > + > +static struct tst_test test = { > + .setup = setup, > + .test_all = run, > + .options = options, > + .needs_tmpdir = 1, > + .forks_child = 1, > +}; Hi Richard, I don't have prior experience with LTP tests, but from reading the code it looks reasonable to me. I assume that .needs_tmpdir = 1 ensures that each test runs in its own new temp dir, which is later removed. I've stared for a while at "rem / exec_time_start < 0.98" trying to understand what is that tst_timeout_remaining() returns that we want to kill that process when the ratio is < 0.98... provided that we convert 1 to float but not the other var. I failed to come up with the answer. I have potential answers for "<0.02" and ">0.98". But I assume you know what you are doing :) Re tst_res(TINFO, "Timeout; killing reproducer"). Not sure how much it pollutes output on 3000 tests. If it is, it can make sense to remove it. Lots of tests run forever, killing it is not something of particular interest generally.
Hi! > I don't have prior experience with LTP tests, but from reading the > code it looks reasonable to me. > > I assume that .needs_tmpdir = 1 ensures that each test runs in its own > new temp dir, which is later removed. Indeed, it's removed recursively by the test library. > I've stared for a while at "rem / exec_time_start < 0.98" trying to > understand what is that tst_timeout_remaining() returns that we want > to kill that process when the ratio is < 0.98... provided that we > convert 1 to float but not the other var. I failed to come up with the > answer. I have potential answers for "<0.02" and ">0.98". But I assume > you know what you are doing :) The tst_timeout_remaining() returns remaining test timeout, so at test start it returns something close to 300 seconds, since that is a default for a LTP tests, so this would probably kill a process quite fast, if I'm reading right, after a bit more than five seconds. I guess that this is something intended for a quick v1 hack rather than for later use. > Re tst_res(TINFO, "Timeout; killing reproducer"). Not sure how much it > pollutes output on 3000 tests. If it is, it can make sense to remove > it. Lots of tests run forever, killing it is not something of > particular interest generally. I guess so.
On Wed, Oct 9, 2019 at 4:54 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > I don't have prior experience with LTP tests, but from reading the > > code it looks reasonable to me. > > > > I assume that .needs_tmpdir = 1 ensures that each test runs in its own > > new temp dir, which is later removed. > > Indeed, it's removed recursively by the test library. :popcorn: It took me several years to figure out how to more or less reliably remove dirs after the fuzzer ;) (no, unlink won't do ;)) > > I've stared for a while at "rem / exec_time_start < 0.98" trying to > > understand what is that tst_timeout_remaining() returns that we want > > to kill that process when the ratio is < 0.98... provided that we > > convert 1 to float but not the other var. I failed to come up with the > > answer. I have potential answers for "<0.02" and ">0.98". But I assume > > you know what you are doing :) Ah, I see, it's not full timeout, then it makes sense. Probably a comment won't harm. > The tst_timeout_remaining() returns remaining test timeout, so at test > start it returns something close to 300 seconds, since that is a default > for a LTP tests, so this would probably kill a process quite fast, if > I'm reading right, after a bit more than five seconds. I guess that this > is something intended for a quick v1 hack rather than for later use. > > > Re tst_res(TINFO, "Timeout; killing reproducer"). Not sure how much it > > pollutes output on 3000 tests. If it is, it can make sense to remove > > it. Lots of tests run forever, killing it is not something of > > particular interest generally. > > I guess so. > > -- > Cyril Hrubis > chrubis@suse.cz
Hi! > > Indeed, it's removed recursively by the test library. > > :popcorn: > > It took me several years to figure out how to more or less reliably > remove dirs after the fuzzer ;) > (no, unlink won't do ;)) I guess that there are things such as immutable file attributes that has to be cleared and many more. Do you have piece of code somewhere that we can look into to spare us from reinventing the wheel?
On Wed, Oct 9, 2019 at 8:04 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > > Indeed, it's removed recursively by the test library. > > > > :popcorn: > > > > It took me several years to figure out how to more or less reliably > > remove dirs after the fuzzer ;) > > (no, unlink won't do ;)) > > I guess that there are things such as immutable file attributes that has > to be cleared and many more. Do you have piece of code somewhere that we > can look into to spare us from reinventing the wheel? Here is what we have: https://github.com/google/syzkaller/blob/c4b9981b5f5b70dc03eb3f76c618398510101a1d/executor/common_linux.h#L2358-L2461 Maybe it can be simplified, but that's what we ended up with after some organic evolution. At least the comments may give some hints as to what may go wrong.
Hi! > > > > Indeed, it's removed recursively by the test library. > > > > > > :popcorn: > > > > > > It took me several years to figure out how to more or less reliably > > > remove dirs after the fuzzer ;) > > > (no, unlink won't do ;)) > > > > I guess that there are things such as immutable file attributes that has > > to be cleared and many more. Do you have piece of code somewhere that we > > can look into to spare us from reinventing the wheel? > > Here is what we have: > https://github.com/google/syzkaller/blob/c4b9981b5f5b70dc03eb3f76c618398510101a1d/executor/common_linux.h#L2358-L2461 > Maybe it can be simplified, but that's what we ended up with after > some organic evolution. At least the comments may give some hints as > to what may go wrong. Thanks a lot! Also I see that you are using namespaces, and much more, to sandbox the fuzzer, I was wondering if we should do that, at least separate user and pid namespace sounds like a good idea to me.
On Thu, Oct 10, 2019 at 11:30 AM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > > > > Indeed, it's removed recursively by the test library. > > > > > > > > :popcorn: > > > > > > > > It took me several years to figure out how to more or less reliably > > > > remove dirs after the fuzzer ;) > > > > (no, unlink won't do ;)) > > > > > > I guess that there are things such as immutable file attributes that has > > > to be cleared and many more. Do you have piece of code somewhere that we > > > can look into to spare us from reinventing the wheel? > > > > Here is what we have: > > https://github.com/google/syzkaller/blob/c4b9981b5f5b70dc03eb3f76c618398510101a1d/executor/common_linux.h#L2358-L2461 > > Maybe it can be simplified, but that's what we ended up with after > > some organic evolution. At least the comments may give some hints as > > to what may go wrong. > > Thanks a lot! > > Also I see that you are using namespaces, and much more, to sandbox the > fuzzer, I was wondering if we should do that, at least separate user and > pid namespace sounds like a good idea to me. I don't know how far you want to go. This sandboxing definitely helps us to isolate processes and make tests more repeatable by avoiding interference (I don't know if LTP, say, runs tests in parallel). mount namespaces are useful to later drop all of test mounts at once, this would solve a significant part of the remote_dir logic. If the temp dir is on tmpfs in the mount namespace as well, then it will be automatically dropped altogether with all contents.
Hi! > > > > > > Indeed, it's removed recursively by the test library. > > > > > > > > > > :popcorn: > > > > > > > > > > It took me several years to figure out how to more or less reliably > > > > > remove dirs after the fuzzer ;) > > > > > (no, unlink won't do ;)) > > > > > > > > I guess that there are things such as immutable file attributes that has > > > > to be cleared and many more. Do you have piece of code somewhere that we > > > > can look into to spare us from reinventing the wheel? > > > > > > Here is what we have: > > > https://github.com/google/syzkaller/blob/c4b9981b5f5b70dc03eb3f76c618398510101a1d/executor/common_linux.h#L2358-L2461 > > > Maybe it can be simplified, but that's what we ended up with after > > > some organic evolution. At least the comments may give some hints as > > > to what may go wrong. > > > > Thanks a lot! > > > > Also I see that you are using namespaces, and much more, to sandbox the > > fuzzer, I was wondering if we should do that, at least separate user and > > pid namespace sounds like a good idea to me. > > I don't know how far you want to go. This sandboxing definitely helps > us to isolate processes and make tests more repeatable by avoiding > interference (I don't know if LTP, say, runs tests in parallel). Not yet, but we are slowly getting to a point where LTP tests could be run in parallel. It's a bit more complicated for functional tests, since there are number of constraints, for which tests should not be run in parallel. And for number of these sandboxing wouldn't help either, since it's more of a matter of available resources than isolation. I'm close to solving first half of the problem, i.e. propagating the test constraints from tests to the testrunner. I also wrote a blog post about this, you can read it at: https://people.kernel.org/metan/towards-parallel-kernel-test-runs But even without running tests in parallel there are resources that have kernel persistence and will outlive the process, such as SysV IPC. So I guess that at least some sandboxing has to be done even for non-parallel runs. > mount namespaces are useful to later drop all of test mounts at once, > this would solve a significant part of the remote_dir logic. If the > temp dir is on tmpfs in the mount namespace as well, then it will be > automatically dropped altogether with all contents. Again, thanks for the hint!
diff --git a/runtest/.gitignore b/runtest/.gitignore new file mode 100644 index 000000000..e3725dd42 --- /dev/null +++ b/runtest/.gitignore @@ -0,0 +1 @@ +syzkaller-repros diff --git a/testcases/kernel/Makefile b/testcases/kernel/Makefile index 3319b3163..0150cfb4f 100644 --- a/testcases/kernel/Makefile +++ b/testcases/kernel/Makefile @@ -53,6 +53,7 @@ SUBDIRS += connectors \ sched \ security \ sound \ + syzkaller-repros \ tracing \ uevents \ diff --git a/testcases/kernel/syzkaller-repros/.gitignore b/testcases/kernel/syzkaller-repros/.gitignore new file mode 100644 index 000000000..dbda1c71f --- /dev/null +++ b/testcases/kernel/syzkaller-repros/.gitignore @@ -0,0 +1 @@ +syzwrap diff --git a/testcases/kernel/syzkaller-repros/Makefile b/testcases/kernel/syzkaller-repros/Makefile new file mode 100644 index 000000000..8e74805c2 --- /dev/null +++ b/testcases/kernel/syzkaller-repros/Makefile @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (c) 2019 Linux Test Project + +top_srcdir ?= ../../.. + +include $(top_srcdir)/include/mk/testcases.mk + +CFLAGS += -D_GNU_SOURCE + +include $(top_srcdir)/include/mk/generic_leaf_target.mk diff --git a/testcases/kernel/syzkaller-repros/README.md b/testcases/kernel/syzkaller-repros/README.md new file mode 100644 index 000000000..e95ae19e2 --- /dev/null +++ b/testcases/kernel/syzkaller-repros/README.md @@ -0,0 +1,39 @@ +LTP wrapper for Syzkaller reproducers +===================================== + +This allows you to run the autogenerated bug reproducers from the Syzkaller +fuzzer within the LTP framework. Meaning that you may use an existing test +runner compatible with the LTP. + +However some extra setup is currently required. + +Instructions +------------ + +1. Download and compile the reproducers. +2. Build the LTP as normal +3. Use the gen-runtest.sh script to create a runtest file +4. Install the LTP and the reproducers to the SUT +5. Execute the tests in the syzkaller-repros runtest file + +For now you can download the reproducers from here: +https://github.com/dvyukov/syzkaller-repros. Soon they will be available on +kernel.org. + +The gen-runtest takes two arguments: + +1. The directory where the reproducer executables are currently accessible +2. The *absolute* path to the directory where they will be on the SUT (If + different, can be omitted) + +For example: +``` +./gen-runtest.sh ~/qa/syzkaller-repros/bin /mnt/syzkaller-repros/bin > +~/qa/ltp-build/runtest/syzkaller-repros +``` + +For the LTP, just doing `make install` will copy all the relevant files +(assuming you put the runtest file in the runtest folder). However you will +need to copy the reproducers yourself. + + diff --git a/testcases/kernel/syzkaller-repros/gen-runtest.sh b/testcases/kernel/syzkaller-repros/gen-runtest.sh new file mode 100755 index 000000000..091818fb2 --- /dev/null +++ b/testcases/kernel/syzkaller-repros/gen-runtest.sh @@ -0,0 +1,8 @@ +#!/usr/bin/sh + +BUILD_DIR=$1 +SUT_DIR=$2 + +for f in $(ls $BUILD_DIR); do + echo $f syzwrap -d ${SUT_DIR:-$BUILD_DIR} -n $f +done diff --git a/testcases/kernel/syzkaller-repros/syzwrap.c b/testcases/kernel/syzkaller-repros/syzwrap.c new file mode 100644 index 000000000..7951d1819 --- /dev/null +++ b/testcases/kernel/syzkaller-repros/syzwrap.c @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2019 Richard Palethorpe <rpalethorpe@suse.com> + * + * Run a single reproducer generated by the Syzkaller fuzzer. + */ + +#include <sys/types.h> +#include <sys/wait.h> +#include <signal.h> +#include <stdio.h> + +#include "tst_test.h" +#include "tst_taint.h" +#include "tst_safe_stdio.h" + +static char *dir; +static char *name; +static char *path; + +static struct tst_option options[] = { + {"d:", &dir, "Mandatory directory containing reproducers"}, + {"n:", &name, "Mandatory executable name of reproducer"}, + {NULL, NULL, NULL} +}; + +static void setup(void) +{ + tst_taint_init(TST_TAINT_W | TST_TAINT_D); + + if (!dir) + tst_brk(TBROK, "No reproducer directory specified"); + + if (!name) + tst_brk(TBROK, "No reproducer name specified"); + + tst_res(TINFO, "https://syzkaller.appspot.com/bug?id=%s", name); + + SAFE_ASPRINTF(&path, "%s/%s", dir, name); + tst_res(TINFO, "%s", path); +} + +static void run(void) +{ + unsigned int backoff = 100; + int rem, status, sent_kill = 0; + float exec_time_start = (float)tst_timeout_remaining(); + int pid = SAFE_FORK(); + + if (!pid) { + execle(path, name, environ); + tst_brk(TBROK | TERRNO, "Failed to exec reproducer"); + } + + while (!waitpid(pid, &status, WNOHANG)) { + rem = tst_timeout_remaining(); + + if (!sent_kill && rem / exec_time_start < 0.98) { + tst_res(TINFO, "Timeout; killing reproducer"); + + TEST(kill(pid, SIGKILL)); + if (TST_RET == -1) + tst_res(TWARN | TTERRNO, "kill() failed"); + else + sent_kill = 1; + } + + usleep(backoff); + backoff = MIN(2 * backoff, 1000000); + } + + if (tst_taint_check()) { + tst_res(TFAIL, "Kernel is tainted"); + } else { + tst_res(TPASS, "Kernel is not tainted"); + } +} + +static struct tst_test test = { + .setup = setup, + .test_all = run, + .options = options, + .needs_tmpdir = 1, + .forks_child = 1, +};