Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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 <nathan@kernel.org>
Tested-by: Nathan Chancellor <nathan@kernel.org>
Acked-by: Daniel Vetter <daniel@ffwll.ch>
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: David Gow <davidgow@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>

authored by

David Gow and committed by
Shuah Khan
56778b49 ceb6a6f0

+33 -9
+7 -3
Documentation/dev-tools/kunit/usage.rst
··· 651 651 } 652 652 653 653 Note that, for functions like device_unregister which only accept a single 654 - pointer-sized argument, it's possible to directly cast that function to 655 - a ``kunit_action_t`` rather than writing a wrapper function, for example: 654 + pointer-sized argument, it's possible to automatically generate a wrapper 655 + with the ``KUNIT_DEFINE_ACTION_WRAPPER()`` macro, for example: 656 656 657 657 .. code-block:: C 658 658 659 - kunit_add_action(test, (kunit_action_t *)&device_unregister, &dev); 659 + KUNIT_DEFINE_ACTION_WRAPPER(device_unregister, device_unregister_wrapper, struct device *); 660 + kunit_add_action(test, &device_unregister_wrapper, &dev); 661 + 662 + You should do this in preference to manually casting to the ``kunit_action_t`` type, 663 + as casting function pointers will break Control Flow Integrity (CFI). 660 664 661 665 ``kunit_add_action`` can fail if, for example, the system is out of memory. 662 666 You can use ``kunit_add_action_or_reset`` instead which runs the action
+21
include/kunit/resource.h
··· 391 391 typedef void (kunit_action_t)(void *); 392 392 393 393 /** 394 + * KUNIT_DEFINE_ACTION_WRAPPER() - Wrap a function for use as a deferred action. 395 + * 396 + * @wrapper: The name of the new wrapper function define. 397 + * @orig: The original function to wrap. 398 + * @arg_type: The type of the argument accepted by @orig. 399 + * 400 + * Defines a wrapper for a function which accepts a single, pointer-sized 401 + * argument. This wrapper can then be passed to kunit_add_action() and 402 + * similar. This should be used in preference to casting a function 403 + * directly to kunit_action_t, as casting function pointers will break 404 + * control flow integrity (CFI), leading to crashes. 405 + */ 406 + #define KUNIT_DEFINE_ACTION_WRAPPER(wrapper, orig, arg_type) \ 407 + static void wrapper(void *in) \ 408 + { \ 409 + arg_type arg = (arg_type)in; \ 410 + orig(arg); \ 411 + } 412 + 413 + 414 + /** 394 415 * kunit_add_action() - Call a function when the test ends. 395 416 * @test: Test case to associate the action with. 396 417 * @action: The function to run on test exit
+1 -4
lib/kunit/kunit-test.c
··· 538 538 #if IS_BUILTIN(CONFIG_KUNIT_TEST) 539 539 540 540 /* This avoids a cast warning if kfree() is passed direct to kunit_add_action(). */ 541 - static void kfree_wrapper(void *p) 542 - { 543 - kfree(p); 544 - } 541 + KUNIT_DEFINE_ACTION_WRAPPER(kfree_wrapper, kfree, const void *); 545 542 546 543 static void kunit_log_test(struct kunit *test) 547 544 {
+4 -2
lib/kunit/test.c
··· 810 810 }; 811 811 #endif 812 812 813 + KUNIT_DEFINE_ACTION_WRAPPER(kfree_action_wrapper, kfree, const void *) 814 + 813 815 void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp) 814 816 { 815 817 void *data; ··· 821 819 if (!data) 822 820 return NULL; 823 821 824 - if (kunit_add_action_or_reset(test, (kunit_action_t *)kfree, data) != 0) 822 + if (kunit_add_action_or_reset(test, kfree_action_wrapper, data) != 0) 825 823 return NULL; 826 824 827 825 return data; ··· 833 831 if (!ptr) 834 832 return; 835 833 836 - kunit_release_action(test, (kunit_action_t *)kfree, (void *)ptr); 834 + kunit_release_action(test, kfree_action_wrapper, (void *)ptr); 837 835 } 838 836 EXPORT_SYMBOL_GPL(kunit_kfree); 839 837