Re: [PATCH v3 2/2] vfs: Add KUnit tests for fdtable

[Jan Kara]

On Thu 27-08-26 04:17:30, Bill Wendling wrote:
> This adds a KUnit test suite for fdtable to verify correct allocation,
> max_fds initialization, and dynamic object size of the fd array under
> __counted_by_ptr when CONFIG_CC_HAS_COUNTED_BY_PTR is enabled.
> 
> Assisted-by: Gemini:3.1-pro-preview
> Signed-off-by: Bill Wendling <morbo@xxxxxxxxxx>

Looks sensible. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
> v2 - Added 'test_dup_fd' testcase for separate allocation path.
> v3 - Add version to the PATCH subject line
> ---
> Cc: Alexander Viro <viro@xxxxxxxxxxxxxxxxxx>
> Cc: Christian Brauner <brauner@xxxxxxxxxx>
> Cc: Jan Kara <jack@xxxxxxx>
> Cc: Kees Cook <kees@xxxxxxxxxx>
> Cc: "Gustavo A. R. Silva" <gustavoars@xxxxxxxxxx>
> Cc: codemender-patching+linux@xxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Cc: linux-fsdevel@xxxxxxxxxxxxxxx
> Cc: linux-hardening@xxxxxxxxxxxxxxx
> ---
>  fs/Kconfig               |  8 +++++
>  fs/file.c                |  4 +++
>  fs/tests/.kunitconfig    |  2 ++
>  fs/tests/fdtable_kunit.c | 72 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 86 insertions(+)
>  create mode 100644 fs/tests/.kunitconfig
>  create mode 100644 fs/tests/fdtable_kunit.c
> 
> diff --git a/fs/Kconfig b/fs/Kconfig
> index e05917adcd60..05b63f7506a7 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -421,4 +421,12 @@ source "fs/unicode/Kconfig"
>  config IO_WQ
>  	bool
>  
> +config FDTABLE_KUNIT_TEST
> +	bool "KUnit test for fdtable" if !KUNIT_ALL_TESTS
> +	depends on KUNIT=y
> +	default KUNIT_ALL_TESTS
> +	help
> +	  This builds the fdtable KUnit tests, which tests various aspects
> +	  of the fdtable structure and allocation.
> +
>  endmenu
> diff --git a/fs/file.c b/fs/file.c
> index 628ca07dc4b1..9c7001b901cf 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -1529,3 +1529,7 @@ int iterate_fd(struct files_struct *files, unsigned n,
>  	return res;
>  }
>  EXPORT_SYMBOL(iterate_fd);
> +
> +#ifdef CONFIG_FDTABLE_KUNIT_TEST
> +#include "tests/fdtable_kunit.c"
> +#endif
> diff --git a/fs/tests/.kunitconfig b/fs/tests/.kunitconfig
> new file mode 100644
> index 000000000000..de67125a9421
> --- /dev/null
> +++ b/fs/tests/.kunitconfig
> @@ -0,0 +1,2 @@
> +CONFIG_KUNIT=y
> +CONFIG_FDTABLE_KUNIT_TEST=y
> diff --git a/fs/tests/fdtable_kunit.c b/fs/tests/fdtable_kunit.c
> new file mode 100644
> index 000000000000..6abd2a8d8f5d
> --- /dev/null
> +++ b/fs/tests/fdtable_kunit.c
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <kunit/test.h>
> +#include <linux/fdtable.h>
> +#include <linux/file.h>
> +
> +static void test_alloc_fdtable(struct kunit *test)
> +{
> +	struct fdtable *fdt;
> +	unsigned int slots = 64;
> +
> +	fdt = alloc_fdtable(slots);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fdt);
> +
> +	/* Check that max_fds is set correctly and is >= slots */
> +	KUNIT_EXPECT_GE(test, fdt->max_fds, slots);
> +
> +	/* Check that fd is allocated */
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fdt->fd);
> +
> +	/*
> +	 * Check dynamic object size of fdt->fd if compiler supports
> +	 * __counted_by_ptr.
> +	 */
> +#ifdef CONFIG_CC_HAS_COUNTED_BY_PTR
> +	KUNIT_EXPECT_EQ(test, __builtin_dynamic_object_size(fdt->fd, 0),
> +			fdt->max_fds * sizeof(struct file *));
> +#endif
> +
> +	__free_fdtable(fdt);
> +}
> +
> +static void test_dup_fd(struct kunit *test)
> +{
> +	struct files_struct *newf;
> +	struct fdtable *fdt;
> +
> +	newf = dup_fd(&init_files, NULL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, newf);
> +
> +	fdt = rcu_dereference_raw(newf->fdt);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fdt);
> +
> +	/* Check that max_fds is set correctly and is >= NR_OPEN_DEFAULT */
> +	KUNIT_EXPECT_GE(test, fdt->max_fds, NR_OPEN_DEFAULT);
> +
> +	/* Check that fd is allocated */
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fdt->fd);
> +
> +	/*
> +	 * Check dynamic object size of fdt->fd if compiler supports
> +	 * __counted_by_ptr.
> +	 */
> +#ifdef CONFIG_CC_HAS_COUNTED_BY_PTR
> +	KUNIT_EXPECT_EQ(test, __builtin_dynamic_object_size(fdt->fd, 0),
> +			fdt->max_fds * sizeof(struct file *));
> +#endif
> +
> +	put_files_struct(newf);
> +}
> +
> +static struct kunit_case fdtable_test_cases[] = {
> +	KUNIT_CASE(test_alloc_fdtable),
> +	KUNIT_CASE(test_dup_fd),
> +	{}
> +};
> +
> +static struct kunit_suite fdtable_test_suite = {
> +	.name = "fdtable",
> +	.test_cases = fdtable_test_cases,
> +};
> +
> +kunit_test_suite(fdtable_test_suite);
> -- 
> 2.55.0.897.gb25b4bd76c-goog
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR