linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Ethan Graham <ethan.w.s.graham@gmail.com>
Cc: ethangraham@google.com, glider@google.com, andreyknvl@gmail.com,
	andy@kernel.org, brauner@kernel.org, brendan.higgins@linux.dev,
	davem@davemloft.net, davidgow@google.com, dhowells@redhat.com,
	dvyukov@google.com, elver@google.com,
	herbert@gondor.apana.org.au, ignat@cloudflare.com, jack@suse.cz,
	jannh@google.com, johannes@sipsolutions.net,
	kasan-dev@googlegroups.com, kunit-dev@googlegroups.com,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, lukas@wunner.de, rmoar@google.com,
	shuah@kernel.org, sj@kernel.org, tarasmadan@google.com
Subject: Re: [PATCH v2 09/10] fs/binfmt_script: add KFuzzTest target for load_script
Date: Fri, 19 Sep 2025 12:19:42 -0700	[thread overview]
Message-ID: <202509191208.D2BCFD366F@keescook> (raw)
In-Reply-To: <20250919145750.3448393-10-ethan.w.s.graham@gmail.com>

On Fri, Sep 19, 2025 at 02:57:49PM +0000, Ethan Graham wrote:
> From: Ethan Graham <ethangraham@google.com>
> 
> Add a KFuzzTest target for the load_script function to serve as a
> real-world example of the framework's usage.
> 
> The load_script function is responsible for parsing the shebang line
> (`#!`) of script files. This makes it an excellent candidate for
> KFuzzTest, as it involves parsing user-controlled data within the
> binary loading path, which is not directly exposed as a system call.
> 
> The provided fuzz target in fs/tests/binfmt_script_kfuzz.c illustrates
> how to fuzz a function that requires more involved setup - here, we only
> let the fuzzer generate input for the `buf` field of struct linux_bprm,
> and manually set the other fields with sensible values inside of the
> FUZZ_TEST body.
> 
> To demonstrate the effectiveness of the fuzz target, a buffer overflow
> bug was injected in the load_script function like so:
> 
> - buf_end = bprm->buf + sizeof(bprm->buf) - 1;
> + buf_end = bprm->buf + sizeof(bprm->buf) + 1;
> 
> Which was caught in around 40 seconds by syzkaller simultaneously
> fuzzing four other targets, a realistic use case where targets are
> continuously fuzzed. It also requires that the fuzzer be smart enough to
> generate an input starting with `#!`.
> 
> While this bug is shallow, the fact that the bug is caught quickly and
> with minimal additional code can potentially be a source of confidence
> when modifying existing implementations or writing new functions.
> 
> Signed-off-by: Ethan Graham <ethangraham@google.com>
> 
> ---
> PR v2:
> - Introduce cleanup logic in the load_script fuzz target.
> ---
> ---
>  fs/binfmt_script.c             |  8 +++++
>  fs/tests/binfmt_script_kfuzz.c | 58 ++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
>  create mode 100644 fs/tests/binfmt_script_kfuzz.c
> 
> diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
> index 637daf6e4d45..c09f224d6d7e 100644
> --- a/fs/binfmt_script.c
> +++ b/fs/binfmt_script.c
> @@ -157,3 +157,11 @@ core_initcall(init_script_binfmt);
>  module_exit(exit_script_binfmt);
>  MODULE_DESCRIPTION("Kernel support for scripts starting with #!");
>  MODULE_LICENSE("GPL");
> +
> +/*
> + * When CONFIG_KFUZZTEST is enabled, we include this _kfuzz.c file to ensure
> + * that KFuzzTest targets are built.
> + */
> +#ifdef CONFIG_KFUZZTEST
> +#include "tests/binfmt_script_kfuzz.c"
> +#endif /* CONFIG_KFUZZTEST */
> diff --git a/fs/tests/binfmt_script_kfuzz.c b/fs/tests/binfmt_script_kfuzz.c
> new file mode 100644
> index 000000000000..26397a465270
> --- /dev/null
> +++ b/fs/tests/binfmt_script_kfuzz.c
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * binfmt_script loader KFuzzTest target
> + *
> + * Copyright 2025 Google LLC
> + */
> +#include <linux/binfmts.h>
> +#include <linux/kfuzztest.h>
> +#include <linux/slab.h>
> +#include <linux/sched/mm.h>
> +
> +struct load_script_arg {
> +	char buf[BINPRM_BUF_SIZE];
> +};
> +
> +FUZZ_TEST(test_load_script, struct load_script_arg)
> +{
> +	struct linux_binprm bprm = {};
> +	char *arg_page;
> +
> +	arg_page = (char *)get_zeroed_page(GFP_KERNEL);
> +	if (!arg_page)
> +		return;
> +
> +	memcpy(bprm.buf, arg->buf, sizeof(bprm.buf));
> +	/*
> +	 * `load_script` calls remove_arg_zero, which expects argc != 0. A
> +	 * static value of 1 is sufficient for fuzzing.
> +	 */
> +	bprm.argc = 1;
> +	bprm.p = (unsigned long)arg_page + PAGE_SIZE;
> +	bprm.filename = kstrdup("fuzz_script", GFP_KERNEL);
> +	if (!bprm.filename)
> +		goto cleanup;
> +	bprm.interp = kstrdup(bprm.filename, GFP_KERNEL);
> +	if (!bprm.interp)
> +		goto cleanup;
> +
> +	bprm.mm = mm_alloc();
> +	if (!bprm.mm)
> +		goto cleanup;
> +
> +	/*
> +	 * Call the target function. We expect it to fail and return an error
> +	 * (e.g., at open_exec), which is fine. The goal is to survive the
> +	 * initial parsing logic without crashing.
> +	 */
> +	load_script(&bprm);
> +
> +cleanup:
> +	if (bprm.mm)
> +		mmput(bprm.mm);
> +	if (bprm.interp)
> +		kfree(bprm.interp);
> +	if (bprm.filename)
> +		kfree(bprm.filename);
> +	free_page((unsigned long)arg_page);
> +}

