Introduce KUnit resource wrappers around platform_driver_register(),
platform_device_alloc(), and platform_device_add() so that test authors
can register platform drivers/devices from their tests and have the
drivers/devices automatically be unregistered when the test is done.

This makes test setup code simpler when a platform driver or platform
device is needed. Add a few test cases at the same time to make sure the
APIs work as intended.

Cc: Brendan Higgins <brendan.higgins@linux.dev>
Reviewed-by: David Gow <davidgow@google.com>
Cc: Rae Moar <rmoar@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 Documentation/dev-tools/kunit/api/index.rst   |   5 +
 .../dev-tools/kunit/api/platformdevice.rst    |  10 +
 include/kunit/platform_device.h               |  20 ++
 lib/kunit/Makefile                            |   4 +-
 lib/kunit/platform-test.c                     | 223 +++++++++++++
 lib/kunit/platform.c                          | 302 ++++++++++++++++++
 6 files changed, 563 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/dev-tools/kunit/api/platformdevice.rst
 create mode 100644 include/kunit/platform_device.h
 create mode 100644 lib/kunit/platform-test.c
 create mode 100644 lib/kunit/platform.c

diff --git a/Documentation/dev-tools/kunit/api/index.rst b/Documentation/dev-tools/kunit/api/index.rst
index 282befa17edf..02b26f5e8750 100644
--- a/Documentation/dev-tools/kunit/api/index.rst
+++ b/Documentation/dev-tools/kunit/api/index.rst
@@ -10,6 +10,7 @@ API Reference
 	resource
 	functionredirection
 	of
+	platformdevice
 
 
 This page documents the KUnit kernel testing API. It is divided into the
@@ -36,3 +37,7 @@ Driver KUnit API
 Documentation/dev-tools/kunit/api/of.rst
 
  - Documents the KUnit device tree (OF) API
+
+Documentation/dev-tools/kunit/api/platformdevice.rst
+
+ - Documents the KUnit platform device API
diff --git a/Documentation/dev-tools/kunit/api/platformdevice.rst b/Documentation/dev-tools/kunit/api/platformdevice.rst
new file mode 100644
index 000000000000..49ddd5729003
--- /dev/null
+++ b/Documentation/dev-tools/kunit/api/platformdevice.rst
@@ -0,0 +1,10 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===================
+Platform Device API
+===================
+
+The KUnit platform device API is used to test platform devices.
+
+.. kernel-doc:: lib/kunit/platform.c
+   :export:
diff --git a/include/kunit/platform_device.h b/include/kunit/platform_device.h
new file mode 100644
index 000000000000..0fc0999d2420
--- /dev/null
+++ b/include/kunit/platform_device.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _KUNIT_PLATFORM_DRIVER_H
+#define _KUNIT_PLATFORM_DRIVER_H
+
+struct kunit;
+struct platform_device;
+struct platform_driver;
+
+struct platform_device *
+kunit_platform_device_alloc(struct kunit *test, const char *name, int id);
+int kunit_platform_device_add(struct kunit *test, struct platform_device *pdev);
+
+int kunit_platform_device_prepare_wait_for_probe(struct kunit *test,
+						 struct platform_device *pdev,
+						 struct completion *x);
+
+int kunit_platform_driver_register(struct kunit *test,
+				   struct platform_driver *drv);
+
+#endif
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 309659a32a78..a980ae62eff6 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -8,7 +8,8 @@ kunit-objs +=				test.o \
 					try-catch.o \
 					executor.o \
 					attributes.o \
-					device.o
+					device.o \
+					platform.o
 
 ifeq ($(CONFIG_KUNIT_DEBUGFS),y)
 kunit-objs +=				debugfs.o
@@ -18,6 +19,7 @@ endif
 obj-y +=				hooks.o
 
 obj-$(CONFIG_KUNIT_TEST) +=		kunit-test.o
+obj-$(CONFIG_KUNIT_TEST) +=		platform-test.o
 
 # string-stream-test compiles built-in only.
 ifeq ($(CONFIG_KUNIT_TEST),y)
