Re: [PATCH 09/10] drivers/auxdisplay: add a KFuzzTest for parse_xy()

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Thu, Dec 4, 2025 at 4:13 PM Ethan Graham <ethan.w.s.graham@gmail.com> wrote:
>
> From: Ethan Graham <ethangraham@google.com>
>
> Add a KFuzzTest fuzzer for the parse_xy() function, located in a new
> file under /drivers/auxdisplay/tests.

drivers/...

(no leading /)

> To validate the correctness and effectiveness of this KFuzzTest target,
> a bug was injected into parse_xy() like so:
>
> drivers/auxdisplay/charlcd.c:179
> - s = p;
> + s = p + 1;
>
> Although a simple off-by-one bug, it requires a specific input sequence
> in order to trigger it, thus demonstrating the power of pairing
> KFuzzTest with a coverage-guided fuzzer like syzkaller.

fuzzers

> Signed-off-by: Ethan Graham <ethangraham@google.com>
> Signed-off-by: Ethan Graham <ethan.w.s.graham@gmail.com>

I believe one of two SoBs is enough.

> Acked-by: Alexander Potapenko <glider@google.com>

...

> --- a/drivers/auxdisplay/Makefile
> +++ b/drivers/auxdisplay/Makefile
> @@ -6,6 +6,9 @@
>  obj-$(CONFIG_ARM_CHARLCD)      += arm-charlcd.o
>  obj-$(CONFIG_CFAG12864B)       += cfag12864b.o cfag12864bfb.o
>  obj-$(CONFIG_CHARLCD)          += charlcd.o
> +ifeq ($(CONFIG_KFUZZTEST),y)
> +CFLAGS_charlcd.o += -include $(src)/tests/charlcd_kfuzz.c
> +endif
>  obj-$(CONFIG_HD44780_COMMON)   += hd44780_common.o
>  obj-$(CONFIG_HD44780)          += hd44780.o
>  obj-$(CONFIG_HT16K33)          += ht16k33.o

Yes, this level of intrusion is fine to me.

...

> +++ b/drivers/auxdisplay/tests/charlcd_kfuzz.c

So, this will require it to be expanded each time we want to add
coverage. Can this be actually generated based on the C
(preprocessed?) level of prototypes listed? Ideally I would like to
see only some small meta-data and then the fuzzer should create the
object based on the profile of the module.

Input like:

bool parse_xy(const char *s $nonnull$, unsigned long *x $nonnull$,
unsigned long *y $nonnull$)
Or even with the expected ranges, and then you can generate a code
that tests the behaviour inside given ranges and outside, including
invalid input, etc.

But okay, the below seems not too big enough.

> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * charlcd KFuzzTest target
> + *
> + * Copyright 2025 Google LLC
> + */
> +#include <linux/kfuzztest.h>
> +
> +struct parse_xy_arg {
> +       const char *s;
> +};

> +static bool parse_xy(const char *s, unsigned long *x, unsigned long *y);

Is it still needed?

I mean, can we make sure that include in this case works as tail one
and not head, because otherwise we would need to add the respective
includes, i.e. for bool type here, which is missing. Also I *hope&
that kfuzztest.h is NOT Yet Another Include EVERYTHING type of
headers. Otherwise it breaks the whole idea behind modularity of the
headers.

> +FUZZ_TEST(test_parse_xy, struct parse_xy_arg)
> +{
> +       unsigned long x, y;
> +
> +       KFUZZTEST_EXPECT_NOT_NULL(parse_xy_arg, s);
> +       KFUZZTEST_ANNOTATE_STRING(parse_xy_arg, s);
> +       parse_xy(arg->s, &x, &y);
> +}


-- 
With Best Regards,
Andy Shevchenko