Re: [PATCH 1/2] Add libfuzzer target to fuzz/x86_instruction_emulator

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.06.2024 21:14, Tamas K Lengyel wrote:
> @@ -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
> +     $(CC) $(CFLAGS) $(LIB_FUZZING_ENGINE) -fsanitize=fuzzer $^ -o $@

What is LIB_FUZZING_ENGINE? I don't think we have any use of that in the
tree anywhere.

I'm further surprised you get away here without wrappers.o.

Finally, despite its base name the lack of an extension suggest to me
this isn't actually a library. Can you help me bring both aspects together?

> @@ -67,7 +70,7 @@ 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) afl-harness afl-harness-cov *.gcda *.gcno 
> *.gcov libfuzzer-harness

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).

> --- 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);

This part of the change clearly needs explaining in the description.
It's not even clear to me in how far this is related to the purpose
of the patch here (iow it may want to be a separate change, depending
on why the change is needed).

Jan