This is a note to let you know that I've just added the patch titled
kunit: Add a macro to wrap a deferred action function
to the 6.7-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
kunit-add-a-macro-to-wrap-a-deferred-action-function.patch
and it can be found in the queue-6.7 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From 56778b49c9a2cbc32c6b0fbd3ba1a9d64192d3af Mon Sep 17 00:00:00 2001
From: David Gow <davidgow@xxxxxxxxxx>
Date: Tue, 28 Nov 2023 15:24:05 +0800
Subject: kunit: Add a macro to wrap a deferred action function
From: David Gow <davidgow@xxxxxxxxxx>
commit 56778b49c9a2cbc32c6b0fbd3ba1a9d64192d3af upstream.
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@xxxxxxxxxx>
Tested-by: Nathan Chancellor <nathan@xxxxxxxxxx>
Acked-by: Daniel Vetter <daniel@xxxxxxxx>
Reviewed-by: Maxime Ripard <mripard@xxxxxxxxxx>
Signed-off-by: David Gow <davidgow@xxxxxxxxxx>
Signed-off-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
Documentation/dev-tools/kunit/usage.rst | 10 +++++++---
include/kunit/resource.h | 21 +++++++++++++++++++++
lib/kunit/kunit-test.c | 5 +----
lib/kunit/test.c | 6 ++++--
4 files changed, 33 insertions(+), 9 deletions(-)
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -651,12 +651,16 @@ For example:
}
Note that, for functions like device_unregister which only accept a single
-pointer-sized argument, it's possible to directly cast that function to
-a ``kunit_action_t`` rather than writing a wrapper function, for example:
+pointer-sized argument, it's possible to automatically generate a wrapper
+with the ``KUNIT_DEFINE_ACTION_WRAPPER()`` macro, for example:
.. code-block:: C
- kunit_add_action(test, (kunit_action_t *)&device_unregister, &dev);
+ KUNIT_DEFINE_ACTION_WRAPPER(device_unregister, device_unregister_wrapper, struct device *);
+ kunit_add_action(test, &device_unregister_wrapper, &dev);
+
+You should do this in preference to manually casting to the ``kunit_action_t`` type,
+as casting function pointers will break Control Flow Integrity (CFI).
``kunit_add_action`` can fail if, for example, the system is out of memory.
You can use ``kunit_add_action_or_reset`` instead which runs the action
--- a/include/kunit/resource.h
+++ b/include/kunit/resource.h
@@ -391,6 +391,27 @@ void kunit_remove_resource(struct kunit
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.
* @action: The function to run on test exit
--- a/lib/kunit/kunit-test.c
+++ b/lib/kunit/kunit-test.c
@@ -538,10 +538,7 @@ static struct kunit_suite kunit_resource
#if IS_BUILTIN(CONFIG_KUNIT_TEST)
/* This avoids a cast warning if kfree() is passed direct to kunit_add_action(). */
-static void kfree_wrapper(void *p)
-{
- kfree(p);
-}
+KUNIT_DEFINE_ACTION_WRAPPER(kfree_wrapper, kfree, const void *);
static void kunit_log_test(struct kunit *test)
{
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -819,6 +819,8 @@ static struct notifier_block kunit_mod_n
};
#endif
+KUNIT_DEFINE_ACTION_WRAPPER(kfree_action_wrapper, kfree, const void *)
+
void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp)
{
void *data;
@@ -828,7 +830,7 @@ void *kunit_kmalloc_array(struct kunit *
if (!data)
return NULL;
- if (kunit_add_action_or_reset(test, (kunit_action_t *)kfree, data) != 0)
+ if (kunit_add_action_or_reset(test, kfree_action_wrapper, data) != 0)
return NULL;
return data;
@@ -840,7 +842,7 @@ void kunit_kfree(struct kunit *test, con
if (!ptr)
return;
- kunit_release_action(test, (kunit_action_t *)kfree, (void *)ptr);
+ kunit_release_action(test, kfree_action_wrapper, (void *)ptr);
}
EXPORT_SYMBOL_GPL(kunit_kfree);
Patches currently in stable-queue which might be from davidgow@xxxxxxxxxx are
queue-6.7/kunit-add-a-macro-to-wrap-a-deferred-action-function.patch