Yay fuzzing hooks! I'm excited about this series overall, but I'm not
a fan of this "manual" init/clean up of bprm.

If you're going to set up a bprm that passes through load_script(), it
needs to be both prepared correctly (alloc_bprm) and cleaned up correctly
(free_bprm). Otherwise, you may be fuzzing impossible states created by
the fuzztest setup. And having a second init/cleanup path in here makes
future refactoring work more of a burden/fragile.

But this is also kind of not a great example of fuzztest utility because
load_script _is_ actually directly accessible from syscalls: it is trivial
to externally fuzz load_script by just writing the buffer to a file and
execve'ing it. :)

-Kees

-- 
Kees Cook


  parent reply	other threads:[~2025-09-19 19:19 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-19 14:57 [PATCH v2 0/10] KFuzzTest: a new kernel fuzzing framework Ethan Graham
2025-09-19 14:57 ` [PATCH v2 01/10] mm/kasan: implement kasan_poison_range Ethan Graham
2025-09-23 16:46   ` Andrey Konovalov
2025-09-19 14:57 ` [PATCH v2 02/10] kfuzztest: add user-facing API and data structures Ethan Graham
2025-09-19 15:05   ` Alexander Potapenko
2025-09-24  8:44   ` Johannes Berg
2025-09-19 14:57 ` [PATCH v2 03/10] kfuzztest: implement core module and input processing Ethan Graham
2025-09-19 15:05   ` Alexander Potapenko
2025-09-19 14:57 ` [PATCH v2 04/10] tools: add kfuzztest-bridge utility Ethan Graham
2025-09-19 15:05   ` Alexander Potapenko
2025-09-19 14:57 ` [PATCH v2 05/10] kfuzztest: add ReST documentation Ethan Graham
2025-09-19 14:57 ` [PATCH v2 06/10] kfuzztest: add KFuzzTest sample fuzz targets Ethan Graham
2025-09-19 14:57 ` [PATCH v2 07/10] crypto: implement KFuzzTest targets for PKCS7 and RSA parsing Ethan Graham
2025-09-19 14:57 ` [PATCH v2 08/10] drivers/auxdisplay: add a KFuzzTest for parse_xy() Ethan Graham
2025-09-19 15:07   ` Alexander Potapenko
2025-09-20 10:53   ` Andy Shevchenko
2025-09-20 12:08     ` Alexander Potapenko
2025-09-20 12:47       ` Lukas Wunner
2025-09-21 18:25         ` Andy Shevchenko
2025-09-24  9:28   ` kernel test robot
2025-09-19 14:57 ` [PATCH v2 09/10] fs/binfmt_script: add KFuzzTest target for load_script Ethan Graham
2025-09-19 15:07   ` Alexander Potapenko
2025-09-19 19:19   ` Kees Cook [this message]
2025-09-19 14:57 ` [PATCH v2 10/10] MAINTAINERS: add maintainer information for KFuzzTest Ethan Graham
2025-09-24  8:32   ` SeongJae Park
2025-09-19 15:04 ` [PATCH v2 0/10] KFuzzTest: a new kernel fuzzing framework Alexander Potapenko
2025-09-24 12:52 ` Johannes Berg
2025-09-25  8:35   ` Ethan Graham
2025-10-24  8:37     ` Johannes Berg
2025-10-28 17:38       ` Alexander Potapenko

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=202509191208.D2BCFD366F@keescook \
    --to=kees@kernel.org \
    --cc=andreyknvl@gmail.com \
    --cc=andy@kernel.org \
    --cc=brauner@kernel.org \
    --cc=brendan.higgins@linux.dev \
    --cc=davem@davemloft.net \
    --cc=davidgow@google.com \
    --cc=dhowells@redhat.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=ethan.w.s.graham@gmail.com \
    --cc=ethangraham@google.com \
    --cc=glider@google.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=ignat@cloudflare.com \
    --cc=jack@suse.cz \
    --cc=jannh@google.com \
    --cc=johannes@sipsolutions.net \
    --cc=kasan-dev@googlegroups.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lukas@wunner.de \
    --cc=rmoar@google.com \
    --cc=shuah@kernel.org \
    --cc=sj@kernel.org \
    --cc=tarasmadan@google.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox