From: Ethan Graham <ethangraham@google.com>

Add KFuzzTest targets for pkcs7_parse_message, rsa_parse_pub_key, and
rsa_parse_priv_key to serve as real-world examples of how the framework is used.

These functions are ideal candidates for KFuzzTest as they perform complex
parsing of user-controlled data but are not directly exposed at the syscall
boundary. This makes them difficult to exercise with traditional fuzzing tools
and showcases the primary strength of the KFuzzTest framework: providing an
interface to fuzz internal, non-exported kernel functions.

The targets are defined directly within the source files of the functions they
test, demonstrating how to colocate fuzz tests with the code under test.

Signed-off-by: Ethan Graham <ethangraham@google.com>
---
 crypto/asymmetric_keys/pkcs7_parser.c | 15 ++++++++++++++
 crypto/rsa_helper.c                   | 29 +++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
index 423d13c47545..e8477f8b0eaf 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -13,6 +13,7 @@
 #include <linux/err.h>
 #include <linux/oid_registry.h>
 #include <crypto/public_key.h>
+#include <linux/kfuzztest.h>
 #include "pkcs7_parser.h"
 #include "pkcs7.asn1.h"
 
@@ -169,6 +170,20 @@ struct pkcs7_message *pkcs7_parse_message(const void *data, size_t datalen)
 }
 EXPORT_SYMBOL_GPL(pkcs7_parse_message);
 
