From 56778b49c9a2cbc32c6b0fbd3ba1a9d64192d3af Mon Sep 17 00:00:00 2001 From: David Gow Date: Tue, 28 Nov 2023 15:24:05 +0800 Subject: kunit: Add a macro to wrap a deferred action function KUnit's deferred action API accepts a void(*)(void *) function pointer which is called when the test is exited. However, we very frequently want to use existing functions which accept a single pointer, but which may not be of type void*. While this is probably dodgy enough to be on the wrong side of the C standard, it's been often used for similar callbacks, and gcc's -Wcast-function-type seems to ignore cases where the only difference is the type of the argument, assuming it's compatible (i.e., they're both pointers to data). However, clang 16 has introduced -Wcast-function-type-strict, which no longer permits any deviation in function pointer type. This seems to be because it'd break CFI, which validates the type of function calls. This rather ruins our attempts to cast functions to defer them, and leaves us with a few options. The one we've chosen is to implement a macro which will generate a wrapper function which accepts a void*, and casts the argument to the appropriate type. For example, if you were trying to wrap: void foo_close(struct foo *handle); you could use: KUNIT_DEFINE_ACTION_WRAPPER(kunit_action_foo_close, foo_close, struct foo *); This would create a new kunit_action_foo_close() function, of type kunit_action_t, which could be passed into kunit_add_action() and similar functions. In addition to defining this macro, update KUnit and its tests to use it. Link: https://github.com/ClangBuiltLinux/linux/issues/1750 Reviewed-by: Nathan Chancellor Tested-by: Nathan Chancellor Acked-by: Daniel Vetter Reviewed-by: Maxime Ripard Signed-off-by: David Gow Signed-off-by: Shuah Khan --- include/kunit/resource.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'include') diff --git a/include/kunit/resource.h b/include/kunit/resource.h index c7383e90f5c9..4ad69a2642a5 100644 --- a/include/kunit/resource.h +++ b/include/kunit/resource.h @@ -390,6 +390,27 @@ void kunit_remove_resource(struct kunit *test, struct kunit_resource *res); /* A 'deferred action' function to be used with kunit_add_action. */ typedef void (kunit_action_t)(void *); +/** + * KUNIT_DEFINE_ACTION_WRAPPER() - Wrap a function for use as a deferred action. + * + * @wrapper: The name of the new wrapper function define. + * @orig: The original function to wrap. + * @arg_type: The type of the argument accepted by @orig. + * + * Defines a wrapper for a function which accepts a single, pointer-sized + * argument. This wrapper can then be passed to kunit_add_action() and + * similar. This should be used in preference to casting a function + * directly to kunit_action_t, as casting function pointers will break + * control flow integrity (CFI), leading to crashes. + */ +#define KUNIT_DEFINE_ACTION_WRAPPER(wrapper, orig, arg_type) \ + static void wrapper(void *in) \ + { \ + arg_type arg = (arg_type)in; \ + orig(arg); \ + } + + /** * kunit_add_action() - Call a function when the test ends. * @test: Test case to associate the action with. -- cgit v1.2.3 From 69dfdce1c5161a37a14720e5f6f62a36e387aa33 Mon Sep 17 00:00:00 2001 From: Rae Moar Date: Wed, 13 Dec 2023 19:44:16 +0000 Subject: kunit: move KUNIT_TABLE out of INIT_DATA Alter the linker section of KUNIT_TABLE to move it out of INIT_DATA and into DATA_DATA. Data for KUnit tests does not need to be in the init section. In order to run tests again after boot the KUnit data cannot be labeled as init data as the kernel could write over it. Add a KUNIT_INIT_TABLE in the next patch for KUnit tests that test init data/functions. Reviewed-by: David Gow Signed-off-by: Rae Moar Signed-off-by: Shuah Khan --- include/asm-generic/vmlinux.lds.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index bae0fe4d499b..1107905d37fc 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -370,7 +370,8 @@ BRANCH_PROFILE() \ TRACE_PRINTKS() \ BPF_RAW_TP() \ - TRACEPOINT_STR() + TRACEPOINT_STR() \ + KUNIT_TABLE() /* * Data section helpers @@ -699,8 +700,7 @@ THERMAL_TABLE(governor) \ EARLYCON_TABLE() \ LSM_TABLE() \ - EARLY_LSM_TABLE() \ - KUNIT_TABLE() + EARLY_LSM_TABLE() #define INIT_TEXT \ *(.init.text .init.text.*) \ -- cgit v1.2.3 From d81f0d7b8b23ec79f80be602ed6129ded27862e8 Mon Sep 17 00:00:00 2001 From: Rae Moar Date: Wed, 13 Dec 2023 19:44:17 +0000 Subject: kunit: add KUNIT_INIT_TABLE to init linker section Add KUNIT_INIT_TABLE to the INIT_DATA linker section. Alter the KUnit macros to create init tests: kunit_test_init_section_suites Update lib/kunit/executor.c to run both the suites in KUNIT_TABLE and KUNIT_INIT_TABLE. Reviewed-by: David Gow Signed-off-by: Rae Moar Signed-off-by: Shuah Khan --- include/asm-generic/vmlinux.lds.h | 9 ++++++++- include/kunit/test.h | 30 +++++++++++++++++++----------- include/linux/module.h | 2 ++ 3 files changed, 29 insertions(+), 12 deletions(-) (limited to 'include') diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index 1107905d37fc..5dd3a61d673d 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -700,7 +700,8 @@ THERMAL_TABLE(governor) \ EARLYCON_TABLE() \ LSM_TABLE() \ - EARLY_LSM_TABLE() + EARLY_LSM_TABLE() \ + KUNIT_INIT_TABLE() #define INIT_TEXT \ *(.init.text .init.text.*) \ @@ -926,6 +927,12 @@ . = ALIGN(8); \ BOUNDED_SECTION_POST_LABEL(.kunit_test_suites, __kunit_suites, _start, _end) +/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */ +#define KUNIT_INIT_TABLE() \ + . = ALIGN(8); \ + BOUNDED_SECTION_POST_LABEL(.kunit_init_test_suites, \ + __kunit_init_suites, _start, _end) + #ifdef CONFIG_BLK_DEV_INITRD #define INIT_RAM_FS \ . = ALIGN(4); \ diff --git a/include/kunit/test.h b/include/kunit/test.h index 20ed9f9275c9..fe79cd736e94 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -337,6 +337,9 @@ void __kunit_test_suites_exit(struct kunit_suite **suites, int num_suites); void kunit_exec_run_tests(struct kunit_suite_set *suite_set, bool builtin); void kunit_exec_list_tests(struct kunit_suite_set *suite_set, bool include_attr); +struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_set, + struct kunit_suite_set suite_set); + #if IS_BUILTIN(CONFIG_KUNIT) int kunit_run_all_tests(void); #else @@ -371,6 +374,11 @@ static inline int kunit_run_all_tests(void) #define kunit_test_suite(suite) kunit_test_suites(&suite) +#define __kunit_init_test_suites(unique_array, ...) \ + static struct kunit_suite *unique_array[] \ + __aligned(sizeof(struct kunit_suite *)) \ + __used __section(".kunit_init_test_suites") = { __VA_ARGS__ } + /** * kunit_test_init_section_suites() - used to register one or more &struct * kunit_suite containing init functions or @@ -378,21 +386,21 @@ static inline int kunit_run_all_tests(void) * * @__suites: a statically allocated list of &struct kunit_suite. * - * This functions identically as kunit_test_suites() except that it suppresses - * modpost warnings for referencing functions marked __init or data marked - * __initdata; this is OK because currently KUnit only runs tests upon boot - * during the init phase or upon loading a module during the init phase. + * This functions similar to kunit_test_suites() except that it compiles the + * list of suites during init phase. + * + * This macro also suffixes the array and suite declarations it makes with + * _probe; so that modpost suppresses warnings about referencing init data + * for symbols named in this manner. * - * NOTE TO KUNIT DEVS: If we ever allow KUnit tests to be run after boot, these - * tests must be excluded. + * Note: these init tests are not able to be run after boot so there is no + * "run" debugfs file generated for these tests. * - * The only thing this macro does that's different from kunit_test_suites is - * that it suffixes the array and suite declarations it makes with _probe; - * modpost suppresses warnings about referencing init data for symbols named in - * this manner. + * Also, do not mark the suite or test case structs with __initdata because + * they will be used after the init phase with debugfs. */ #define kunit_test_init_section_suites(__suites...) \ - __kunit_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe), \ + __kunit_init_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe), \ ##__suites) #define kunit_test_init_section_suite(suite) \ diff --git a/include/linux/module.h b/include/linux/module.h index a98e188cf37b..9cd0009bd050 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -540,6 +540,8 @@ struct module { struct static_call_site *static_call_sites; #endif #if IS_ENABLED(CONFIG_KUNIT) + int num_kunit_init_suites; + struct kunit_suite **kunit_init_suites; int num_kunit_suites; struct kunit_suite **kunit_suites; #endif -- cgit v1.2.3 From 6c4ea2f48de9860217ddfedee081d485dbeea7e8 Mon Sep 17 00:00:00 2001 From: Rae Moar Date: Wed, 13 Dec 2023 19:44:19 +0000 Subject: kunit: add is_init test attribute Add is_init test attribute of type bool. Add to_string, get, and filter methods to lib/kunit/attributes.c. Mark each of the tests in the init section with the is_init=true attribute. Add is_init to the attributes documentation. Reviewed-by: David Gow Signed-off-by: Rae Moar Signed-off-by: Shuah Khan --- include/kunit/test.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/kunit/test.h b/include/kunit/test.h index fe79cd736e94..b163b9984b33 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -253,6 +253,7 @@ struct kunit_suite { struct dentry *debugfs; struct string_stream *log; int suite_init_err; + bool is_init; }; /* Stores an array of suites, end points one past the end */ -- cgit v1.2.3 From d03c720e03bd9bf0b784d80b5d3ede7e2daf3b6e Mon Sep 17 00:00:00 2001 From: "davidgow@google.com" Date: Fri, 15 Dec 2023 15:39:08 +0800 Subject: kunit: Add APIs for managing devices Tests for drivers often require a struct device to pass to other functions. While it's possible to create these with root_device_register(), or to use something like a platform device, this is both a misuse of those APIs, and can be difficult to clean up after, for example, a failed assertion. Add some KUnit-specific functions for registering and unregistering a struct device: - kunit_device_register() - kunit_device_register_with_driver() - kunit_device_unregister() These helpers allocate a on a 'kunit' bus which will either probe the driver passed in (kunit_device_register_with_driver), or will create a stub driver (kunit_device_register) which is cleaned up on test shutdown. Devices are automatically unregistered on test shutdown, but can be manually unregistered earlier with kunit_device_unregister() in order to, for example, test device release code. Reviewed-by: Matti Vaittinen Reviewed-by: Maxime Ripard Signed-off-by: David Gow Reviewed-by: Greg Kroah-Hartman Signed-off-by: Shuah Khan --- include/kunit/device.h | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 include/kunit/device.h (limited to 'include') diff --git a/include/kunit/device.h b/include/kunit/device.h new file mode 100644 index 000000000000..2450110ad64e --- /dev/null +++ b/include/kunit/device.h @@ -0,0 +1,80 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * KUnit basic device implementation + * + * Helpers for creating and managing fake devices for KUnit tests. + * + * Copyright (C) 2023, Google LLC. + * Author: David Gow + */ + +#ifndef _KUNIT_DEVICE_H +#define _KUNIT_DEVICE_H + +#if IS_ENABLED(CONFIG_KUNIT) + +#include + +struct device; +struct device_driver; + +/** + * kunit_driver_create() - Create a struct device_driver attached to the kunit_bus + * @test: The test context object. + * @name: The name to give the created driver. + * + * Creates a struct device_driver attached to the kunit_bus, with the name @name. + * This driver will automatically be cleaned up on test exit. + * + * Return: a stub struct device_driver, managed by KUnit, with the name @name. + */ +struct device_driver *kunit_driver_create(struct kunit *test, const char *name); + +/** + * kunit_device_register() - Create a struct device for use in KUnit tests + * @test: The test context object. + * @name: The name to give the created device. + * + * Creates a struct kunit_device (which is a struct device) with the given name, + * and a corresponding driver. The device and driver will be cleaned up on test + * exit, or when kunit_device_unregister is called. See also + * kunit_device_register_with_driver, if you wish to provide your own + * struct device_driver. + * + * Return: a pointer to a struct device which will be cleaned up when the test + * exits, or an error pointer if the device could not be allocated or registered. + */ +struct device *kunit_device_register(struct kunit *test, const char *name); + +/** + * kunit_device_register_with_driver() - Create a struct device for use in KUnit tests + * @test: The test context object. + * @name: The name to give the created device. + * @drv: The struct device_driver to associate with the device. + * + * Creates a struct kunit_device (which is a struct device) with the given + * name, and driver. The device will be cleaned up on test exit, or when + * kunit_device_unregister is called. See also kunit_device_register, if you + * wish KUnit to create and manage a driver for you. + * + * Return: a pointer to a struct device which will be cleaned up when the test + * exits, or an error pointer if the device could not be allocated or registered. + */ +struct device *kunit_device_register_with_driver(struct kunit *test, + const char *name, + const struct device_driver *drv); + +/** + * kunit_device_unregister() - Unregister a KUnit-managed device + * @test: The test context object which created the device + * @dev: The device. + * + * Unregisters and destroys a struct device which was created with + * kunit_device_register or kunit_device_register_with_driver. If KUnit created + * a driver, cleans it up as well. + */ +void kunit_device_unregister(struct kunit *test, struct device *dev); + +#endif + +#endif -- cgit v1.2.3 From fcbac39b7d5e776bed058281af5d3248b94f1282 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Thu, 21 Dec 2023 10:38:56 +0000 Subject: kunit: Allow passing function pointer to kunit_activate_static_stub() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Swap the arguments to typecheck_fn() in kunit_activate_static_stub() so that real_fn_addr can be either the function itself or a pointer to that function. This is useful to simplify redirecting static functions in a module. Having to pass the actual function meant that it must be exported from the module. Either making the 'static' and EXPORT_SYMBOL*() conditional (which makes the code messy), or change it to always exported (which increases the export namespace and prevents the compiler inlining a trivial stub function in non-test builds). With the original definition of kunit_activate_static_stub() the address of real_fn_addr was passed to typecheck_fn() as the type to be passed. This meant that if real_fn_addr was a pointer-to-function it would resolve to a ** instead of a *, giving an error like this: error: initialization of ‘int (**)(int)’ from incompatible pointer type ‘int (*)(int)’ [-Werror=incompatible-pointer-types] kunit_activate_static_stub(test, add_one_fn_ptr, subtract_one); | ^~~~~~~~~~~~ ./include/linux/typecheck.h:21:25: note: in definition of macro ‘typecheck_fn’ 21 | ({ typeof(type) __tmp = function; \ Swapping the arguments to typecheck_fn makes it take the type of a pointer to the replacement function. Either a function or a pointer to function can be assigned to that. For example: static int some_function(int x) { /* whatever */ } int (* some_function_ptr)(int) = some_function; static int replacement(int x) { /* whatever */ } Then: kunit_activate_static_stub(test, some_function, replacement); yields: typecheck_fn(typeof(&replacement), some_function); and: kunit_activate_static_stub(test, some_function_ptr, replacement); yields: typecheck_fn(typeof(&replacement), some_function_ptr); The two typecheck_fn() then resolve to: int (*__tmp)(int) = some_function; and int (*__tmp)(int) = some_function_ptr; Both of these are valid. In the first case the compiler inserts an implicit '&' to take the address of the supplied function, and in the second case the RHS is already a pointer to the same type. Signed-off-by: Richard Fitzgerald Reviewed-by: Rae Moar Reviewed-by: David Gow Signed-off-by: Shuah Khan --- include/kunit/static_stub.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include') diff --git a/include/kunit/static_stub.h b/include/kunit/static_stub.h index 85315c80b303..bf940322dfc0 100644 --- a/include/kunit/static_stub.h +++ b/include/kunit/static_stub.h @@ -93,7 +93,7 @@ void __kunit_activate_static_stub(struct kunit *test, * The redirection can be disabled again with kunit_deactivate_static_stub(). */ #define kunit_activate_static_stub(test, real_fn_addr, replacement_addr) do { \ - typecheck_fn(typeof(&real_fn_addr), replacement_addr); \ + typecheck_fn(typeof(&replacement_addr), real_fn_addr); \ __kunit_activate_static_stub(test, real_fn_addr, replacement_addr); \ } while (0) -- cgit v1.2.3 From 7ece381aa72d430ee117958abb5bb23e21d72f1d Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Wed, 20 Dec 2023 15:52:56 +0000 Subject: kunit: Protect string comparisons against NULL Add NULL checks to KUNIT_BINARY_STR_ASSERTION() so that it will fail cleanly if either pointer is NULL, instead of causing a NULL pointer dereference in the strcmp(). A test failure could be that a string is unexpectedly NULL. This could be trapped by KUNIT_ASSERT_NOT_NULL() but that would terminate the test at that point. It's preferable that the KUNIT_EXPECT_STR*() macros can handle NULL pointers as a failure. Signed-off-by: Richard Fitzgerald Reviewed-by: David Gow Reviewed-by: Muhammad Usama Anjum Signed-off-by: Shuah Khan --- include/kunit/test.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include') diff --git a/include/kunit/test.h b/include/kunit/test.h index b163b9984b33..c2ce379c329b 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -758,7 +758,7 @@ do { \ .right_text = #right, \ }; \ \ - if (likely(strcmp(__left, __right) op 0)) \ + if (likely((__left) && (__right) && (strcmp(__left, __right) op 0))) \ break; \ \ \ -- cgit v1.2.3