diff --git a/lib/kunit/platform-test.c b/lib/kunit/platform-test.c
new file mode 100644
index 000000000000..b4fbedadc55e
--- /dev/null
+++ b/lib/kunit/platform-test.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for platform driver infrastructure.
+ */
+
+#include <linux/platform_device.h>
+
+#include <kunit/platform_device.h>
+#include <kunit/test.h>
+
+/*
+ * Test that kunit_platform_device_alloc() creates a platform device.
+ */
+static void kunit_platform_device_alloc_test(struct kunit *test)
+{
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
+			kunit_platform_device_alloc(test, "kunit-platform", 1));
+}
+
+/*
+ * Test that kunit_platform_device_add() registers a platform device on the
+ * platform bus with the proper name and id.
+ */
+static void kunit_platform_device_add_test(struct kunit *test)
+{
+	struct platform_device *pdev;
+	const char *name = "kunit-platform-add";
+	const int id = -1;
+
+	pdev = kunit_platform_device_alloc(test, name, id);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+
+	KUNIT_EXPECT_EQ(test, 0, kunit_platform_device_add(test, pdev));
+	KUNIT_EXPECT_TRUE(test, dev_is_platform(&pdev->dev));
+	KUNIT_EXPECT_STREQ(test, pdev->name, name);
+	KUNIT_EXPECT_EQ(test, pdev->id, id);
+}
+
+/*
+ * Test that kunit_platform_device_add() called twice with the same device name
+ * and id fails the second time and properly cleans up.
+ */
+static void kunit_platform_device_add_twice_fails_test(struct kunit *test)
+{
+	struct platform_device *pdev;
+	const char *name = "kunit-platform-add-2";
+	const int id = -1;
+
+	pdev = kunit_platform_device_alloc(test, name, id);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+	KUNIT_ASSERT_EQ(test, 0, kunit_platform_device_add(test, pdev));
+
+	pdev = kunit_platform_device_alloc(test, name, id);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+
+	KUNIT_EXPECT_NE(test, 0, kunit_platform_device_add(test, pdev));
+}
+
+static int kunit_platform_device_find_by_name(struct device *dev, const void *data)
+{
+	return strcmp(dev_name(dev), data) == 0;
+}
+
+/*
+ * Test that kunit_platform_device_add() cleans up by removing the platform
+ * device when the test finishes. */
+static void kunit_platform_device_add_cleans_up(struct kunit *test)
+{
+	struct platform_device *pdev;
+	const char *name = "kunit-platform-clean";
+	const int id = -1;
+	struct kunit fake;
+	struct device *dev;
+
+	kunit_init_test(&fake, "kunit_platform_device_add_fake_test", NULL);
+	KUNIT_ASSERT_EQ(test, fake.status, KUNIT_SUCCESS);
+
+	pdev = kunit_platform_device_alloc(&fake, name, id);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+	KUNIT_ASSERT_EQ(test, 0, kunit_platform_device_add(&fake, pdev));
+	dev = bus_find_device(&platform_bus_type, NULL, name,
+			      kunit_platform_device_find_by_name);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
+	put_device(dev);
+
+	/* Remove pdev */
+	kunit_cleanup(&fake);
+
+	/*
+	 * Failing to migrate the kunit_resource would lead to an extra
+	 * put_device() call on the platform device. The best we can do here is
+	 * make sure the device no longer exists on the bus, but if something
+	 * is wrong we'll see a refcount underflow here. We can't test for a
+	 * refcount underflow because the kref matches the lifetime of the
+	 * device which should already be freed and could be used by something
+	 * else.
+	 */
+	dev = bus_find_device(&platform_bus_type, NULL, name,
+			      kunit_platform_device_find_by_name);
+	KUNIT_EXPECT_PTR_EQ(test, NULL, dev);
+	put_device(dev);
+}
+
+/*
+ * Test suite for struct platform_device kunit APIs
+ */
+static struct kunit_case kunit_platform_device_test_cases[] = {
+	KUNIT_CASE(kunit_platform_device_alloc_test),
+	KUNIT_CASE(kunit_platform_device_add_test),
+	KUNIT_CASE(kunit_platform_device_add_twice_fails_test),
+	KUNIT_CASE(kunit_platform_device_add_cleans_up),
+	{}
+};
+
+static struct kunit_suite kunit_platform_device_suite = {
+	.name = "kunit_platform_device",
+	.test_cases = kunit_platform_device_test_cases,
+};
+
+struct kunit_platform_driver_test_context {
+	struct platform_driver pdrv;
+	const char *data;
+};
+
+static const char * const test_data = "test data";
+
+static inline struct kunit_platform_driver_test_context *
+to_test_context(struct platform_device *pdev)
+{
+	return container_of(to_platform_driver(pdev->dev.driver),
+			    struct kunit_platform_driver_test_context,
+			    pdrv);
+}
+
+static int kunit_platform_driver_probe(struct platform_device *pdev)
+{
+	struct kunit_platform_driver_test_context *ctx;
+
+	ctx = to_test_context(pdev);
+	ctx->data = test_data;
+
+	return 0;
+}
+
+/* Test that kunit_platform_driver_register() registers a driver that probes. */
+static void kunit_platform_driver_register_test(struct kunit *test)
+{
+	struct platform_device *pdev;
+	struct kunit_platform_driver_test_context *ctx;
+	DECLARE_COMPLETION_ONSTACK(comp);
+	const char *name = "kunit-platform-register";
+
+	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+	pdev = kunit_platform_device_alloc(test, name, -1);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+	KUNIT_ASSERT_EQ(test, 0, kunit_platform_device_add(test, pdev));
+
+	ctx->pdrv.probe = kunit_platform_driver_probe;
+	ctx->pdrv.driver.name = name;
+	ctx->pdrv.driver.owner = THIS_MODULE;
+
+	KUNIT_ASSERT_EQ(test, 0, kunit_platform_device_prepare_wait_for_probe(test, pdev, &comp));
+
+	KUNIT_EXPECT_EQ(test, 0, kunit_platform_driver_register(test, &ctx->pdrv));
+	KUNIT_EXPECT_NE(test, 0, wait_for_completion_timeout(&comp, 3 * HZ));
+	KUNIT_EXPECT_STREQ(test, ctx->data, test_data);
+}
+
+/*
+ * Test that kunit_platform_device_prepare_wait_for_probe() completes the completion
+ * when the device is already probed.
+ */
+static void kunit_platform_device_prepare_wait_for_probe_completes_when_already_probed(struct kunit *test)
+{
+	struct platform_device *pdev;
+	struct kunit_platform_driver_test_context *ctx;
+	DECLARE_COMPLETION_ONSTACK(comp);
+	const char *name = "kunit-platform-wait";
+
+	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+
+	pdev = kunit_platform_device_alloc(test, name, -1);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+	KUNIT_ASSERT_EQ(test, 0, kunit_platform_device_add(test, pdev));
+
+	ctx->pdrv.probe = kunit_platform_driver_probe;
+	ctx->pdrv.driver.name = name;
+	ctx->pdrv.driver.owner = THIS_MODULE;
+
+	/* Make sure driver has actually probed */
+	KUNIT_ASSERT_EQ(test, 0, kunit_platform_device_prepare_wait_for_probe(test, pdev, &comp));
+	KUNIT_ASSERT_EQ(test, 0, kunit_platform_driver_register(test, &ctx->pdrv));
+	KUNIT_ASSERT_NE(test, 0, wait_for_completion_timeout(&comp, 3 * HZ));
+
+	reinit_completion(&comp);
+	KUNIT_ASSERT_EQ(test, 0, kunit_platform_device_prepare_wait_for_probe(test, pdev, &comp));
+
+	KUNIT_EXPECT_NE(test, 0, wait_for_completion_timeout(&comp, HZ));
+}
+
+static struct kunit_case kunit_platform_driver_test_cases[] = {
+	KUNIT_CASE(kunit_platform_driver_register_test),
+	KUNIT_CASE(kunit_platform_device_prepare_wait_for_probe_completes_when_already_probed),
+	{}
+};
+
+/*
+ * Test suite for struct platform_driver kunit APIs
+ */
+static struct kunit_suite kunit_platform_driver_suite = {
+	.name = "kunit_platform_driver",
+	.test_cases = kunit_platform_driver_test_cases,
+};
+
+kunit_test_suites(
+	&kunit_platform_device_suite,
+	&kunit_platform_driver_suite,
+);
+
+MODULE_LICENSE("GPL");
diff --git a/lib/kunit/platform.c b/lib/kunit/platform.c
new file mode 100644
index 000000000000..ba1b0006dc45
--- /dev/null
+++ b/lib/kunit/platform.c
@@ -0,0 +1,302 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test managed platform driver
+ */
+
+#include <linux/completion.h>
+#include <linux/device/bus.h>
+#include <linux/device/driver.h>
+#include <linux/platform_device.h>
+
+#include <kunit/platform_device.h>
+#include <kunit/resource.h>
+
+struct kunit_platform_device_alloc_params {
+	const char *name;
+	int id;
+};
+
+static int kunit_platform_device_alloc_init(struct kunit_resource *res, void *context)
+{
+	struct kunit_platform_device_alloc_params *params = context;
+	struct platform_device *pdev;
+
+	pdev = platform_device_alloc(params->name, params->id);
+	if (!pdev)
+		return -ENOMEM;
+
+	res->data = pdev;
+
+	return 0;
+}
+
+static void kunit_platform_device_alloc_exit(struct kunit_resource *res)
+{
+	struct platform_device *pdev = res->data;
+
+	platform_device_put(pdev);
+}
+
+/**
+ * kunit_platform_device_alloc() - Allocate a KUnit test managed platform device
+ * @test: test context
+ * @name: device name of platform device to alloc
+ * @id: identifier of platform device to alloc.
+ *
+ * Allocate a test managed platform device. The device is put when the test completes.
+ *
+ * Return: Allocated platform device on success, NULL on failure.
+ */
+struct platform_device *
+kunit_platform_device_alloc(struct kunit *test, const char *name, int id)
+{
+	struct kunit_platform_device_alloc_params params = {
+		.name = name,
+		.id = id,
+	};
+
+	return kunit_alloc_resource(test,
+				    kunit_platform_device_alloc_init,
+				    kunit_platform_device_alloc_exit,
+				    GFP_KERNEL, &params);
+}
+EXPORT_SYMBOL_GPL(kunit_platform_device_alloc);
+
+static void kunit_platform_device_add_exit(struct kunit_resource *res)
+{
+	struct platform_device *pdev = res->data;
+
+	platform_device_unregister(pdev);
+}
+
+static bool
+kunit_platform_device_alloc_match(struct kunit *test,
+				  struct kunit_resource *res, void *match_data)
+{
+	struct platform_device *pdev = match_data;
+
+	return res->data == pdev && res->free != kunit_platform_device_alloc_exit;
+}
+
+KUNIT_DEFINE_ACTION_WRAPPER(platform_device_unregister_wrapper,
+			    platform_device_unregister, struct platform_device *);
+/**
+ * kunit_platform_device_add() - Register a KUnit test managed platform device
+ * @test: test context
+ * @pdev: platform device to add
+ *
+ * Register a test managed platform device. The device is unregistered when the
+ * test completes.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int kunit_platform_device_add(struct kunit *test, struct platform_device *pdev)
+{
+	struct kunit_resource *res;
+	int ret;
+
+	ret = platform_device_add(pdev);
+	if (ret)
+		return ret;
+
+	res = kunit_find_resource(test, kunit_platform_device_alloc_match, pdev);
+	if (res) {
+		/*
+		 * Transfer the reference count of the platform device if it
+		 * was allocated with kunit_platform_device_alloc(). In this
+		 * case, calling platform_device_put() when the test exits from
+		 * kunit_platform_device_alloc_exit() would lead to reference
+		 * count underflow because platform_device_unregister_wrapper()
+		 * calls platform_device_unregister() which also calls
+		 * platform_device_put().
+		 *
+		 * Usually callers transfer the refcount initialized in
+		 * platform_device_alloc() to platform_device_add() by calling
+		 * platform_device_unregister() when platform_device_add()
+		 * succeeds or platform_device_put() when it fails. KUnit has to
+		 * keep this straight by redirecting the free routine for the
+		 * resource to the right function. Luckily this only has to
+		 * account for the success scenario.
+		 */
+		res->free = kunit_platform_device_add_exit;
+		kunit_put_resource(res);
+	} else {
+		ret = kunit_add_action_or_reset(test, platform_device_unregister_wrapper, pdev);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kunit_platform_device_add);
+
+struct kunit_platform_device_probe_nb {
+	struct completion *x;
+	struct device *dev;
+	struct notifier_block nb;
+};
+
+static int kunit_platform_device_probe_notify(struct notifier_block *nb,
+					      unsigned long event, void *data)
+{
+	struct kunit_platform_device_probe_nb *knb;
+	struct device *dev = data;
+
+	knb = container_of(nb, struct kunit_platform_device_probe_nb, nb);
+	if (event != BUS_NOTIFY_BOUND_DRIVER || knb->dev != dev)
+		return NOTIFY_DONE;
+
+	complete(knb->x);
+
+	return NOTIFY_OK;
+}
+
+static void kunit_platform_device_probe_nb_remove(void *nb)
+{
+	bus_unregister_notifier(&platform_bus_type, nb);
+}
+
+/**
+ * kunit_platform_device_prepare_wait_for_probe() - Prepare a completion
+ * variable to wait for a platform device to probe
+ * @test: test context
+ * @pdev: platform device to prepare to wait for probe of
+ * @x: completion variable completed when @dev has probed
+ *
+ * Prepare a completion variable @x to wait for @pdev to probe. Waiting on the
+ * completion forces a preemption, allowing the platform driver to probe.
+ *
+ * Example
+ *
+ * .. code-block:: c
+ *
+ *	static int kunit_platform_driver_probe(struct platform_device *pdev)
+ *	{
+ *		return 0;
+ *	}
+ *
+ *	static void kunit_platform_driver_test(struct kunit *test)
+ *	{
+ *		struct platform_device *pdev;
+ *		struct platform_driver *pdrv;
+ *		DECLARE_COMPLETION_ONSTACK(comp);
+ *
+ *		pdev = kunit_platform_device_alloc(test, "kunit-platform", -1);
+ *		KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+ *		KUNIT_ASSERT_EQ(test, 0, kunit_platform_device_add(test, pdev));
+ *
+ *		pdrv = kunit_kzalloc(test, sizeof(*pdrv), GFP_KERNEL);
+ *		KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdrv);
+ *
+ *		pdrv->probe = kunit_platform_driver_probe;
+ *		pdrv->driver.name = "kunit-platform";
+ *		pdrv->driver.owner = THIS_MODULE;
+ *
+ *		KUNIT_ASSERT_EQ(test, 0, kunit_platform_device_prepare_wait_for_probe(test, pdev, &comp));
+ *		KUNIT_ASSERT_EQ(test, 0, kunit_platform_driver_register(test, pdrv));
+ *
+ *		KUNIT_EXPECT_NE(test, 0, wait_for_completion_timeout(&comp, 3 * HZ));
+ *	}
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int kunit_platform_device_prepare_wait_for_probe(struct kunit *test,
+						 struct platform_device *pdev,
+						 struct completion *x)
+{
+	struct device *dev = &pdev->dev;
+	struct kunit_platform_device_probe_nb *knb;
+	bool bound;
+
+	knb = kunit_kzalloc(test, sizeof(*knb), GFP_KERNEL);
+	if (!knb)
+		return -ENOMEM;
+
+	knb->nb.notifier_call = kunit_platform_device_probe_notify;
+	knb->dev = dev;
+	knb->x = x;
+
+	device_lock(dev);
+	bound = device_is_bound(dev);
+	if (bound) {
+		device_unlock(dev);
+		complete(x);
+		kunit_kfree(test, knb);
+		return 0;
+	}
+
+	bus_register_notifier(&platform_bus_type, &knb->nb);
+	device_unlock(&pdev->dev);
+
+	return kunit_add_action_or_reset(test, kunit_platform_device_probe_nb_remove, &knb->nb);
+}
+EXPORT_SYMBOL_GPL(kunit_platform_device_prepare_wait_for_probe);
+
+KUNIT_DEFINE_ACTION_WRAPPER(platform_driver_unregister_wrapper,
+			    platform_driver_unregister, struct platform_driver *);
+/**
+ * kunit_platform_driver_register() - Register a KUnit test managed platform driver
+ * @test: test context
+ * @drv: platform driver to register
+ *
+ * Register a test managed platform driver. This allows callers to embed the
+ * @drv in a container structure and use container_of() in the probe function
+ * to pass information to KUnit tests.
+ *
+ * Example
+ *
+ * .. code-block:: c
+ *
+ *	struct kunit_test_context {
+ *		struct platform_driver pdrv;
+ *		const char *data;
+ *	};
+ *
+ *	static inline struct kunit_test_context *
+ *	to_test_context(struct platform_device *pdev)
+ *	{
+ *		return container_of(to_platform_driver(pdev->dev.driver),
+ *				    struct kunit_test_context,
+ *				    pdrv);
+ *	}
+ *
+ *	static int kunit_platform_driver_probe(struct platform_device *pdev)
+ *	{
+ *		struct kunit_test_context *ctx;
+ *
+ *		ctx = to_test_context(pdev);
+ *		ctx->data = "test data";
+ *
+ *		return 0;
+ *	}
+ *
+ *	static void kunit_platform_driver_test(struct kunit *test)
+ *	{
+ *		struct kunit_test_context *ctx;
+ *
+ *		ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+ *		KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+ *
+ *		ctx->pdrv.probe = kunit_platform_driver_probe;
+ *		ctx->pdrv.driver.name = "kunit-platform";
+ *		ctx->pdrv.driver.owner = THIS_MODULE;
+ *
+ *		KUNIT_EXPECT_EQ(test, 0, kunit_platform_driver_register(test, &ctx->pdrv));
+ *		<... wait for driver to probe ...>
+ *		KUNIT_EXPECT_STREQ(test, ctx->data, "test data");
+ *	}
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int kunit_platform_driver_register(struct kunit *test,
+				   struct platform_driver *drv)
+{
+	int ret;
+
+	ret = platform_driver_register(drv);
+	if (ret)
+		return ret;
+
+	return kunit_add_action_or_reset(test, platform_driver_unregister_wrapper, drv);
+}
+EXPORT_SYMBOL_GPL(kunit_platform_driver_register);
-- 
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git

On Sat, 6 Jul 2024 at 12:55, Stephen Boyd <sboyd@kernel.org> wrote:
>
> Introduce KUnit resource wrappers around platform_driver_register(),
> platform_device_alloc(), and platform_device_add() so that test authors
> can register platform drivers/devices from their tests and have the
> drivers/devices automatically be unregistered when the test is done.
>
> This makes test setup code simpler when a platform driver or platform
> device is needed. Add a few test cases at the same time to make sure the
> APIs work as intended.
>
> Cc: Brendan Higgins <brendan.higgins@linux.dev>
> Reviewed-by: David Gow <davidgow@google.com>
> Cc: Rae Moar <rmoar@google.com>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Hmm... this is failing under KASAN for me. I'll take a closer look
next week, but in case there's anything super-obvious, here's the
report:

> [15:54:33] BUG: KASAN: slab-use-after-free in kobject_put+0x224/0x280
> [15:54:33] Read of size 1 at addr ffff888002ca484c by task kunit_try_catch/226
> [15:54:33]
> [15:54:33] CPU: 0 PID: 226 Comm: kunit_try_catch Tainted: G      D          N 6.10.0-rc1-g21fbb4cfd3cc #25
> [15:54:33] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [15:54:33] Call Trace:
> [15:54:33]  <TASK>
> [15:54:33]  dump_stack_lvl+0x3f/0x50
> [15:54:33]  print_report+0xcd/0x620
> [15:54:33]  ? kobject_put+0x224/0x280
> [15:54:33]  kasan_report+0xde/0x110
> [15:54:33]  ? kobject_put+0x224/0x280
> [15:54:33]  kobject_put+0x224/0x280
> [15:54:33]  kunit_remove_resource+0x12c/0x1d0
> [15:54:33]  kunit_cleanup+0x8d/0x130
> [15:54:33]  ? __pfx_kunit_try_run_case_cleanup+0x10/0x10
> [15:54:33]  ? __pfx_kunit_generic_run_threadfn_adapter+0x10/0x10
> [15:54:33]  kunit_generic_run_threadfn_adapter+0x7b/0xe0
> [15:54:33]  kthread+0x289/0x360
> [15:54:33]  ? __pfx_kthread+0x10/0x10
> [15:54:33]  ret_from_fork+0x2f/0x70
> [15:54:33]  ? __pfx_kthread+0x10/0x10
> [15:54:33]  ret_from_fork_asm+0x19/0x30
> [15:54:33]  </TASK>
> [15:54:33]
> [15:54:33] Allocated by task 225:
> [15:54:33]  kasan_save_stack+0x33/0x60
> [15:54:33]  kasan_save_track+0x14/0x30
> [15:54:33]  __kasan_kmalloc+0x8f/0xa0
> [15:54:33]  platform_device_alloc+0x26/0x200
> [15:54:33]  kunit_platform_device_alloc_init+0x52/0xc0
> [15:54:33]  __kunit_add_resource+0x98/0x1e0
> [15:54:33]  kunit_platform_device_alloc+0xe7/0x170
> [15:54:33]  kunit_platform_device_add_test+0x91/0x560
> [15:54:33]  kunit_try_run_case+0x1ad/0x490
> [15:54:33]  kunit_generic_run_threadfn_adapter+0x7b/0xe0
> [15:54:33]  kthread+0x289/0x360
> [15:54:33]  ret_from_fork+0x2f/0x70
> [15:54:33]  ret_from_fork_asm+0x19/0x30
> [15:54:33]
> [15:54:33] Freed by task 226:
> [15:54:33]  kasan_save_stack+0x33/0x60
> [15:54:33]  kasan_save_track+0x14/0x30
> [15:54:33]  kasan_save_free_info+0x3b/0x60
> [15:54:33]  __kasan_slab_free+0x101/0x160
> [15:54:33]  kfree+0x94/0x190
> [15:54:33]  device_release+0x9a/0x210
> [15:54:33]  kobject_put+0x150/0x280
> [15:54:33]  kunit_remove_resource+0x12c/0x1d0
> [15:54:33]  kunit_cleanup+0x8d/0x130
> [15:54:33]  kunit_generic_run_threadfn_adapter+0x7b/0xe0
> [15:54:33]  kthread+0x289/0x360
> [15:54:33]  ret_from_fork+0x2f/0x70
> [15:54:33]  ret_from_fork_asm+0x19/0x30
> [15:54:33]
> [15:54:33] The buggy address belongs to the object at ffff888002ca4800
> [15:54:33]  which belongs to the cache kmalloc-1k of size 1024
> [15:54:33] The buggy address is located 76 bytes inside of
> [15:54:33]  freed 1024-byte region [ffff888002ca4800, ffff888002ca4c00)
> [15:54:33]
> [15:54:33] The buggy address belongs to the physical page:
> [15:54:33] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x2ca4
> [15:54:33] head: order:2 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
> [15:54:33] flags: 0x4000000000000040(head|zone=1)
> [15:54:33] page_type: 0xffffefff(slab)
> [15:54:33] raw: 4000000000000040 ffff888001041b00 dead000000000122 0000000000000000
> [15:54:33] raw: 0000000000000000 0000000080080008 00000001ffffefff 0000000000000000
> [15:54:33] head: 4000000000000040 ffff888001041b00 dead000000000122 0000000000000000
> [15:54:33] head: 0000000000000000 0000000080080008 00000001ffffefff 0000000000000000
> [15:54:33] head: 4000000000000002 ffffea00000b2901 ffffffffffffffff 0000000000000000
> [15:54:33] head: 0000000000000004 0000000000000000 00000000ffffffff 0000000000000000
> [15:54:33] page dumped because: kasan: bad access detected
> [15:54:33]
> [15:54:33] Memory state around the buggy address:
> [15:54:33]  ffff888002ca4700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [15:54:33]  ffff888002ca4780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [15:54:33] >ffff888002ca4800: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [15:54:33]                                               ^
> [15:54:33]  ffff888002ca4880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [15:54:33]  ffff888002ca4900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [15:54:33] ==================================================================
> [15:54:33] ------------[ cut here ]------------
> [15:54:33] refcount_t: underflow; use-after-free.
> [15:54:33] WARNING: CPU: 0 PID: 226 at lib/refcount.c:28 refcount_warn_saturate+0xf2/0x150
> [15:54:33] CPU: 0 PID: 226 Comm: kunit_try_catch Tainted: G    B D          N 6.10.0-rc1-g21fbb4cfd3cc #25
> [15:54:33] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [15:54:33] RIP: 0010:refcount_warn_saturate+0xf2/0x150
> [15:54:33] Code: 3b aa cb 00 01 e8 3e c1 b0 ff 0f 0b eb 91 80 3d 2a aa cb 00 00 75 88 48 c7 c7 40 f3 88 ad c6 05 1a aa cb 00 01 e8 1e c1 b0 ff <0f> 0b e9 6e ff ff ff 80 3d 0a aa cb 00 00 0f 85 61 ff ff ff 48 c7
> [15:54:33] RSP: 0018:ffff88800357fe58 EFLAGS: 00000286
> [15:54:33] RAX: 0000000000000000 RBX: ffff888002ca4848 RCX: 1ffffffff5b920d4
> [15:54:33] RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffffffffadc906a0
> [15:54:33] RBP: 0000000000000003 R08: 0000000000000000 R09: fffffbfff5b920d4
> [15:54:33] R10: 0000000000000003 R11: 0000000000000001 R12: ffff88800111fb28
> [15:54:33] R13: ffff88800111fb28 R14: ffff88800111fb28 R15: 0000000000000286
> [15:54:33] FS:  0000000000000000(0000) GS:ffffffffadc6f000(0000) knlGS:0000000000000000
> [15:54:33] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [15:54:33] CR2: dffffc0000000000 CR3: 000000000a25a000 CR4: 00000000000006f0
> [15:54:33] Call Trace:
> [15:54:33]  <TASK>
> [15:54:33]  ? __warn+0xb0/0x170
> [15:54:33]  ? refcount_warn_saturate+0xf2/0x150
> [15:54:33]  ? report_bug+0x298/0x350
> [15:54:33]  ? handle_bug+0x6d/0x90
> [15:54:33]  ? exc_invalid_op+0x17/0x40
> [15:54:33]  ? asm_exc_invalid_op+0x1a/0x20
> [15:54:33]  ? refcount_warn_saturate+0xf2/0x150
> [15:54:33]  ? refcount_warn_saturate+0xf2/0x150
> [15:54:33]  kunit_remove_resource+0x12c/0x1d0
> [15:54:33]  kunit_cleanup+0x8d/0x130
> [15:54:33]  ? __pfx_kunit_try_run_case_cleanup+0x10/0x10
> [15:54:33]  ? __pfx_kunit_generic_run_threadfn_adapter+0x10/0x10
> [15:54:33]  kunit_generic_run_threadfn_adapter+0x7b/0xe0
> [15:54:33]  kthread+0x289/0x360
> [15:54:33]  ? __pfx_kthread+0x10/0x10
> [15:54:33]  ret_from_fork+0x2f/0x70
> [15:54:33]  ? __pfx_kthread+0x10/0x10
> [15:54:33]  ret_from_fork_asm+0x19/0x30
> [15:54:33]  </TASK>
> [15:54:33] ---[ end trace 0000000000000000 ]---
> [15:54:33] [FAILED] kunit_platform_device_add_test

It repros here with:
./tools/testing/kunit/kunit.py run --arch x86_64 --kconfig_add
CONFIG_KASAN=y --kconfig_add CONFIG_KASAN_VMALLOC=y
kunit_platform_driver

Seems to otherwise pass on all of my default configs/archs.

Cheers,
-- David

Quoting David Gow (2024-07-06 01:04:25)
> On Sat, 6 Jul 2024 at 12:55, Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Introduce KUnit resource wrappers around platform_driver_register(),
> > platform_device_alloc(), and platform_device_add() so that test authors
> > can register platform drivers/devices from their tests and have the
> > drivers/devices automatically be unregistered when the test is done.
> >
> > This makes test setup code simpler when a platform driver or platform
> > device is needed. Add a few test cases at the same time to make sure the
> > APIs work as intended.
> >
> > Cc: Brendan Higgins <brendan.higgins@linux.dev>
> > Reviewed-by: David Gow <davidgow@google.com>
> > Cc: Rae Moar <rmoar@google.com>
> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> > ---
> 
> Hmm... this is failing under KASAN for me. I'll take a closer look
> next week, but in case there's anything super-obvious, here's the
> report:

Thanks. I reproduced with your commandline and I'll take a look.

Quoting Stephen Boyd (2024-07-07 11:02:06)
> Quoting David Gow (2024-07-06 01:04:25)
> > On Sat, 6 Jul 2024 at 12:55, Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Introduce KUnit resource wrappers around platform_driver_register(),
> > > platform_device_alloc(), and platform_device_add() so that test authors
> > > can register platform drivers/devices from their tests and have the
> > > drivers/devices automatically be unregistered when the test is done.
> > >
> > > This makes test setup code simpler when a platform driver or platform
> > > device is needed. Add a few test cases at the same time to make sure the
> > > APIs work as intended.
> > >
> > > Cc: Brendan Higgins <brendan.higgins@linux.dev>
> > > Reviewed-by: David Gow <davidgow@google.com>
> > > Cc: Rae Moar <rmoar@google.com>
> > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> > > ---
> > 
> > Hmm... this is failing under KASAN for me. I'll take a closer look
> > next week, but in case there's anything super-obvious, here's the
> > report:
> 
> Thanks. I reproduced with your commandline and I'll take a look.
> 

Seems that I forgot to fold this in when I was testing.

----8<---
diff --git a/lib/kunit/platform.c b/lib/kunit/platform.c
index ba1b0006dc45..0b518de26065 100644
--- a/lib/kunit/platform.c
+++ b/lib/kunit/platform.c
@@ -75,7 +75,7 @@ kunit_platform_device_alloc_match(struct kunit *test,
 {
 	struct platform_device *pdev = match_data;
 
-	return res->data == pdev && res->free != kunit_platform_device_alloc_exit;
+	return res->data == pdev && res->free == kunit_platform_device_alloc_exit;
 }
 
 KUNIT_DEFINE_ACTION_WRAPPER(platform_device_unregister_wrapper,