+struct pkcs7_parse_message_arg {
+	const void *data;
+	size_t datalen;
+};
+
+FUZZ_TEST(test_pkcs7_parse_message, struct pkcs7_parse_message_arg)
+{
+	KFUZZTEST_EXPECT_NOT_NULL(pkcs7_parse_message_arg, data);
+	KFUZZTEST_ANNOTATE_LEN(pkcs7_parse_message_arg, datalen, data);
+	KFUZZTEST_EXPECT_LE(pkcs7_parse_message_arg, datalen, 16 * PAGE_SIZE);
+
+	pkcs7_parse_message(arg->data, arg->datalen);
+}
+
 /**
  * pkcs7_get_content_data - Get access to the PKCS#7 content
  * @pkcs7: The preparsed PKCS#7 message to access
diff --git a/crypto/rsa_helper.c b/crypto/rsa_helper.c
index 94266f29049c..79b7ddc7c48d 100644
--- a/crypto/rsa_helper.c
+++ b/crypto/rsa_helper.c
@@ -9,6 +9,7 @@
 #include <linux/export.h>
 #include <linux/err.h>
 #include <linux/fips.h>
+#include <linux/kfuzztest.h>
 #include <crypto/internal/rsa.h>
 #include "rsapubkey.asn1.h"
 #include "rsaprivkey.asn1.h"
@@ -166,6 +167,20 @@ int rsa_parse_pub_key(struct rsa_key *rsa_key, const void *key,
 }
 EXPORT_SYMBOL_GPL(rsa_parse_pub_key);
 
+struct rsa_parse_pub_key_arg {
+	const void *key;
+	size_t key_len;
+};
+
+FUZZ_TEST(test_rsa_parse_pub_key, struct rsa_parse_pub_key_arg)
+{
+	KFUZZTEST_EXPECT_NOT_NULL(rsa_parse_pub_key_arg, key);
+	KFUZZTEST_EXPECT_LE(rsa_parse_pub_key_arg, key_len, 16 * PAGE_SIZE);
+
+	struct rsa_key out;
+	rsa_parse_pub_key(&out, arg->key, arg->key_len);
+}
+
 /**
  * rsa_parse_priv_key() - decodes the BER encoded buffer and stores in the
  *                        provided struct rsa_key, pointers to the raw key
@@ -184,3 +199,17 @@ int rsa_parse_priv_key(struct rsa_key *rsa_key, const void *key,
 	return asn1_ber_decoder(&rsaprivkey_decoder, rsa_key, key, key_len);
 }
 EXPORT_SYMBOL_GPL(rsa_parse_priv_key);
+
+struct rsa_parse_priv_key_arg {
+	const void *key;
+	size_t key_len;
+};
+
+FUZZ_TEST(test_rsa_parse_priv_key, struct rsa_parse_priv_key_arg)
+{
+	KFUZZTEST_EXPECT_NOT_NULL(rsa_parse_priv_key_arg, key);
+	KFUZZTEST_EXPECT_LE(rsa_parse_priv_key_arg, key_len, 16 * PAGE_SIZE);
+
+	struct rsa_key out;
+	rsa_parse_priv_key(&out, arg->key, arg->key_len);
+}
-- 
2.51.0.rc0.205.g4a044479a3-goog

[+Cc crypto maintainers]

On Wed, 13 Aug 2025 at 15:38, Ethan Graham <ethan.w.s.graham@gmail.com> wrote:
>
> From: Ethan Graham <ethangraham@google.com>

Should also Cc crypto maintainers, as they'll be the ones giving
feedback on how interesting this is to them. Use
./scripts/get_maintainer.pl for that in the next round, and either add
the Cc list below your Signed-off-by so that git send-email picks it
up only for this patch, or just for the whole series (normally
preferred, so maintainers get context of the full series).

> Add KFuzzTest targets for pkcs7_parse_message, rsa_parse_pub_key, and
> rsa_parse_priv_key to serve as real-world examples of how the framework is used.
>
> These functions are ideal candidates for KFuzzTest as they perform complex
> parsing of user-controlled data but are not directly exposed at the syscall
> boundary. This makes them difficult to exercise with traditional fuzzing tools
> and showcases the primary strength of the KFuzzTest framework: providing an
> interface to fuzz internal, non-exported kernel functions.
>
> The targets are defined directly within the source files of the functions they
> test, demonstrating how to colocate fuzz tests with the code under test.
>
> Signed-off-by: Ethan Graham <ethangraham@google.com>
> ---
>  crypto/asymmetric_keys/pkcs7_parser.c | 15 ++++++++++++++
>  crypto/rsa_helper.c                   | 29 +++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
>
> diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
> index 423d13c47545..e8477f8b0eaf 100644
> --- a/crypto/asymmetric_keys/pkcs7_parser.c
> +++ b/crypto/asymmetric_keys/pkcs7_parser.c
> @@ -13,6 +13,7 @@
>  #include <linux/err.h>
>  #include <linux/oid_registry.h>
>  #include <crypto/public_key.h>
> +#include <linux/kfuzztest.h>
>  #include "pkcs7_parser.h"
>  #include "pkcs7.asn1.h"
>
> @@ -169,6 +170,20 @@ struct pkcs7_message *pkcs7_parse_message(const void *data, size_t datalen)
>  }
>  EXPORT_SYMBOL_GPL(pkcs7_parse_message);
>
> +struct pkcs7_parse_message_arg {
> +       const void *data;
> +       size_t datalen;
> +};
> +
> +FUZZ_TEST(test_pkcs7_parse_message, struct pkcs7_parse_message_arg)
> +{
> +       KFUZZTEST_EXPECT_NOT_NULL(pkcs7_parse_message_arg, data);
> +       KFUZZTEST_ANNOTATE_LEN(pkcs7_parse_message_arg, datalen, data);
> +       KFUZZTEST_EXPECT_LE(pkcs7_parse_message_arg, datalen, 16 * PAGE_SIZE);
> +
> +       pkcs7_parse_message(arg->data, arg->datalen);
> +}
> +
>  /**
>   * pkcs7_get_content_data - Get access to the PKCS#7 content
>   * @pkcs7: The preparsed PKCS#7 message to access
> diff --git a/crypto/rsa_helper.c b/crypto/rsa_helper.c
> index 94266f29049c..79b7ddc7c48d 100644
> --- a/crypto/rsa_helper.c
> +++ b/crypto/rsa_helper.c
> @@ -9,6 +9,7 @@
>  #include <linux/export.h>
>  #include <linux/err.h>
>  #include <linux/fips.h>
> +#include <linux/kfuzztest.h>
>  #include <crypto/internal/rsa.h>
>  #include "rsapubkey.asn1.h"
>  #include "rsaprivkey.asn1.h"
> @@ -166,6 +167,20 @@ int rsa_parse_pub_key(struct rsa_key *rsa_key, const void *key,
>  }
>  EXPORT_SYMBOL_GPL(rsa_parse_pub_key);
>
> +struct rsa_parse_pub_key_arg {
> +       const void *key;
> +       size_t key_len;
> +};
> +
> +FUZZ_TEST(test_rsa_parse_pub_key, struct rsa_parse_pub_key_arg)
> +{
> +       KFUZZTEST_EXPECT_NOT_NULL(rsa_parse_pub_key_arg, key);
> +       KFUZZTEST_EXPECT_LE(rsa_parse_pub_key_arg, key_len, 16 * PAGE_SIZE);
> +
> +       struct rsa_key out;
> +       rsa_parse_pub_key(&out, arg->key, arg->key_len);
> +}
> +
>  /**
>   * rsa_parse_priv_key() - decodes the BER encoded buffer and stores in the
>   *                        provided struct rsa_key, pointers to the raw key
> @@ -184,3 +199,17 @@ int rsa_parse_priv_key(struct rsa_key *rsa_key, const void *key,
>         return asn1_ber_decoder(&rsaprivkey_decoder, rsa_key, key, key_len);
>  }
>  EXPORT_SYMBOL_GPL(rsa_parse_priv_key);
> +
> +struct rsa_parse_priv_key_arg {
> +       const void *key;
> +       size_t key_len;
> +};
> +
> +FUZZ_TEST(test_rsa_parse_priv_key, struct rsa_parse_priv_key_arg)
> +{
> +       KFUZZTEST_EXPECT_NOT_NULL(rsa_parse_priv_key_arg, key);
> +       KFUZZTEST_EXPECT_LE(rsa_parse_priv_key_arg, key_len, 16 * PAGE_SIZE);
> +
> +       struct rsa_key out;
> +       rsa_parse_priv_key(&out, arg->key, arg->key_len);
> +}
> --
> 2.51.0.rc0.205.g4a044479a3-goog
>

On Wed, Aug 13, 2025 at 7:14 PM Marco Elver <elver@google.com> wrote:
>
> [+Cc crypto maintainers]
>
> On Wed, 13 Aug 2025 at 15:38, Ethan Graham <ethan.w.s.graham@gmail.com> wrote:
> >
> > From: Ethan Graham <ethangraham@google.com>
>
> Should also Cc crypto maintainers, as they'll be the ones giving

Thanks Marco!

> feedback on how interesting this is to them. Use
> ./scripts/get_maintainer.pl for that in the next round, and either add
> the Cc list below your Signed-off-by so that git send-email picks it
> up only for this patch, or just for the whole series (normally
> preferred, so maintainers get context of the full series).
>
> > Add KFuzzTest targets for pkcs7_parse_message, rsa_parse_pub_key, and
> > rsa_parse_priv_key to serve as real-world examples of how the framework is used.
> >
> > These functions are ideal candidates for KFuzzTest as they perform complex
> > parsing of user-controlled data but are not directly exposed at the syscall
> > boundary. This makes them difficult to exercise with traditional fuzzing tools
> > and showcases the primary strength of the KFuzzTest framework: providing an
> > interface to fuzz internal, non-exported kernel functions.
> >
> > The targets are defined directly within the source files of the functions they
> > test, demonstrating how to colocate fuzz tests with the code under test.
> >
> > Signed-off-by: Ethan Graham <ethangraham@google.com>
> > ---
> >  crypto/asymmetric_keys/pkcs7_parser.c | 15 ++++++++++++++
> >  crypto/rsa_helper.c                   | 29 +++++++++++++++++++++++++++
> >  2 files changed, 44 insertions(+)
> >
> > diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
> > index 423d13c47545..e8477f8b0eaf 100644
> > --- a/crypto/asymmetric_keys/pkcs7_parser.c
> > +++ b/crypto/asymmetric_keys/pkcs7_parser.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/err.h>
> >  #include <linux/oid_registry.h>
> >  #include <crypto/public_key.h>
> > +#include <linux/kfuzztest.h>
> >  #include "pkcs7_parser.h"
> >  #include "pkcs7.asn1.h"
> >
> > @@ -169,6 +170,20 @@ struct pkcs7_message *pkcs7_parse_message(const void *data, size_t datalen)
> >  }
> >  EXPORT_SYMBOL_GPL(pkcs7_parse_message);
> >
> > +struct pkcs7_parse_message_arg {
> > +       const void *data;
> > +       size_t datalen;
> > +};
> > +
> > +FUZZ_TEST(test_pkcs7_parse_message, struct pkcs7_parse_message_arg)

Not sure if it has been mentioned elsewhere, but one thing I already
don't like about it is that these definitions "pollute" the actual
source files. Might not be such a big deal here, but kernel source
files for core subsystems tend to become quite large and complex
already, so not a great idea to make them even larger and harder to
follow with fuzz definitions.

As far as I'm aware, for the same reason KUnit [1] is not that popular
(or at least less popular than other approaches, like selftests [2]).
Is it possible to make it that these definitions live in separate
files or even closer to selftests?

Ignat

> > +{
> > +       KFUZZTEST_EXPECT_NOT_NULL(pkcs7_parse_message_arg, data);
> > +       KFUZZTEST_ANNOTATE_LEN(pkcs7_parse_message_arg, datalen, data);
> > +       KFUZZTEST_EXPECT_LE(pkcs7_parse_message_arg, datalen, 16 * PAGE_SIZE);
> > +
> > +       pkcs7_parse_message(arg->data, arg->datalen);
> > +}
> > +
> >  /**
> >   * pkcs7_get_content_data - Get access to the PKCS#7 content
> >   * @pkcs7: The preparsed PKCS#7 message to access
> > diff --git a/crypto/rsa_helper.c b/crypto/rsa_helper.c
> > index 94266f29049c..79b7ddc7c48d 100644
> > --- a/crypto/rsa_helper.c
> > +++ b/crypto/rsa_helper.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/export.h>
> >  #include <linux/err.h>
> >  #include <linux/fips.h>
> > +#include <linux/kfuzztest.h>
> >  #include <crypto/internal/rsa.h>
> >  #include "rsapubkey.asn1.h"
> >  #include "rsaprivkey.asn1.h"
> > @@ -166,6 +167,20 @@ int rsa_parse_pub_key(struct rsa_key *rsa_key, const void *key,
> >  }
> >  EXPORT_SYMBOL_GPL(rsa_parse_pub_key);
> >
> > +struct rsa_parse_pub_key_arg {
> > +       const void *key;
> > +       size_t key_len;
> > +};
> > +
> > +FUZZ_TEST(test_rsa_parse_pub_key, struct rsa_parse_pub_key_arg)
> > +{
> > +       KFUZZTEST_EXPECT_NOT_NULL(rsa_parse_pub_key_arg, key);
> > +       KFUZZTEST_EXPECT_LE(rsa_parse_pub_key_arg, key_len, 16 * PAGE_SIZE);
> > +
> > +       struct rsa_key out;
> > +       rsa_parse_pub_key(&out, arg->key, arg->key_len);
> > +}
> > +
> >  /**
> >   * rsa_parse_priv_key() - decodes the BER encoded buffer and stores in the
> >   *                        provided struct rsa_key, pointers to the raw key
> > @@ -184,3 +199,17 @@ int rsa_parse_priv_key(struct rsa_key *rsa_key, const void *key,
> >         return asn1_ber_decoder(&rsaprivkey_decoder, rsa_key, key, key_len);
> >  }
> >  EXPORT_SYMBOL_GPL(rsa_parse_priv_key);
> > +
> > +struct rsa_parse_priv_key_arg {
> > +       const void *key;
> > +       size_t key_len;
> > +};
> > +
> > +FUZZ_TEST(test_rsa_parse_priv_key, struct rsa_parse_priv_key_arg)
> > +{
> > +       KFUZZTEST_EXPECT_NOT_NULL(rsa_parse_priv_key_arg, key);
> > +       KFUZZTEST_EXPECT_LE(rsa_parse_priv_key_arg, key_len, 16 * PAGE_SIZE);
> > +
> > +       struct rsa_key out;
> > +       rsa_parse_priv_key(&out, arg->key, arg->key_len);
> > +}
> > --
> > 2.51.0.rc0.205.g4a044479a3-goog
> >

[1]: https://docs.kernel.org/dev-tools/kunit/index.html
[2]: https://docs.kernel.org/dev-tools/kselftest.html

On Thu, Aug 14, 2025 at 04:28:13PM +0100, Ignat Korchagin wrote:
> Not sure if it has been mentioned elsewhere, but one thing I already
> don't like about it is that these definitions "pollute" the actual
> source files. Might not be such a big deal here, but kernel source
> files for core subsystems tend to become quite large and complex
> already, so not a great idea to make them even larger and harder to
> follow with fuzz definitions.
> 
> As far as I'm aware, for the same reason KUnit [1] is not that popular
> (or at least less popular than other approaches, like selftests [2]).
> Is it possible to make it that these definitions live in separate
> files or even closer to selftests?

That's not the impression I get.  KUnit suites are normally defined in
separate files, and KUnit seems to be increasing in popularity.
KFuzzTest can use separate files too, it looks like?

Would it make any sense for fuzz tests to be a special type of KUnit
test, instead of a separate framework?

- Eric

On Fri, Aug 15, 2025 at 2:18 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Thu, Aug 14, 2025 at 04:28:13PM +0100, Ignat Korchagin wrote:
> > Not sure if it has been mentioned elsewhere, but one thing I already
> > don't like about it is that these definitions "pollute" the actual
> > source files. Might not be such a big deal here, but kernel source
> > files for core subsystems tend to become quite large and complex
> > already, so not a great idea to make them even larger and harder to
> > follow with fuzz definitions.
> >
> > As far as I'm aware, for the same reason KUnit [1] is not that popular
> > (or at least less popular than other approaches, like selftests [2]).
> > Is it possible to make it that these definitions live in separate
> > files or even closer to selftests?
>
> That's not the impression I get.  KUnit suites are normally defined in
> separate files, and KUnit seems to be increasing in popularity.

Great! Either I was wrong from the start or it changed and I haven't
looked there recently.

> KFuzzTest can use separate files too, it looks like?
>
> Would it make any sense for fuzz tests to be a special type of KUnit
> test, instead of a separate framework?

I think so, if possible. There is always some hurdles adopting new
framework, but if it would be a new feature of an existing one (either
KUnit or selftests - whatever fits better semantically), the existing
users of that framework are more likely to pick it up.

> - Eric

On Fri, 15 Aug 2025 at 15:00, Ignat Korchagin <ignat@cloudflare.com> wrote:
>
> On Fri, Aug 15, 2025 at 2:18 AM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Thu, Aug 14, 2025 at 04:28:13PM +0100, Ignat Korchagin wrote:
> > > Not sure if it has been mentioned elsewhere, but one thing I already
> > > don't like about it is that these definitions "pollute" the actual
> > > source files. Might not be such a big deal here, but kernel source
> > > files for core subsystems tend to become quite large and complex
> > > already, so not a great idea to make them even larger and harder to
> > > follow with fuzz definitions.
> > >
> > > As far as I'm aware, for the same reason KUnit [1] is not that popular
> > > (or at least less popular than other approaches, like selftests [2]).
> > > Is it possible to make it that these definitions live in separate
> > > files or even closer to selftests?
> >
> > That's not the impression I get.  KUnit suites are normally defined in
> > separate files, and KUnit seems to be increasing in popularity.
>
> Great! Either I was wrong from the start or it changed and I haven't
> looked there recently.
>
> > KFuzzTest can use separate files too, it looks like?
> >
> > Would it make any sense for fuzz tests to be a special type of KUnit
> > test, instead of a separate framework?
>
> I think so, if possible. There is always some hurdles adopting new
> framework, but if it would be a new feature of an existing one (either
> KUnit or selftests - whatever fits better semantically), the existing
> users of that framework are more likely to pick it up.

The dependency would be in name only (i.e. "branding"). Right now it's
possible to use KFuzzTest without the KUnit dependency. So there is
technical merit to decouple.

Would sufficient documentation, and perhaps suggesting separate files
to be the canonical way of defining KFuzzTests, improve the situation?

For example something like:
For subsystem foo.c, define a KFuzzTest in foo_kfuzz.c, and then in
the Makfile add "obj-$(CONFIG_KFUZZTEST) += foo_kfuzz.o".
Alternatively, to test internal static functions, place the KFuzzTest
harness in a file foo_kfuzz.h, and include at the bottom of foo.c.

Alex, Ethan, and KUnit folks: What's your preference?

On Tue, 19 Aug 2025 at 18:08, Marco Elver <elver@google.com> wrote:
>
> On Fri, 15 Aug 2025 at 15:00, Ignat Korchagin <ignat@cloudflare.com> wrote:
> >
> > On Fri, Aug 15, 2025 at 2:18 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > >
> > > On Thu, Aug 14, 2025 at 04:28:13PM +0100, Ignat Korchagin wrote:
> > > > Not sure if it has been mentioned elsewhere, but one thing I already
> > > > don't like about it is that these definitions "pollute" the actual
> > > > source files. Might not be such a big deal here, but kernel source
> > > > files for core subsystems tend to become quite large and complex
> > > > already, so not a great idea to make them even larger and harder to
> > > > follow with fuzz definitions.
> > > >
> > > > As far as I'm aware, for the same reason KUnit [1] is not that popular
> > > > (or at least less popular than other approaches, like selftests [2]).
> > > > Is it possible to make it that these definitions live in separate
> > > > files or even closer to selftests?
> > >
> > > That's not the impression I get.  KUnit suites are normally defined in
> > > separate files, and KUnit seems to be increasing in popularity.
> >
> > Great! Either I was wrong from the start or it changed and I haven't
> > looked there recently.
> >
> > > KFuzzTest can use separate files too, it looks like?
> > >
> > > Would it make any sense for fuzz tests to be a special type of KUnit
> > > test, instead of a separate framework?
> >
> > I think so, if possible. There is always some hurdles adopting new
> > framework, but if it would be a new feature of an existing one (either
> > KUnit or selftests - whatever fits better semantically), the existing
> > users of that framework are more likely to pick it up.
>
> The dependency would be in name only (i.e. "branding"). Right now it's
> possible to use KFuzzTest without the KUnit dependency. So there is
> technical merit to decouple.
>

There's definitely some overlap between KFuzzTest and KUnit, from the
relatively superficial API similarities: both having similar
ASSERT/EXPECT macros; to the more specific: KUnit parameterised tests
allow running the same 'test' code against several different pieces of
input data.

Then again, there are definitely some differences, too: KUnit doesn't
have a way of describing complex binary data (though it's definitely a
feature we'd like one day), and the purpose of KUnit tests, while
having some overlap, is different than exposing fuzz targets.

If there's a bit of KUnit functionality you can reasonably re-use or
otherwise take advantage of, I'd not discourage you from doing so.
There's a balance to be struck between taking the extra dependency and
ending up with duplicate implementations of the same thing.

I also think that what Ignat says below around simply ensuring that
the API is familiar (i.e., not deviating from what KUnit or other
frameworks do without a good reason) is a good middle ground here.

So my gut feeling is that you could end up with one of three things:
- Make KFuzzTests a special case of (parameterised) KUnit tests. This
would probably involve adding a way to run the tests with a parameter
from debugfs or a kernel command-line argument using the metadata
format, and teaching the fuzzer to run KUnit tests. KUnit already has
an attributes mechanism that could be used to note which tests are
fuzz targets, and maybe even providing some of the annotation, but
there'd be some work needed to improve it. The big advantage here is
that you'd automatically gain the ability to use KUnit helpers to set
up things like memory regions, fake devices, etc.
- Share some of the implementation details between KUnit and
KFuzzTest, but keep them as separate things. We already have a bunch
of, e.g., work on assertions, logging, etc, which could possibly be
helpful. This could be done by depending on CONFIG_KUNIT or by
splitting those out into a shared test library.
- Keep them separate, but be careful to make the APIs similar enough
to be familiar. KFuzzTest already looks pretty similar to me, so I
think we're already in a good place here.

Personally, I'd quite like for there to be a bit more overlap/code
sharing -- at least eventually -- as I could see some benefits to
"borrowing" some KFuzzTest code to allow, e.g., providing custom
inputs/outputs for tests.

> Would sufficient documentation, and perhaps suggesting separate files
> to be the canonical way of defining KFuzzTests, improve the situation?
>
> For example something like:
> For subsystem foo.c, define a KFuzzTest in foo_kfuzz.c, and then in
> the Makfile add "obj-$(CONFIG_KFUZZTEST) += foo_kfuzz.o".
> Alternatively, to test internal static functions, place the KFuzzTest
> harness in a file foo_kfuzz.h, and include at the bottom of foo.c.
>
> Alex, Ethan, and KUnit folks: What's your preference?

I think that keeping tests in separate files by default is the right
way to go, but obviously either #including them or using a whole bunch
of conditional symbol exports (either with symbol namespaces or
something like EXPORT_SYMBOL_IF_KUNIT) will be necessary in some cases
to get coverage of internal functions.

I'd suggest being a little careful with the naming scheme, as Linus
was not happy with the foo_test.c names we were using as they make tab
completion more annoying; we ended up putting tests in a 'tests/'
subdirectory where appropriate:
https://docs.kernel.org/dev-tools/kunit/style.html

But ultimately, I think this is a style decision, not a critically
important technical one: provide some good practices to follow -- and
encourage people to be consistent -- but understand that occasionally
a maintainer will override it (sometimes even for good reason).

Cheers,
-- David

On Tue, Aug 19, 2025 at 12:08 PM Marco Elver <elver@google.com> wrote:
> For example something like:
> For subsystem foo.c, define a KFuzzTest in foo_kfuzz.c, and then in
> the Makfile add "obj-$(CONFIG_KFUZZTEST) += foo_kfuzz.o".

I agree that fuzz targets should only be built if CONFIG_KFUZZTEST is
enabled. Building a separate foo_kfuzz.o is probably ideal, but will
need to think about how to cleanly handle static functions.

> Alternatively, to test internal static functions, place the KFuzzTest
> harness in a file foo_kfuzz.h, and include at the bottom of foo.c.
>
> Alex, Ethan, and KUnit folks: What's your preference?

I think placing fuzz targets in separate files is a step in the right
direction. Including a foo_kfuzz.h file inside of the source does still
pollute the file to some extent but certainly less than having one or
more KFuzzTest targets defined alongside the code.

On Tue, Aug 19, 2025 at 11:08 AM Marco Elver <elver@google.com> wrote:
>
> On Fri, 15 Aug 2025 at 15:00, Ignat Korchagin <ignat@cloudflare.com> wrote:
> >
> > On Fri, Aug 15, 2025 at 2:18 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > >
> > > On Thu, Aug 14, 2025 at 04:28:13PM +0100, Ignat Korchagin wrote:
> > > > Not sure if it has been mentioned elsewhere, but one thing I already
> > > > don't like about it is that these definitions "pollute" the actual
> > > > source files. Might not be such a big deal here, but kernel source
> > > > files for core subsystems tend to become quite large and complex
> > > > already, so not a great idea to make them even larger and harder to
> > > > follow with fuzz definitions.
> > > >
> > > > As far as I'm aware, for the same reason KUnit [1] is not that popular
> > > > (or at least less popular than other approaches, like selftests [2]).
> > > > Is it possible to make it that these definitions live in separate
> > > > files or even closer to selftests?
> > >
> > > That's not the impression I get.  KUnit suites are normally defined in
> > > separate files, and KUnit seems to be increasing in popularity.
> >
> > Great! Either I was wrong from the start or it changed and I haven't
> > looked there recently.
> >
> > > KFuzzTest can use separate files too, it looks like?
> > >
> > > Would it make any sense for fuzz tests to be a special type of KUnit
> > > test, instead of a separate framework?
> >
> > I think so, if possible. There is always some hurdles adopting new
> > framework, but if it would be a new feature of an existing one (either
> > KUnit or selftests - whatever fits better semantically), the existing
> > users of that framework are more likely to pick it up.
>
> The dependency would be in name only (i.e. "branding"). Right now it's
> possible to use KFuzzTest without the KUnit dependency. So there is
> technical merit to decouple.

Probably strong (Kbuild) dependency is not what I was thinking about,
rather just semantical similarity. That is, if I "learned" KUnit -
KFuzzTest is easy to pick up for me.

> Would sufficient documentation, and perhaps suggesting separate files
> to be the canonical way of defining KFuzzTests, improve the situation?

Probably.

> For example something like:
> For subsystem foo.c, define a KFuzzTest in foo_kfuzz.c, and then in
> the Makfile add "obj-$(CONFIG_KFUZZTEST) += foo_kfuzz.o".
> Alternatively, to test internal static functions, place the KFuzzTest
> harness in a file foo_kfuzz.h, and include at the bottom of foo.c.

Having includes at the bottom of the file feels weird and "leaks"
kfuzz tests into the sources. Perhaps we can somehow rely on the fact
that kernel is a flat address space and you can always get the address
of a symbol (even if static - similar to how eBPF kprobes do it)? Or
have a bit more complex Kbuild configuration: for example
"foo_kfuzz.c" would include "foo.c" (although including .c files also
feels weird). If CONFIG_KFUZZTEST is disabled, Kbuild just includes
"foo.o", if enabled we include "foo_kfuzz.o" (which includes foo.c as
a source).

Ignat

> Alex, Ethan, and KUnit folks: What's your preference?