[an error occurred while processing this directive]
[an error occurred while processing this directive]
|
|
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAG_fn=XWr1_Qvzqq3_dUm-3DjpCFxBz7SbYaW8OMZ1BohjVYDA@mail.gmail.com>
Date: Wed, 3 Sep 2025 10:40:55 +0200
From: Alexander Potapenko <glider@...gle.com>
To: Ethan Graham <ethan.w.s.graham@...il.com>
Cc: ethangraham@...gle.com, andreyknvl@...il.com, brendan.higgins@...ux.dev,
davidgow@...gle.com, dvyukov@...gle.com, jannh@...gle.com, elver@...gle.com,
rmoar@...gle.com, shuah@...nel.org, tarasmadan@...gle.com,
kasan-dev@...glegroups.com, kunit-dev@...glegroups.com,
linux-kernel@...r.kernel.org, linux-mm@...ck.org, dhowells@...hat.com,
lukas@...ner.de, ignat@...udflare.com, herbert@...dor.apana.org.au,
davem@...emloft.net, linux-crypto@...r.kernel.org
Subject: Re: [PATCH v2 RFC 2/7] kfuzztest: add user-facing API and data structures
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -112,6 +112,26 @@ ASSERT(__relocate_kernel_end - __relocate_kernel_start <= KEXEC_CONTROL_CODE_MAX
> #else
> #define KEXEC_RELOCATE_KERNEL
> #endif
> +
> +#ifdef CONFIG_KFUZZTEST
> +#define KFUZZTEST_TABLE \
> + . = ALIGN(PAGE_SIZE); \
> + __kfuzztest_targets_start = .; \
> + KEEP(*(.kfuzztest_target)); \
> + __kfuzztest_targets_end = .; \
> + . = ALIGN(PAGE_SIZE); \
> + __kfuzztest_constraints_start = .; \
> + KEEP(*(.kfuzztest_constraint)); \
> + __kfuzztest_constraints_end = .; \
> + . = ALIGN(PAGE_SIZE); \
> + __kfuzztest_annotations_start = .; \
> + KEEP(*(.kfuzztest_annotation)); \
> + __kfuzztest_annotations_end = .;
> +
> +#else /* CONFIG_KFUZZTEST */
> +#define KFUZZTEST_TABLE
> +#endif /* CONFIG_KFUZZTEST */
I think the definition of KFUZZTEST_TABLE should better be in
include/asm-generic/vmlinux.lds.h, so that it can be used by other
architectures.
> + * KFuzzTest receives its input from userspace as a single binary blob. This
> + * format allows for the serialization of complex, pointer-rich C structures
> + * into a flat buffer that can be safely passed into the kernel. This format
> + * requires only a single copy from userspace into a kenrel buffer, and no
Nit: kernel
> + * further kernel allocations. Pointers are patched internally using a "region"
> + * system where each region corresponds to some pointed-to data.
> + *
> + * Regions should be padded to respect alignment constraints of their underlying
> + * types, and should be followed by at least 8 bytes of padding. These padded
> + * regions are poisoned by KFuzzTest to ensure that KASAN catches OOB accesses.
> + *
> + * The format consists of a prefix and three main components:
Nit: s/prefix/header?
> + * 1. An 8-byte header: Contains KFUZZTEST_MAGIC in the first 4 bytes, and the
> + * version number in the subsequent 4 bytes. This ensures backwards
> + * compatibility in the event of future format changes.
> + * 2. A reloc_region_array: Defines the memory layout of the target structure
> + * by partitioning the payload into logical regions. Each logical region
> + * should contain the byte representation of the type that it represents,
> + * including any necessary padding. The region descriptors should be
> + * ordered by offset ascending.
> + * 3. A reloc_table: Provides "linking" instructions that tell the kernel how
> + * to patch pointer fields to point to the correct regions. By design,
> + * the first region (index 0) is passed as input into a FUZZ_TEST.
> + * 4. A Payload: The raw binary data for the structure and its associated
> + * buffers. This should be aligned to the maximum alignment of all
> + * regions to satisfy alignment requirements of the input types, but this
> + * isn't checked by the parser.
Maybe also call it "target structure" here?
> + * For a detailed specification of the binary layout see the full documentation
> + * at: Documentation/dev-tools/kfuzztest.rst
> + */
> +
> +/**
> + * struct reloc_region - single contiguous memory region in the payload
> + *
> + * @offset: The byte offset of this region from the start of the payload, which
> + * should be aligned to the alignment requirements of the region's
> + * underlying type.
> + * @size: The size of this region in bytes.
> + */
> +struct reloc_region {
> + uint32_t offset;
> + uint32_t size;
> +};
> +
> +/**
> + * struct reloc_region_array - array of regions in an input
Nit: newline here for consistency.
> +#define __KFUZZTEST_DEFINE_CONSTRAINT(arg_type, field, val1, val2, tpe) \
> + static struct kfuzztest_constraint __constraint_##arg_type##_##field __section(".kfuzztest_constraint") \
> + __used = { \
> + .input_type = "struct " #arg_type, \
> + .field_name = #field, \
> + .value1 = (uintptr_t)val1, \
> + .value2 = (uintptr_t)val2, \
> + .type = tpe, \
> + }
> +
> +/**
> + * KFUZZTEST_EXPECT_EQ - constrain a field to be equal to a value
> + *
> + * @arg_type: name of the input structure, without the leading "struct ".
> + * @field: some field that is comparable
> + * @val: a value of the same type as @arg_type.@...ld
> + */
> +#define KFUZZTEST_EXPECT_EQ(arg_type, field, val) \
> + do { \
> + if (arg->field != val) \
> + return; \
> + __KFUZZTEST_DEFINE_CONSTRAINT(arg_type, field, val, 0x0, EXPECT_EQ); \
Doesn't the compiler complain about defining __used in the middle of the block?
Maybe move it before the if statement?
> + * KFUZZTEST_EXPECT_NE - constrain a field to be not equal to a value
Nit: you could probably save some space and extract the boilerplate
from KFUZZTEST_EXPECT_XX into a helper macro.
> +config KFUZZTEST
> + bool "KFuzzTest - enable support for internal fuzz targets"
> + depends on DEBUG_FS && DEBUG_KERNEL
Given that you only have the sections defined for x86, you should
probably put something like "depends on X86_64" here.
If you go for it, please mention somewhere that the framework is only
available for x86_64, and add "x86:" to the patch title.
An alternative would be to add KFUZZTEST_TABLE to vmlinux.lds.S for
every architecture.
Powered by blists - more mailing lists