| Message ID | d14436e64c650b388936a921837b984772a4fceb.1719355322.git.tamas@tklengyel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] Add libfuzzer target to fuzz/x86_instruction_emulator | expand |
On Tue, Jun 25, 2024 at 6:47 PM Tamas K Lengyel <tamas@tklengyel.com> wrote: > > This target enables integration into oss-fuzz. Changing invalid input return > to -1 as values other then 0/-1 are reserved by libfuzzer. Also adding the > missing __wrap_vsnprintf wrapper which is required for successful oss-fuzz > build. > > Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> Patch ping.
On 09.07.2024 17:37, Tamas K Lengyel wrote: > On Tue, Jun 25, 2024 at 6:47 PM Tamas K Lengyel <tamas@tklengyel.com> wrote: >> >> This target enables integration into oss-fuzz. Changing invalid input return >> to -1 as values other then 0/-1 are reserved by libfuzzer. Also adding the >> missing __wrap_vsnprintf wrapper which is required for successful oss-fuzz >> build. >> >> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> > > Patch ping. It's on my list of things to look at, yet even if fully ack-ed it couldn't go in right now anyway. Jan
On Tue, Jul 9, 2024 at 12:12 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 09.07.2024 17:37, Tamas K Lengyel wrote: > > On Tue, Jun 25, 2024 at 6:47 PM Tamas K Lengyel <tamas@tklengyel.com> wrote: > >> > >> This target enables integration into oss-fuzz. Changing invalid input return > >> to -1 as values other then 0/-1 are reserved by libfuzzer. Also adding the > >> missing __wrap_vsnprintf wrapper which is required for successful oss-fuzz > >> build. > >> > >> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> > > > > Patch ping. > > It's on my list of things to look at, yet even if fully ack-ed it couldn't > go in right now anyway. Thanks, just wanted to make sure it's not lost. Tamas
On 26.06.2024 00:47, Tamas K Lengyel wrote: > This target enables integration into oss-fuzz. Changing invalid input return > to -1 as values other then 0/-1 are reserved by libfuzzer. And existing behavior (for afl) is unaffected because there we (wrongly) ignore the return value altogether. > @@ -67,7 +70,8 @@ distclean: clean > > .PHONY: clean > clean: > - rm -f *.a *.o $(DEPS_RM) afl-harness afl-harness-cov *.gcda *.gcno *.gcov > + rm -f *.a *.o $(DEPS_RM) *.gcda *.gcno *.gcov \ > + afl-harness afl-harness-cov libfuzzer-harness > rm -rf x86_emulate x86-emulate.c x86-emulate.h wrappers.c cpuid.c This is what I said for v1: "I'm inclined to suggest it's time to split this line (e.g. keeping all the wildcard patterns together and moving the rest to a new rm invocation)." Could this really be misunderstood to mean anything other than clean: rm -f *.a *.o $(DEPS_RM) *.gcda *.gcno *.gcov rm -f afl-harness afl-harness-cov libfuzzer-harness rm -rf x86_emulate x86-emulate.c x86-emulate.h wrappers.c cpuid.c ? With that Acked-by: Jan Beulich <jbeulich@suse.com> and I'm kind of okay making that adjustment myself while committing. Jan
On Thu, Jul 18, 2024 at 7:03 AM Jan Beulich <jbeulich@suse.com> wrote: > > On 26.06.2024 00:47, Tamas K Lengyel wrote: > > This target enables integration into oss-fuzz. Changing invalid input return > > to -1 as values other then 0/-1 are reserved by libfuzzer. > > And existing behavior (for afl) is unaffected because there we (wrongly) > ignore the return value altogether. > > > @@ -67,7 +70,8 @@ distclean: clean > > > > .PHONY: clean > > clean: > > - rm -f *.a *.o $(DEPS_RM) afl-harness afl-harness-cov *.gcda *.gcno *.gcov > > + rm -f *.a *.o $(DEPS_RM) *.gcda *.gcno *.gcov \ > > + afl-harness afl-harness-cov libfuzzer-harness > > rm -rf x86_emulate x86-emulate.c x86-emulate.h wrappers.c cpuid.c > > This is what I said for v1: > > "I'm inclined to suggest it's time to split this line (e.g. keeping all the > wildcard patterns together and moving the rest to a new rm invocation)." > > Could this really be misunderstood to mean anything other than > > clean: > rm -f *.a *.o $(DEPS_RM) *.gcda *.gcno *.gcov > rm -f afl-harness afl-harness-cov libfuzzer-harness > rm -rf x86_emulate x86-emulate.c x86-emulate.h wrappers.c cpuid.c > > ? Evidently, yes. > With that > Acked-by: Jan Beulich <jbeulich@suse.com> > and I'm kind of okay making that adjustment myself while committing. Thanks! That is appreciated. Tamas
On 26.06.2024 00:47, Tamas K Lengyel wrote: > This target enables integration into oss-fuzz. Changing invalid input return > to -1 as values other then 0/-1 are reserved by libfuzzer. Also adding the > missing __wrap_vsnprintf wrapper which is required for successful oss-fuzz > build. > > Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> I've reverted this right away, because of ... > @@ -58,6 +58,9 @@ afl-harness: afl-harness.o $(OBJS) cpuid.o wrappers.o > afl-harness-cov: afl-harness-cov.o $(patsubst %.o,%-cov.o,$(OBJS)) cpuid.o wrappers.o > $(CC) $(CFLAGS) $(GCOV_FLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ > > +libfuzzer-harness: $(OBJS) cpuid.o wrappers.o > + $(CC) $(CFLAGS) $(LIB_FUZZING_ENGINE) -fsanitize=fuzzer $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ ... this causing gcc: error: unrecognized argument to '-fsanitize=' option: 'fuzzer' make[6]: *** [Makefile:62: libfuzzer-harness] Error 1 with apparently a fair set of gcc-s used by distro-s we use for CI. Jan
On Mon, Jul 22, 2024 at 5:20 AM Jan Beulich <jbeulich@suse.com> wrote: > > On 26.06.2024 00:47, Tamas K Lengyel wrote: > > This target enables integration into oss-fuzz. Changing invalid input return > > to -1 as values other then 0/-1 are reserved by libfuzzer. Also adding the > > missing __wrap_vsnprintf wrapper which is required for successful oss-fuzz > > build. > > > > Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> > > I've reverted this right away, because of ... > > > @@ -58,6 +58,9 @@ afl-harness: afl-harness.o $(OBJS) cpuid.o wrappers.o > > afl-harness-cov: afl-harness-cov.o $(patsubst %.o,%-cov.o,$(OBJS)) cpuid.o wrappers.o > > $(CC) $(CFLAGS) $(GCOV_FLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ > > > > +libfuzzer-harness: $(OBJS) cpuid.o wrappers.o > > + $(CC) $(CFLAGS) $(LIB_FUZZING_ENGINE) -fsanitize=fuzzer $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ > > ... this causing > > gcc: error: unrecognized argument to '-fsanitize=' option: 'fuzzer' > make[6]: *** [Makefile:62: libfuzzer-harness] Error 1 > > with apparently a fair set of gcc-s used by distro-s we use for CI. Well let me see if I can hack the Makefile to only build this with clang.. Tamas
On 22.07.2024 13:03, Tamas K Lengyel wrote: > On Mon, Jul 22, 2024 at 5:20 AM Jan Beulich <jbeulich@suse.com> wrote: >> >> On 26.06.2024 00:47, Tamas K Lengyel wrote: >>> This target enables integration into oss-fuzz. Changing invalid input return >>> to -1 as values other then 0/-1 are reserved by libfuzzer. Also adding the >>> missing __wrap_vsnprintf wrapper which is required for successful oss-fuzz >>> build. >>> >>> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> >> >> I've reverted this right away, because of ... >> >>> @@ -58,6 +58,9 @@ afl-harness: afl-harness.o $(OBJS) cpuid.o wrappers.o >>> afl-harness-cov: afl-harness-cov.o $(patsubst %.o,%-cov.o,$(OBJS)) cpuid.o wrappers.o >>> $(CC) $(CFLAGS) $(GCOV_FLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ >>> >>> +libfuzzer-harness: $(OBJS) cpuid.o wrappers.o >>> + $(CC) $(CFLAGS) $(LIB_FUZZING_ENGINE) -fsanitize=fuzzer $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ >> >> ... this causing >> >> gcc: error: unrecognized argument to '-fsanitize=' option: 'fuzzer' >> make[6]: *** [Makefile:62: libfuzzer-harness] Error 1 >> >> with apparently a fair set of gcc-s used by distro-s we use for CI. > > Well let me see if I can hack the Makefile to only build this with clang.. And perhaps a new enough version thereof? Or has the option been around there forever? Jan
On 22.07.2024 13:03, Tamas K Lengyel wrote: > On Mon, Jul 22, 2024 at 5:20 AM Jan Beulich <jbeulich@suse.com> wrote: >> >> On 26.06.2024 00:47, Tamas K Lengyel wrote: >>> This target enables integration into oss-fuzz. Changing invalid input return >>> to -1 as values other then 0/-1 are reserved by libfuzzer. Also adding the >>> missing __wrap_vsnprintf wrapper which is required for successful oss-fuzz >>> build. >>> >>> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> >> >> I've reverted this right away, because of ... >> >>> @@ -58,6 +58,9 @@ afl-harness: afl-harness.o $(OBJS) cpuid.o wrappers.o >>> afl-harness-cov: afl-harness-cov.o $(patsubst %.o,%-cov.o,$(OBJS)) cpuid.o wrappers.o >>> $(CC) $(CFLAGS) $(GCOV_FLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ >>> >>> +libfuzzer-harness: $(OBJS) cpuid.o wrappers.o >>> + $(CC) $(CFLAGS) $(LIB_FUZZING_ENGINE) -fsanitize=fuzzer $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ >> >> ... this causing >> >> gcc: error: unrecognized argument to '-fsanitize=' option: 'fuzzer' >> make[6]: *** [Makefile:62: libfuzzer-harness] Error 1 >> >> with apparently a fair set of gcc-s used by distro-s we use for CI. > > Well let me see if I can hack the Makefile to only build this with clang.. Oh, and - please don't special case Clang. Instead please check for option availability (e.g. using cc-option), such that for possible future gcc, when support there may have been added, we'd then build it there as well. Jan
On Mon, Jul 22, 2024 at 7:08 AM Jan Beulich <jbeulich@suse.com> wrote: > > On 22.07.2024 13:03, Tamas K Lengyel wrote: > > On Mon, Jul 22, 2024 at 5:20 AM Jan Beulich <jbeulich@suse.com> wrote: > >> > >> On 26.06.2024 00:47, Tamas K Lengyel wrote: > >>> This target enables integration into oss-fuzz. Changing invalid input return > >>> to -1 as values other then 0/-1 are reserved by libfuzzer. Also adding the > >>> missing __wrap_vsnprintf wrapper which is required for successful oss-fuzz > >>> build. > >>> > >>> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> > >> > >> I've reverted this right away, because of ... > >> > >>> @@ -58,6 +58,9 @@ afl-harness: afl-harness.o $(OBJS) cpuid.o wrappers.o > >>> afl-harness-cov: afl-harness-cov.o $(patsubst %.o,%-cov.o,$(OBJS)) cpuid.o wrappers.o > >>> $(CC) $(CFLAGS) $(GCOV_FLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ > >>> > >>> +libfuzzer-harness: $(OBJS) cpuid.o wrappers.o > >>> + $(CC) $(CFLAGS) $(LIB_FUZZING_ENGINE) -fsanitize=fuzzer $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ > >> > >> ... this causing > >> > >> gcc: error: unrecognized argument to '-fsanitize=' option: 'fuzzer' > >> make[6]: *** [Makefile:62: libfuzzer-harness] Error 1 > >> > >> with apparently a fair set of gcc-s used by distro-s we use for CI. > > > > Well let me see if I can hack the Makefile to only build this with clang.. > > Oh, and - please don't special case Clang. Instead please check for option > availability (e.g. using cc-option), such that for possible future gcc, > when support there may have been added, we'd then build it there as well. I decided to just not include the libfuzzer harness in the default 'all' target. Tamas
On 22.07.2024 13:29, Tamas K Lengyel wrote: > On Mon, Jul 22, 2024 at 7:08 AM Jan Beulich <jbeulich@suse.com> wrote: >> >> On 22.07.2024 13:03, Tamas K Lengyel wrote: >>> On Mon, Jul 22, 2024 at 5:20 AM Jan Beulich <jbeulich@suse.com> wrote: >>>> >>>> On 26.06.2024 00:47, Tamas K Lengyel wrote: >>>>> This target enables integration into oss-fuzz. Changing invalid input return >>>>> to -1 as values other then 0/-1 are reserved by libfuzzer. Also adding the >>>>> missing __wrap_vsnprintf wrapper which is required for successful oss-fuzz >>>>> build. >>>>> >>>>> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> >>>> >>>> I've reverted this right away, because of ... >>>> >>>>> @@ -58,6 +58,9 @@ afl-harness: afl-harness.o $(OBJS) cpuid.o wrappers.o >>>>> afl-harness-cov: afl-harness-cov.o $(patsubst %.o,%-cov.o,$(OBJS)) cpuid.o wrappers.o >>>>> $(CC) $(CFLAGS) $(GCOV_FLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ >>>>> >>>>> +libfuzzer-harness: $(OBJS) cpuid.o wrappers.o >>>>> + $(CC) $(CFLAGS) $(LIB_FUZZING_ENGINE) -fsanitize=fuzzer $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ >>>> >>>> ... this causing >>>> >>>> gcc: error: unrecognized argument to '-fsanitize=' option: 'fuzzer' >>>> make[6]: *** [Makefile:62: libfuzzer-harness] Error 1 >>>> >>>> with apparently a fair set of gcc-s used by distro-s we use for CI. >>> >>> Well let me see if I can hack the Makefile to only build this with clang.. >> >> Oh, and - please don't special case Clang. Instead please check for option >> availability (e.g. using cc-option), such that for possible future gcc, >> when support there may have been added, we'd then build it there as well. > > I decided to just not include the libfuzzer harness in the default 'all' target. Hmm. I'll look (and comment) there, but I'm not sure that's a route we want to take. Goals generally ought to work or be unavailable, I'm inclined to say. Jan
On Mon, Jul 22, 2024 at 7:34 AM Jan Beulich <jbeulich@suse.com> wrote: > > On 22.07.2024 13:29, Tamas K Lengyel wrote: > > On Mon, Jul 22, 2024 at 7:08 AM Jan Beulich <jbeulich@suse.com> wrote: > >> > >> On 22.07.2024 13:03, Tamas K Lengyel wrote: > >>> On Mon, Jul 22, 2024 at 5:20 AM Jan Beulich <jbeulich@suse.com> wrote: > >>>> > >>>> On 26.06.2024 00:47, Tamas K Lengyel wrote: > >>>>> This target enables integration into oss-fuzz. Changing invalid input return > >>>>> to -1 as values other then 0/-1 are reserved by libfuzzer. Also adding the > >>>>> missing __wrap_vsnprintf wrapper which is required for successful oss-fuzz > >>>>> build. > >>>>> > >>>>> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> > >>>> > >>>> I've reverted this right away, because of ... > >>>> > >>>>> @@ -58,6 +58,9 @@ afl-harness: afl-harness.o $(OBJS) cpuid.o wrappers.o > >>>>> afl-harness-cov: afl-harness-cov.o $(patsubst %.o,%-cov.o,$(OBJS)) cpuid.o wrappers.o > >>>>> $(CC) $(CFLAGS) $(GCOV_FLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ > >>>>> > >>>>> +libfuzzer-harness: $(OBJS) cpuid.o wrappers.o > >>>>> + $(CC) $(CFLAGS) $(LIB_FUZZING_ENGINE) -fsanitize=fuzzer $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ > >>>> > >>>> ... this causing > >>>> > >>>> gcc: error: unrecognized argument to '-fsanitize=' option: 'fuzzer' > >>>> make[6]: *** [Makefile:62: libfuzzer-harness] Error 1 > >>>> > >>>> with apparently a fair set of gcc-s used by distro-s we use for CI. > >>> > >>> Well let me see if I can hack the Makefile to only build this with clang.. > >> > >> Oh, and - please don't special case Clang. Instead please check for option > >> availability (e.g. using cc-option), such that for possible future gcc, > >> when support there may have been added, we'd then build it there as well. > > > > I decided to just not include the libfuzzer harness in the default 'all' target. > > Hmm. I'll look (and comment) there, but I'm not sure that's a route we want to > take. Goals generally ought to work or be unavailable, I'm inclined to say. That Makefile already has targets that are not part of all so I don't think it's a big deal. Tamas
diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile index 1e4c6b37f5..7b6655805f 100644 --- a/tools/fuzz/x86_instruction_emulator/Makefile +++ b/tools/fuzz/x86_instruction_emulator/Makefile @@ -3,7 +3,7 @@ include $(XEN_ROOT)/tools/Rules.mk .PHONY: x86-insn-fuzz-all ifeq ($(CONFIG_X86_64),y) -x86-insn-fuzz-all: x86-insn-fuzzer.a fuzz-emul.o afl +x86-insn-fuzz-all: x86-insn-fuzzer.a fuzz-emul.o afl libfuzzer else x86-insn-fuzz-all: endif @@ -58,6 +58,9 @@ afl-harness: afl-harness.o $(OBJS) cpuid.o wrappers.o afl-harness-cov: afl-harness-cov.o $(patsubst %.o,%-cov.o,$(OBJS)) cpuid.o wrappers.o $(CC) $(CFLAGS) $(GCOV_FLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ +libfuzzer-harness: $(OBJS) cpuid.o wrappers.o + $(CC) $(CFLAGS) $(LIB_FUZZING_ENGINE) -fsanitize=fuzzer $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@ + # Common targets .PHONY: all all: x86-insn-fuzz-all @@ -67,7 +70,8 @@ distclean: clean .PHONY: clean clean: - rm -f *.a *.o $(DEPS_RM) afl-harness afl-harness-cov *.gcda *.gcno *.gcov + rm -f *.a *.o $(DEPS_RM) *.gcda *.gcno *.gcov \ + afl-harness afl-harness-cov libfuzzer-harness rm -rf x86_emulate x86-emulate.c x86-emulate.h wrappers.c cpuid.c .PHONY: install @@ -81,4 +85,7 @@ afl: afl-harness .PHONY: afl-cov afl-cov: afl-harness-cov +.PHONY: libfuzzer +libfuzzer: libfuzzer-harness + -include $(DEPS_INCLUDE) diff --git a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c index eeeb6931f4..2ba9ca9e0b 100644 --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c @@ -906,14 +906,12 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size) if ( size <= DATA_OFFSET ) { - printf("Input too small\n"); - return 1; + return -1; } if ( size > FUZZ_CORPUS_SIZE ) { - printf("Input too large\n"); - return 1; + return -1; } memcpy(&input, data_p, size); diff --git a/tools/tests/x86_emulator/wrappers.c b/tools/tests/x86_emulator/wrappers.c index 3829a6f416..8f3bd1656f 100644 --- a/tools/tests/x86_emulator/wrappers.c +++ b/tools/tests/x86_emulator/wrappers.c @@ -91,6 +91,17 @@ int __wrap_snprintf(char *buf, size_t n, const char *fmt, ...) return rc; } +int __wrap_vsnprintf(char *buf, size_t n, const char *fmt, va_list varg) +{ + int rc; + + emul_save_fpu_state(); + rc = __real_vsnprintf(buf, n, fmt, varg); + emul_restore_fpu_state(); + + return rc; +} + char *__wrap_strstr(const char *s1, const char *s2) { char *s;
This target enables integration into oss-fuzz. Changing invalid input return to -1 as values other then 0/-1 are reserved by libfuzzer. Also adding the missing __wrap_vsnprintf wrapper which is required for successful oss-fuzz build. Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> --- tools/fuzz/x86_instruction_emulator/Makefile | 11 +++++++++-- tools/fuzz/x86_instruction_emulator/fuzz-emul.c | 6 ++---- tools/tests/x86_emulator/wrappers.c | 11 +++++++++++ 3 files changed, 22 insertions(+), 6 deletions(-)