Re: [RFC 01/10] kmod: add test driver to stress test the module loader

From: Kees Cook
Date: Thu Dec 08 2016 - 15:24:45 EST

On Thu, Dec 8, 2016 at 10:47 AM, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> wrote:
> This adds a new stress test driver for kmod: the kernel module loader.
> The new stress test driver, test_kmod, is only enabled as a module right
> now. It should be possible to load this as built-in and load tests early
> (refer to the force_init_test module parameter), however since a lot of
> test can get a system out of memory fast we leave this disabled for now.
>
> Using a system with 1024 MiB of RAM can *easily* get your kernel
> OOM fast with this test driver.
>
> The test_kmod driver exposes API knobs for us to fine tune simple
> request_module() and get_fs_type() calls. Since these API calls
> only allow each one parameter a test driver for these is rather
> simple. Other factors that can help out test driver though are
> the number of calls we issue and knowing current limitations of
> each. This exposes configuration as much as possible through
> userspace to be able to build tests directly from userspace.
>
> Since it allows multiple misc devices its will eventually (once we
> add a knob to let us create new devices at will) also be possible to
> perform more tests in parallel, provided you have enough memory.
>
> We only enable tests we know work as of right now.
>
> Demo screenshots:
>
> # tools/testing/selftests/kmod/kmod.sh
> kmod_test_0001_driver: OK! - loading kmod test
> kmod_test_0001_driver: OK! - Return value: 256 (MODULE_NOT_FOUND), expected MODULE_NOT_FOUND
> kmod_test_0001_fs: OK! - loading kmod test
> kmod_test_0001_fs: OK! - Return value: -22 (-EINVAL), expected -EINVAL
> kmod_test_0002_driver: OK! - loading kmod test
> kmod_test_0002_driver: OK! - Return value: 256 (MODULE_NOT_FOUND), expected MODULE_NOT_FOUND
> kmod_test_0002_fs: OK! - loading kmod test
> kmod_test_0002_fs: OK! - Return value: -22 (-EINVAL), expected -EINVAL
> kmod_test_0003: OK! - loading kmod test
> kmod_test_0003: OK! - Return value: 0 (SUCCESS), expected SUCCESS
> kmod_test_0004: OK! - loading kmod test
> kmod_test_0004: OK! - Return value: 0 (SUCCESS), expected SUCCESS
> kmod_test_0005: OK! - loading kmod test
> kmod_test_0005: OK! - Return value: 0 (SUCCESS), expected SUCCESS
> kmod_test_0006: OK! - loading kmod test
> kmod_test_0006: OK! - Return value: 0 (SUCCESS), expected SUCCESS
> kmod_test_0005: OK! - loading kmod test
> kmod_test_0005: OK! - Return value: 0 (SUCCESS), expected SUCCESS
> kmod_test_0006: OK! - loading kmod test
> kmod_test_0006: OK! - Return value: 0 (SUCCESS), expected SUCCESS
> Test completed
>
> You can also request for specific tests:
>
> # tools/testing/selftests/kmod/kmod.sh -t 0001
> kmod_test_0001_driver: OK! - loading kmod test
> kmod_test_0001_driver: OK! - Return value: 256 (MODULE_NOT_FOUND), expected MODULE_NOT_FOUND
> kmod_test_0001_fs: OK! - loading kmod test
> kmod_test_0001_fs: OK! - Return value: -22 (-EINVAL), expected -EINVAL
> Test completed
>
> Lastly, the current available number of tests:
>
> # tools/testing/selftests/kmod/kmod.sh --help
> Usage: tools/testing/selftests/kmod/kmod.sh [ -t <4-number-digit> ]
> Valid tests: 0001-0009
>
> 0001 - Simple test - 1 thread for empty string
> 0002 - Simple test - 1 thread for modules/filesystems that do not exist
> 0003 - Simple test - 1 thread for get_fs_type() only
> 0004 - Simple test - 2 threads for get_fs_type() only
> 0005 - multithreaded tests with default setup - request_module() only
> 0006 - multithreaded tests with default setup - get_fs_type() only
> 0007 - multithreaded tests with default setup test request_module() and get_fs_type()
> 0008 - multithreaded - push kmod_concurrent over max_modprobes for request_module()
> 0009 - multithreaded - push kmod_concurrent over max_modprobes for get_fs_type()
>
> The following test cases currently fail, as such they are not currently
> enabled by default:
>
> # tools/testing/selftests/kmod/kmod.sh -t 0007
> # tools/testing/selftests/kmod/kmod.sh -t 0008
> # tools/testing/selftests/kmod/kmod.sh -t 0009
> # tools/testing/selftests/kmod/kmod.sh -t 0010
> # tools/testing/selftests/kmod/kmod.sh -t 0011
>
> To be sure to run them as intended please unload both of the modules:
>
> o test_module
> o xfs
>
> And ensure they are not loaded on your system prior to testing them.
> If you use these paritions for your rootfs you can change the default
> test driver used for get_fs_type() by exporting it into your
> environment. For example of other test defaults you can override
> refer to kmod.sh allow_user_defaults().
>
> Behind the scenes this is how we fine tune at a test case prior to
> hitting a trigger to run it:
>
> cat /sys/devices/virtual/misc/test_kmod0/config
> echo -n "2" > /sys/devices/virtual/misc/test_kmod0/config_test_case
> echo -n "ext4" > /sys/devices/virtual/misc/test_kmod0/config_test_fs
> echo -n "80" > /sys/devices/virtual/misc/test_kmod0/config_num_threads
> cat /sys/devices/virtual/misc/test_kmod0/config
> echo -n "1" > /sys/devices/virtual/misc/test_kmod0/config_num_threads
>
> Finally to trigger:
>
> echo -n "1" > /sys/devices/virtual/misc/test_kmod0/trigger_config
>
> The kmod.sh script uses the above constructs to build differnt test cases.

Typo: different

> A bit of interpretation of the current failures follows, first two
> premises:
>
> a) When request_module() is used userspace figures out an optimized version of
> module order for us. Once it finds the modules it needs, as per depmod
> symbol dep map, it will finit_module() the respective modules which
> are needed for the original request_module() request.
>
> b) We have an optimization in place whereby if a kernel uses
> request_module() on a module already loaded we never bother
> userspace as the module already is loaded. This is all handled by
> kernel/kmod.c.
>
> A few things to consider to help identify root causes of issues:
>
> 0) kmod 19 has a broken heuristic for modules being assumed to be
> built-in to your kernel and will return 0 even though request_module()
> failed. Upgrade to a newer version of kmod.
>
> 1) A get_fs_type() call for "xfs" will request_module() for
> "fs-xfs", not for "xfs". The optimization in kernel described in b)
> fails to catch if we have a lot of consecutive get_fs_type() calls.
> The reason is the optimization in place does not look for aliases. This
> means two consecutive get_fs_type() calls will bump kmod_concurrent, whereas
> request_module() will not.
>
> This one explanation why test case 0009 fails at least once for
> get_fs_type().
>
> 2) If a module fails to load --- for whatever reason (kmod_concurrent
> limit reached, file not yet present due to rootfs switch, out of memory)
> we have a period of time during which module request for the same name
> either with request_module() or get_fs_type() will *also* fail to load
> even if the file for the module is ready.
>
> This explains why *multiple* NULLs are possible on test 0009.
>
> 3) finit_module() consumes quite a bit of memory.

Is this due to reading the module into kernel memory or something else?

> 4) Filesystems typically also have more dependent modules than other
> modules, its important to note though that even though a get_fs_type() call
> does not incur additional kmod_concurrent bumps, since userspace
> loads dependencies it finds it needs via finit_module_fd(), it *will*
> take much more memory to load a module with a lot of dependencies.
>
> Because of 3) and 4) we will easily run into out of memory failures
> with certain tests. For instance test 0006 fails on qemu with 1024 MiB
> of RAM. It panics a box after reaping all userspace processes and still
> not having enough memory to reap.

Are the buffers not released until after all the dependent modules are
loaded? I thought it would load one by one?

> Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx>

This is a great selftest, thanks for working on it!

Notes below...

> ---
> lib/Kconfig.debug | 25 +
> lib/Makefile | 1 +
> lib/test_kmod.c | 1248 +++++++++++++++++++++++++++++++++
> tools/testing/selftests/kmod/Makefile | 11 +
> tools/testing/selftests/kmod/config | 7 +
> tools/testing/selftests/kmod/kmod.sh | 449 ++++++++++++
> 6 files changed, 1741 insertions(+)
> create mode 100644 lib/test_kmod.c
> create mode 100644 tools/testing/selftests/kmod/Makefile
> create mode 100644 tools/testing/selftests/kmod/config
> create mode 100755 tools/testing/selftests/kmod/kmod.sh
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 7446097f72bd..6cad548e0682 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1994,6 +1994,31 @@ config BUG_ON_DATA_CORRUPTION
>
> If unsure, say N.
>
> +config TEST_KMOD
> + tristate "kmod stress tester"
> + default n
> + depends on m
> + select TEST_LKM
> + select XFS_FS
> + select TUN
> + select BTRFS_FS

Since the desired FS can be changed at runtime, maybe these selects
aren't needed?

> + help
> + Test the kernel's module loading mechanism: kmod. kmod implements
> + support to load modules using the Linux kernel's usermode helper.
> + This test provides a series of tests against kmod.
> +
> + Although technically you can either build test_kmod as a module or
> + into the kernel we disallow building it into the kernel since
> + it stress tests request_module() and this will very likely cause
> + some issues by taking over precious threads available from other
> + module load requests, ultimately this could be fatal.
> +
> + To run tests run:
> +
> + tools/testing/selftests/kmod/kmod.sh --help
> +
> + If unsure, say N.
> +
> source "samples/Kconfig"
>
> source "lib/Kconfig.kgdb"
> diff --git a/lib/Makefile b/lib/Makefile
> index d15e235f72ea..3c5a14821e16 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
> obj-$(CONFIG_TEST_PRINTF) += test_printf.o
> obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
> obj-$(CONFIG_TEST_UUID) += test_uuid.o
> +obj-$(CONFIG_TEST_KMOD) += test_kmod.o
>
> ifeq ($(CONFIG_DEBUG_KOBJECT),y)
> CFLAGS_kobject.o += -DDEBUG
> diff --git a/lib/test_kmod.c b/lib/test_kmod.c
> new file mode 100644
> index 000000000000..63fded83b9b6
> --- /dev/null
> +++ b/lib/test_kmod.c
> @@ -0,0 +1,1248 @@
> +/*
> + * kmod stress test driver
> + *
> + * Copyright (C) 2016 Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of copyleft-next (version 0.3.1 or later) as published
> + * at http://copyleft-next.org/.
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +/*
> + * This driver provides an interface to trigger and test the kernel's
> + * module loader through a series of configurations and a few triggers.
> + * To test this driver use the following script as root:
> + *
> + * tools/testing/selftests/kmod/kmod.sh --help
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/kmod.h>
> +#include <linux/printk.h>
> +#include <linux/kthread.h>
> +#include <linux/sched.h>
> +#include <linux/fs.h>
> +#include <linux/miscdevice.h>
> +#include <linux/vmalloc.h>
> +#include <linux/slab.h>
> +#include <linux/device.h>
> +
> +#define TEST_START_NUM_THREADS 50
> +#define TEST_START_DRIVER "test_module"
> +#define TEST_START_TEST_FS "xfs"
> +#define TEST_START_TEST_CASE TEST_KMOD_DRIVER
> +
> +
> +static bool force_init_test = false;
> +module_param(force_init_test, bool_enable_only, 0644);
> +MODULE_PARM_DESC(force_init_test,
> + "Force kicking a test immediatley after driver loads");

Typo: immediately

> +
> +/*
> + * For device allocation / registration
> + */
> +static DEFINE_MUTEX(reg_dev_mutex);
> +static LIST_HEAD(reg_test_devs);
> +
> +/*
> + * num_test_devs actually represents the *next* ID of the next
> + * device we will allow to create.
> + */
> +static int num_test_devs;
> +
> +/**
> + * enum kmod_test_case - linker table test case
> + *
> + * If you add a test case, please be sure to review if you need to se
> + * @need_mod_put for your tests case.
> + *
> + * @TEST_KMOD_DRIVER: stress tests request_module()
> + * @TEST_KMOD_FS_TYPE: stress tests get_fs_type()
> + */
> +enum kmod_test_case {
> + __TEST_KMOD_INVALID = 0,
> +
> + TEST_KMOD_DRIVER,
> + TEST_KMOD_FS_TYPE,
> +
> + __TEST_KMOD_MAX,
> +};
> +
> +struct test_config {
> + char *test_driver;
> + char *test_fs;
> + unsigned int num_threads;
> + enum kmod_test_case test_case;
> + int test_result;
> +};
> +
> +struct kmod_test_device;
> +
> +/**
> + * kmod_test_device_info - thread info
> + *
> + * @ret_sync: return value if request_module() is used, sync request for
> + * @TEST_KMOD_DRIVER
> + * @fs_sync: return value of get_fs_type() for @TEST_KMOD_FS_TYPE
> + * @thread_idx: thread ID
> + * @test_dev: test device test is being performed under
> + * @need_mod_put: Some tests (get_fs_type() is one) requires putting the module
> + * (module_put(fs_sync->owner)) when done, otherwise you will not be able
> + * to unload the respective modules and re-test. We use this to keep
> + * accounting of when we need this and to help out in case we need to
> + * error out and deal with module_put() on error.
> + */
> +struct kmod_test_device_info {
> + int ret_sync;
> + struct file_system_type *fs_sync;
> + struct task_struct *task_sync;
> + unsigned int thread_idx;
> + struct kmod_test_device *test_dev;
> + bool need_mod_put;
> +};
> +
> +/**
> + * kmod_test_device - test device to help test kmod
> + *
> + * @dev_idx: unique ID for test device
> + * @config: configuration for the test
> + * @misc_dev: we use a misc device under the hood
> + * @dev: pointer to misc_dev's own struct device
> + * @config_mutex: protects configuration of test
> + * @trigger_mutex: the test trigger can only be fired once at a time
> + * @thread_lock: protects @done count, and the @info per each thread
> + * @done: number of threads which have completed or failed
> + * @test_is_oom: when we run out of memory, use this to halt moving forward
> + * @kthreads_done: completion used to signal when all work is done
> + * @list: needed to be part of the reg_test_devs
> + * @info: array of info for each thread
> + */
> +struct kmod_test_device {
> + int dev_idx;
> + struct test_config config;
> + struct miscdevice misc_dev;
> + struct device *dev;
> + struct mutex config_mutex;
> + struct mutex trigger_mutex;
> + struct mutex thread_mutex;
> +
> + unsigned int done;
> +
> + bool test_is_oom;
> + struct completion kthreads_done;
> + struct list_head list;
> +
> + struct kmod_test_device_info *info;
> +};
> +
> +static const char *test_case_str(enum kmod_test_case test_case)
> +{
> + switch (test_case) {
> + case TEST_KMOD_DRIVER:
> + return "TEST_KMOD_DRIVER";
> + case TEST_KMOD_FS_TYPE:
> + return "TEST_KMOD_FS_TYPE";
> + default:
> + return "invalid";
> + }
> +}
> +
> +static struct miscdevice *dev_to_misc_dev(struct device *dev)
> +{
> + return dev_get_drvdata(dev);
> +}
> +
> +static struct kmod_test_device *misc_dev_to_test_dev(struct miscdevice *misc_dev)
> +{
> + return container_of(misc_dev, struct kmod_test_device, misc_dev);
> +}
> +
> +static struct kmod_test_device *dev_to_test_dev(struct device *dev)
> +{
> + struct miscdevice *misc_dev;
> +
> + misc_dev = dev_to_misc_dev(dev);
> +
> + return misc_dev_to_test_dev(misc_dev);
> +}
> +
> +/* Must run with thread_mutex held */
> +static void kmod_test_done_check(struct kmod_test_device *test_dev,
> + unsigned int idx)
> +{
> + struct test_config *config = &test_dev->config;
> +
> + test_dev->done++;
> + dev_dbg(test_dev->dev, "Done thread count: %u\n", test_dev->done);
> +
> + if (test_dev->done == config->num_threads) {
> + dev_info(test_dev->dev, "Done: %u threads have all run now\n",
> + test_dev->done);
> + dev_info(test_dev->dev, "Last thread to run: %u\n", idx);
> + complete(&test_dev->kthreads_done);
> + }
> +}
> +
> +static void test_kmod_put_module(struct kmod_test_device_info *info)
> +{
> + struct kmod_test_device *test_dev = info->test_dev;
> + struct test_config *config = &test_dev->config;
> +
> + if (!info->need_mod_put)
> + return;
> +
> + switch (config->test_case) {
> + case TEST_KMOD_DRIVER:
> + break;
> + case TEST_KMOD_FS_TYPE:
> + if (info && info->fs_sync && info->fs_sync->owner)
> + module_put(info->fs_sync->owner);
> + break;
> + default:
> + BUG();
> + }
> +
> + info->need_mod_put = true;
> +}
> +
> +static int run_request(void *data)
> +{
> + struct kmod_test_device_info *info = data;
> + struct kmod_test_device *test_dev = info->test_dev;
> + struct test_config *config = &test_dev->config;
> +
> + switch (config->test_case) {
> + case TEST_KMOD_DRIVER:
> + info->ret_sync = request_module("%s", config->test_driver);
> + break;
> + case TEST_KMOD_FS_TYPE:
> + info->fs_sync = get_fs_type(config->test_fs);
> + info->need_mod_put = true;
> + break;
> + default:
> + /* __trigger_config_run() already checked for test sanity */
> + BUG();
> + return -EINVAL;
> + }
> +
> + dev_dbg(test_dev->dev, "Ran thread %u\n", info->thread_idx);
> +
> + test_kmod_put_module(info);
> +
> + mutex_lock(&test_dev->thread_mutex);
> + info->task_sync = NULL;
> + kmod_test_done_check(test_dev, info->thread_idx);
> + mutex_unlock(&test_dev->thread_mutex);
> +
> + return 0;
> +}
> +
> +static int tally_work_test(struct kmod_test_device_info *info)
> +{
> + struct kmod_test_device *test_dev = info->test_dev;
> + struct test_config *config = &test_dev->config;
> + int err_ret = 0;
> +
> + switch (config->test_case) {
> + case TEST_KMOD_DRIVER:
> + /*
> + * Only capture errors, if one is found that's
> + * enough, for now.
> + */
> + if (info->ret_sync != 0)
> + err_ret = info->ret_sync;
> + dev_info(test_dev->dev,
> + "Sync thread %d return status: %d\n",
> + info->thread_idx, info->ret_sync);
> + break;
> + case TEST_KMOD_FS_TYPE:
> + /* For now we make this simple */
> + if (!info->fs_sync)
> + err_ret = -EINVAL;
> + dev_info(test_dev->dev, "Sync thread %u fs: %s\n",
> + info->thread_idx, info->fs_sync ? config->test_fs :
> + "NULL");
> + break;
> + default:
> + BUG();
> + }
> +
> + return err_ret;
> +}
> +
> +/*
> + * XXX: add result option to display if all errors did not match.
> + * For now we just keep any error code if one was found.
> + *
> + * If this ran it means *all* tasks were created fine and we
> + * are now just collecting results.
> + *
> + * Only propagate errors, do not override with a subsequent sucess case.
> + */
> +static void tally_up_work(struct kmod_test_device *test_dev)
> +{
> + struct test_config *config = &test_dev->config;
> + struct kmod_test_device_info *info;
> + unsigned int idx;
> + int err_ret = 0;
> + int ret = 0;
> +
> + mutex_lock(&test_dev->thread_mutex);
> +
> + dev_info(test_dev->dev, "Results:\n");
> +
> + for (idx=0; idx < config->num_threads; idx++) {
> + info = &test_dev->info[idx];
> + ret = tally_work_test(info);
> + if (ret)
> + err_ret = ret;
> + }
> +
> + /*
> + * Note: request_module() returns 256 for a module not found even
> + * though modprobe itself returns 1.
> + */
> + config->test_result = err_ret;
> +
> + mutex_unlock(&test_dev->thread_mutex);
> +}
> +
> +static int try_one_request(struct kmod_test_device *test_dev, unsigned int idx)
> +{
> + struct kmod_test_device_info *info = &test_dev->info[idx];
> + int fail_ret = -ENOMEM;
> +
> + mutex_lock(&test_dev->thread_mutex);
> +
> + info->thread_idx = idx;
> + info->test_dev = test_dev;
> + info->task_sync = kthread_run(run_request, info, "%s-%u",
> + KBUILD_MODNAME, idx);
> +
> + if (!info->task_sync || IS_ERR(info->task_sync)) {
> + test_dev->test_is_oom = true;
> + dev_err(test_dev->dev, "Setting up thread %u failed\n", idx);
> + info->task_sync = NULL;
> + goto err_out;
> + } else
> + dev_dbg(test_dev->dev, "Kicked off thread %u\n", idx);
> +
> + mutex_unlock(&test_dev->thread_mutex);
> +
> + return 0;
> +
> +err_out:
> + info->ret_sync = fail_ret;
> + mutex_unlock(&test_dev->thread_mutex);
> +
> + return fail_ret;
> +}
> +
> +static void test_dev_kmod_stop_tests(struct kmod_test_device *test_dev)
> +{
> + struct test_config *config = &test_dev->config;
> + struct kmod_test_device_info *info;
> + unsigned int i;
> +
> + dev_info(test_dev->dev, "Ending request_module() tests\n");
> +
> + mutex_lock(&test_dev->thread_mutex);
> +
> + for (i=0; i < config->num_threads; i++) {
> + info = &test_dev->info[i];
> + if (info->task_sync && !IS_ERR(info->task_sync)) {
> + dev_info(test_dev->dev,
> + "Stopping still-running thread %i\n", i);
> + kthread_stop(info->task_sync);
> + }
> +
> + /*
> + * info->task_sync is well protected, it can only be
> + * NULL or a pointer to a struct. If its NULL we either
> + * never ran, or we did and we completed the work. Completed
> + * tasks *always* put the module for us. This is a sanity
> + * check -- just in case.
> + */
> + if (info->task_sync && info->need_mod_put)
> + test_kmod_put_module(info);
> + }
> +
> + mutex_unlock(&test_dev->thread_mutex);
> +}
> +
> +/*
> + * Only wait *iff* we did not run into any errors during all of our thread
> + * set up. If run into any issues we stop threads and just bail out with
> + * an error to the trigger. This also means we don't need any tally work
> + * for any threads which fail.
> + */
> +static int try_requests(struct kmod_test_device *test_dev)
> +{
> + struct test_config *config = &test_dev->config;
> + unsigned int idx;
> + int ret;
> + bool any_error = false;
> +
> + for (idx=0; idx < config->num_threads; idx++) {
> + if (test_dev->test_is_oom) {
> + any_error = true;
> + break;
> + }
> +
> + ret = try_one_request(test_dev, idx);
> + if (ret) {
> + any_error = true;
> + break;
> + }
> + }
> +
> + if (!any_error) {
> + test_dev->test_is_oom = false;
> + dev_info(test_dev->dev,
> + "No errors were found while initializing threads\n");
> + wait_for_completion(&test_dev->kthreads_done);
> + tally_up_work(test_dev);
> + } else {
> + test_dev->test_is_oom = true;
> + dev_info(test_dev->dev,
> + "At least one thread failed to start, stop all work\n");
> + test_dev_kmod_stop_tests(test_dev);
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static int run_test_driver(struct kmod_test_device *test_dev)
> +{
> + struct test_config *config = &test_dev->config;
> +
> + dev_info(test_dev->dev, "Test case: %s (%u)\n",
> + test_case_str(config->test_case),
> + config->test_case);
> + dev_info(test_dev->dev, "Test driver to load: %s\n",
> + config->test_driver);
> + dev_info(test_dev->dev, "Number of threads to run: %u\n",
> + config->num_threads);
> + dev_info(test_dev->dev, "Thread IDs will range from 0 - %u\n",
> + config->num_threads - 1);
> +
> + return try_requests(test_dev);
> +}
> +
> +static int run_test_fs_type(struct kmod_test_device *test_dev)
> +{
> + struct test_config *config = &test_dev->config;
> +
> + dev_info(test_dev->dev, "Test case: %s (%u)\n",
> + test_case_str(config->test_case),
> + config->test_case);
> + dev_info(test_dev->dev, "Test filesystem to load: %s\n",
> + config->test_fs);
> + dev_info(test_dev->dev, "Number of threads to run: %u\n",
> + config->num_threads);
> + dev_info(test_dev->dev, "Thread IDs will range from 0 - %u\n",
> + config->num_threads - 1);
> +
> + return try_requests(test_dev);
> +}
> +
> +static ssize_t config_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct kmod_test_device *test_dev = dev_to_test_dev(dev);
> + struct test_config *config = &test_dev->config;
> + int len = 0;
> +
> + mutex_lock(&test_dev->config_mutex);
> +
> + len += sprintf(buf, "Custom trigger configuration for: %s\n",
> + dev_name(dev));
> +
> + len += sprintf(buf+len, "Number of threads:\t%u\n",
> + config->num_threads);
> +
> + len += sprintf(buf+len, "Test_case:\t%s (%u)\n",
> + test_case_str(config->test_case),
> + config->test_case);
> +
> + if (config->test_driver)
> + len += sprintf(buf+len, "driver:\t%s\n",
> + config->test_driver);
> + else
> + len += sprintf(buf+len, "driver:\tEMTPY\n");
> +
> + if (config->test_fs)
> + len += sprintf(buf+len, "fs:\t%s\n",
> + config->test_fs);
> + else
> + len += sprintf(buf+len, "fs:\tEMTPY\n");

These should all use snprintf...

> +
> +
> + mutex_unlock(&test_dev->config_mutex);
> +
> + return len;
> +}
> +static DEVICE_ATTR_RO(config);
> +
> +/*
> + * This ensures we don't allow kicking threads through if our configuration
> + * is faulty.
> + */
> +static int __trigger_config_run(struct kmod_test_device *test_dev)
> +{
> + struct test_config *config = &test_dev->config;
> +
> + test_dev->done = 0;
> +
> + switch (config->test_case) {
> + case TEST_KMOD_DRIVER:
> + return run_test_driver(test_dev);
> + case TEST_KMOD_FS_TYPE:
> + return run_test_fs_type(test_dev);
> + default:
> + dev_warn(test_dev->dev,
> + "Invalid test case requested: %u\n",
> + config->test_case);
> + return -EINVAL;
> + }
> +}
> +
> +static int trigger_config_run(struct kmod_test_device *test_dev)
> +{
> + struct test_config *config = &test_dev->config;
> + int ret;
> +
> + mutex_lock(&test_dev->trigger_mutex);
> + mutex_lock(&test_dev->config_mutex);
> +
> + ret = __trigger_config_run(test_dev);
> + if (ret < 0)
> + goto out;
> + dev_info(test_dev->dev, "General test result: %d\n",
> + config->test_result);
> +
> + /*
> + * We must return 0 after a trigger even unless something went
> + * wrong with the setup of the test. If the test setup went fine
> + * then userspace must just check the result of config->test_result.
> + * One issue with relying on the return from a call in the kernel
> + * is if the kernel returns a possitive value using this trigger
> + * will not return the value to userspace, it would be lost.
> + *
> + * By not relying on capturing the return value of tests we are using
> + * through the trigger it also us to run tests with set -e and only
> + * fail when something went wrong with the driver upon trigger
> + * requests.
> + */
> + ret = 0;
> +
> +out:
> + mutex_unlock(&test_dev->config_mutex);
> + mutex_unlock(&test_dev->trigger_mutex);
> +
> + return ret;
> +}
> +
> +static ssize_t
> +trigger_config_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct kmod_test_device *test_dev = dev_to_test_dev(dev);
> + int ret;
> +
> + if (test_dev->test_is_oom)
> + return -ENOMEM;
> +
> + /* For all intents and purposes we don't care what userspace
> + * sent this trigger, we care only that we were triggered.
> + * We treat the return value only for caputuring issues with
> + * the test setup. At this point all the test variables should
> + * have been allocated so typically this should never fail.
> + */
> + ret = trigger_config_run(test_dev);
> + if (unlikely(ret < 0))
> + goto out;
> +
> + /*
> + * Note: any return > 0 will be treated as success
> + * and the error value will not be available to userspace.
> + * Do not rely on trying to send to userspace a test value
> + * return value as possitive return errors will be lost.
> + */
> + if (WARN_ON(ret > 0))
> + return -EINVAL;
> +
> + ret = count;
> +out:
> + return ret;
> +}
> +static DEVICE_ATTR_WO(trigger_config);
> +
> +/*
> + * XXX: move to kstrncpy() once merged.
> + *
> + * Users should use kfree_const() when freeing these.
> + */
> +static int __kstrncpy(char **dst, const char *name, size_t count, gfp_t gfp)
> +{
> + *dst = kstrndup(name, count, gfp);
> + if (!*dst)
> + return -ENOSPC;
> + return count;
> +}
> +
> +static int config_copy_test_driver_name(struct test_config *config,
> + const char *name,
> + size_t count)
> +{
> + return __kstrncpy(&config->test_driver, name, count, GFP_KERNEL);
> +}
> +
> +
> +static int config_copy_test_fs(struct test_config *config, const char *name,
> + size_t count)
> +{
> + return __kstrncpy(&config->test_fs, name, count, GFP_KERNEL);
> +}
> +
> +static void __kmod_config_free(struct test_config *config)
> +{
> + if (!config)
> + return;
> +
> + kfree_const(config->test_driver);
> + config->test_driver = NULL;
> +
> + kfree_const(config->test_fs);
> + config->test_driver = NULL;
> +}
> +
> +static void kmod_config_free(struct kmod_test_device *test_dev)
> +{
> + struct test_config *config;
> +
> + if (!test_dev)
> + return;
> +
> + config = &test_dev->config;
> +
> + mutex_lock(&test_dev->config_mutex);
> + __kmod_config_free(config);
> + mutex_unlock(&test_dev->config_mutex);
> +}
> +
> +static ssize_t config_test_driver_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct kmod_test_device *test_dev = dev_to_test_dev(dev);
> + struct test_config *config = &test_dev->config;
> + int copied;
> +
> + mutex_lock(&test_dev->config_mutex);
> +
> + kfree_const(config->test_driver);
> + config->test_driver = NULL;
> +
> + copied = config_copy_test_driver_name(config, buf, count);
> + mutex_unlock(&test_dev->config_mutex);
> +
> + return copied;
> +}
> +
> +static ssize_t config_test_driver_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct kmod_test_device *test_dev = dev_to_test_dev(dev);
> + struct test_config *config = &test_dev->config;
> +
> + mutex_lock(&test_dev->config_mutex);
> + strcpy(buf, config->test_driver);
> + strcat(buf, "\n");

IIUC, the show/store API uses a max size of PAGE_SIZE. If that's
correct, it's possible that this show routine could write past the end
of buf, due to the end newline, etc. Best to use snprintf like you do
below for the other shows.

> + mutex_unlock(&test_dev->config_mutex);
> +
> + return strlen(buf) + 1;
> +}
> +static DEVICE_ATTR(config_test_driver, 0644, config_test_driver_show,
> + config_test_driver_store);
> +
> +static ssize_t config_test_fs_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct kmod_test_device *test_dev = dev_to_test_dev(dev);
> + struct test_config *config = &test_dev->config;
> + int copied;
> +
> + mutex_lock(&test_dev->config_mutex);
> +
> + kfree_const(config->test_fs);
> + config->test_fs = NULL;
> +
> + copied = config_copy_test_fs(config, buf, count);
> + mutex_unlock(&test_dev->config_mutex);
> +
> + return copied;
> +}
> +
> +static ssize_t config_test_fs_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct kmod_test_device *test_dev = dev_to_test_dev(dev);
> + struct test_config *config = &test_dev->config;
> +
> + mutex_lock(&test_dev->config_mutex);
> + strcpy(buf, config->test_fs);
> + strcat(buf, "\n");
> + mutex_unlock(&test_dev->config_mutex);

Same here... (which, btw, could likely use to be a helper function,
the show and store functions here are identical except for test_driver
vs test_fs).

> +
> + return strlen(buf) + 1;
> +}
> +static DEVICE_ATTR(config_test_fs, 0644, config_test_fs_show,
> + config_test_fs_store);
> +
> +static int trigger_config_run_driver(struct kmod_test_device *test_dev,
> + const char *test_driver)
> +{
> + int copied;
> + struct test_config *config = &test_dev->config;
> +
> + mutex_lock(&test_dev->config_mutex);
> +
> + config->test_case = TEST_KMOD_DRIVER;
> +
> + kfree_const(config->test_driver);
> + config->test_driver = NULL;
> +
> + copied = config_copy_test_driver_name(config, test_driver,
> + strlen(test_driver));
> + mutex_unlock(&test_dev->config_mutex);
> +
> + if (copied != strlen(test_driver)) {

Can't these copied tests just check < 0? (i.e. avoid the repeated
strlen which can be fragile.)

> + test_dev->test_is_oom = true;
> + return -EINVAL;
> + }
> +
> + test_dev->test_is_oom = false;
> +
> + return trigger_config_run(test_dev);
> +}
> +
> +static int trigger_config_run_fs(struct kmod_test_device *test_dev,
> + const char *fs_type)
> +{
> + int copied;
> + struct test_config *config = &test_dev->config;
> +
> + mutex_lock(&test_dev->config_mutex);
> + config->test_case = TEST_KMOD_FS_TYPE;
> +
> + kfree_const(config->test_fs);
> + config->test_driver = NULL;
> +
> + copied = config_copy_test_fs(config, fs_type, strlen(fs_type));
> + mutex_unlock(&test_dev->config_mutex);
> +
> + if (copied != strlen(fs_type)) {
> + test_dev->test_is_oom = true;
> + return -EINVAL;
> + }
> +
> + test_dev->test_is_oom = false;
> +
> + return trigger_config_run(test_dev);
> +}

These two functions are almost identical too. Only test_case and the
copy function change...

> +
> +static void free_test_dev_info(struct kmod_test_device *test_dev)
> +{
> + if (test_dev->info) {
> + vfree(test_dev->info);
> + test_dev->info = NULL;
> + }
> +}

vfree() already checks for NULL, you can drop the if.

> +
> +static int kmod_config_sync_info(struct kmod_test_device *test_dev)
> +{
> + struct test_config *config = &test_dev->config;
> +
> + free_test_dev_info(test_dev);
> + test_dev->info = vzalloc(config->num_threads *
> + sizeof(struct kmod_test_device_info));
> + if (!test_dev->info) {
> + dev_err(test_dev->dev, "Cannot alloc test_dev info\n");
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Old kernels may not have this, if you want to port this code to
> + * test it on older kernels.
> + */
> +#ifdef get_kmod_umh_limit
> +static unsigned int kmod_init_test_thread_limit(void)
> +{
> + return get_kmod_umh_limit();
> +}
> +#else
> +static unsigned int kmod_init_test_thread_limit(void)
> +{
> + return TEST_START_NUM_THREADS;
> +}
> +#endif
> +
> +static int __kmod_config_init(struct kmod_test_device *test_dev)
> +{
> + struct test_config *config = &test_dev->config;
> + int ret = -ENOMEM, copied;
> +
> + __kmod_config_free(config);
> +
> + copied = config_copy_test_driver_name(config, TEST_START_DRIVER,
> + strlen(TEST_START_DRIVER));
> + if (copied != strlen(TEST_START_DRIVER))
> + goto err_out;
> +
> + copied = config_copy_test_fs(config, TEST_START_TEST_FS,
> + strlen(TEST_START_TEST_FS));
> + if (copied != strlen(TEST_START_TEST_FS))
> + goto err_out;
> +
> + config->num_threads = kmod_init_test_thread_limit();
> + config->test_result = 0;
> + config->test_case = TEST_START_TEST_CASE;
> +
> + ret = kmod_config_sync_info(test_dev);
> + if (ret)
> + goto err_out;
> +
> + test_dev->test_is_oom = false;
> +
> + return 0;
> +
> +err_out:
> + test_dev->test_is_oom = true;
> + WARN_ON(test_dev->test_is_oom);
> +
> + __kmod_config_free(config);
> +
> + return ret;
> +}
> +
> +static ssize_t reset_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct kmod_test_device *test_dev = dev_to_test_dev(dev);
> + int ret;
> +
> + mutex_lock(&test_dev->trigger_mutex);
> + mutex_lock(&test_dev->config_mutex);
> +
> + ret = __kmod_config_init(test_dev);
> + if (ret < 0) {
> + ret = -ENOMEM;
> + dev_err(dev, "could not alloc settings for config trigger: %d\n",
> + ret);
> + goto out;
> + }
> +
> + dev_info(dev, "reset\n");
> + ret = count;
> +
> +out:
> + mutex_unlock(&test_dev->config_mutex);
> + mutex_unlock(&test_dev->trigger_mutex);
> +
> + return ret;
> +}
> +static DEVICE_ATTR_WO(reset);
> +
> +static int test_dev_config_update_uint_sync(struct kmod_test_device *test_dev,
> + const char *buf, size_t size,
> + unsigned int *config,
> + int (*test_sync)(struct kmod_test_device *test_dev))
> +{
> + int ret;
> + char *end;
> + long new = simple_strtol(buf, &end, 0);
> + unsigned int old_val;
> + if (end == buf || new > UINT_MAX)
> + return -EINVAL;
> +
> + mutex_lock(&test_dev->config_mutex);
> +
> + old_val = *config;
> + *(unsigned int *)config = new;
> +
> + ret = test_sync(test_dev);
> + if (ret) {
> + *(unsigned int *)config = old_val;
> +
> + ret = test_sync(test_dev);
> + WARN_ON(ret);
> +
> + mutex_unlock(&test_dev->config_mutex);
> + return -EINVAL;
> + }
> +
> + mutex_unlock(&test_dev->config_mutex);
> + /* Always return full write size even if we didn't consume all */
> + return size;
> +}
> +
> +static int test_dev_config_update_uint_range(struct kmod_test_device *test_dev,
> + const char *buf, size_t size,
> + unsigned int *config,
> + unsigned int min,
> + unsigned int max)
> +{
> + char *end;
> + long new = simple_strtol(buf, &end, 0);
> + if (end == buf || new < min || new > max || new > UINT_MAX)
> + return -EINVAL;
> +
> + mutex_lock(&test_dev->config_mutex);
> + *(unsigned int *)config = new;

config is already an unsigned int *, why cast?

> + mutex_unlock(&test_dev->config_mutex);
> +
> + /* Always return full write size even if we didn't consume all */
> + return size;
> +}
> +
> +static int test_dev_config_update_int(struct kmod_test_device *test_dev,
> + const char *buf, size_t size,
> + int *config)
> +{
> + char *end;
> + long new = simple_strtol(buf, &end, 0);
> + if (end == buf || new > INT_MAX || new < INT_MIN)
> + return -EINVAL;
> + mutex_lock(&test_dev->config_mutex);
> + *(int *)config = new;

config is already an int *, why cast?

> + mutex_unlock(&test_dev->config_mutex);
> + /* Always return full write size even if we didn't consume all */
> + return size;
> +}
> +
> +static ssize_t test_dev_config_show_int(struct kmod_test_device *test_dev,
> + char *buf,
> + int config)
> +{
> + int val;
> +
> + mutex_lock(&test_dev->config_mutex);
> + val = config;
> + mutex_unlock(&test_dev->config_mutex);
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n", val);
> +}
> +
> +static ssize_t test_dev_config_show_uint(struct kmod_test_device *test_dev,
> + char *buf,
> + unsigned int config)
> +{
> + unsigned int val;
> +
> + mutex_lock(&test_dev->config_mutex);
> + val = config;
> + mutex_unlock(&test_dev->config_mutex);
> +
> + return snprintf(buf, PAGE_SIZE, "%u\n", val);
> +}
> +
> +static ssize_t test_result_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct kmod_test_device *test_dev = dev_to_test_dev(dev);
> + struct test_config *config = &test_dev->config;
> +
> + return test_dev_config_update_int(test_dev, buf, count,
> + &config->test_result);
> +}
> +
> +static ssize_t config_num_threads_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct kmod_test_device *test_dev = dev_to_test_dev(dev);
> + struct test_config *config = &test_dev->config;
> +
> + return test_dev_config_update_uint_sync(test_dev, buf, count,
> + &config->num_threads,
> + kmod_config_sync_info);
> +}
> +
> +static ssize_t config_num_threads_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct kmod_test_device *test_dev = dev_to_test_dev(dev);
> + struct test_config *config = &test_dev->config;
> +
> + return test_dev_config_show_int(test_dev, buf, config->num_threads);
> +}
> +static DEVICE_ATTR(config_num_threads, 0644, config_num_threads_show,
> + config_num_threads_store);
> +
> +static ssize_t config_test_case_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct kmod_test_device *test_dev = dev_to_test_dev(dev);
> + struct test_config *config = &test_dev->config;
> +
> + return test_dev_config_update_uint_range(test_dev, buf, count,
> + &config->test_case,
> + __TEST_KMOD_INVALID + 1,
> + __TEST_KMOD_MAX - 1);
> +}
> +
> +static ssize_t config_test_case_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct kmod_test_device *test_dev = dev_to_test_dev(dev);
> + struct test_config *config = &test_dev->config;
> +
> + return test_dev_config_show_uint(test_dev, buf, config->test_case);
> +}
> +static DEVICE_ATTR(config_test_case, 0644, config_test_case_show,
> + config_test_case_store);
> +
> +static ssize_t test_result_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct kmod_test_device *test_dev = dev_to_test_dev(dev);
> + struct test_config *config = &test_dev->config;
> +
> + return test_dev_config_show_int(test_dev, buf, config->test_result);
> +}
> +static DEVICE_ATTR(test_result, 0644, test_result_show, test_result_store);
> +
> +#define TEST_KMOD_DEV_ATTR(name) &dev_attr_##name.attr
> +
> +static struct attribute *test_dev_attrs[] = {
> + TEST_KMOD_DEV_ATTR(trigger_config),
> + TEST_KMOD_DEV_ATTR(config),
> + TEST_KMOD_DEV_ATTR(reset),
> +
> + TEST_KMOD_DEV_ATTR(config_test_driver),
> + TEST_KMOD_DEV_ATTR(config_test_fs),
> + TEST_KMOD_DEV_ATTR(config_num_threads),
> + TEST_KMOD_DEV_ATTR(config_test_case),
> + TEST_KMOD_DEV_ATTR(test_result),
> +
> + NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(test_dev);
> +
> +static int kmod_config_init(struct kmod_test_device *test_dev)
> +{
> + int ret;
> +
> + mutex_lock(&test_dev->config_mutex);
> + ret = __kmod_config_init(test_dev);
> + mutex_unlock(&test_dev->config_mutex);
> +
> + return ret;
> +}
> +
> +/*
> + * XXX: this could perhaps be made generic already too, but a hunt
> + * for actual users would be needed first. It could be generic
> + * if other test drivers end up using a similar mechanism.
> + */
> +const char *test_dev_get_name(const char *base, int idx, gfp_t gfp)
> +{
> + const char *name_const;
> + char *name;
> +
> + if (!base)
> + return NULL;
> + if (strlen(base) > 30)
> + return NULL;

why?

> + name = kzalloc(1024, gfp);
> + if (!name)
> + return NULL;
> +
> + strncat(name, base, strlen(base));
> + sprintf(name+(strlen(base)), "%d", idx);
> + name_const = kstrdup_const(name, gfp);
> +
> + kfree(name);
> +
> + return name_const;
> +}

What is going on here? Why not just:
return kasprintf(gfp, "%s%d", base, idx);

For all of that code? And kstrdup_const is pointless here since it'll
always just do the dup (as the kmalloc source isn't in rodata).

> +
> +static struct kmod_test_device *alloc_test_dev_kmod(int idx)
> +{
> + int ret;
> + struct kmod_test_device *test_dev;
> + struct miscdevice *misc_dev;
> +
> + test_dev = vzalloc(sizeof(struct kmod_test_device));
> + if (!test_dev) {
> + pr_err("Cannot alloc test_dev\n");
> + goto err_out;
> + }
> +
> + mutex_init(&test_dev->config_mutex);
> + mutex_init(&test_dev->trigger_mutex);
> + mutex_init(&test_dev->thread_mutex);
> +
> + init_completion(&test_dev->kthreads_done);
> +
> + ret = kmod_config_init(test_dev);
> + if (ret < 0) {
> + pr_err("Cannot alloc kmod_config_init()\n");
> + goto err_out_free;
> + }
> +
> + test_dev->dev_idx = idx;
> + misc_dev = &test_dev->misc_dev;
> +
> + misc_dev->minor = MISC_DYNAMIC_MINOR;
> + misc_dev->name = test_dev_get_name("test_kmod", test_dev->dev_idx,
> + GFP_KERNEL);
> + if (!misc_dev->name) {
> + pr_err("Cannot alloc misc_dev->name\n");
> + goto err_out_free_config;
> + }
> + misc_dev->groups = test_dev_groups;
> +
> + return test_dev;
> +
> +err_out_free_config:
> + free_test_dev_info(test_dev);
> + kmod_config_free(test_dev);
> +err_out_free:
> + vfree(test_dev);
> + test_dev = NULL;
> +err_out:
> + return NULL;
> +}
> +
> +static void free_test_dev_kmod(struct kmod_test_device *test_dev)
> +{
> + if (test_dev) {
> + kfree_const(test_dev->misc_dev.name);
> + test_dev->misc_dev.name = NULL;
> + free_test_dev_info(test_dev);
> + kmod_config_free(test_dev);
> + vfree(test_dev);
> + test_dev = NULL;
> + }
> +}
> +
> +static struct kmod_test_device *register_test_dev_kmod(void)
> +{
> + struct kmod_test_device *test_dev = NULL;
> + int ret;
> +
> + mutex_unlock(&reg_dev_mutex);
> +
> + /* int should suffice for number of devices, test for wrap */
> + if (unlikely(num_test_devs + 1) < 0) {
> + pr_err("reached limit of number of test devices\n");
> + goto out;
> + }
> +
> + test_dev = alloc_test_dev_kmod(num_test_devs);
> + if (!test_dev)
> + goto out;
> +
> + ret = misc_register(&test_dev->misc_dev);
> + if (ret) {
> + pr_err("could not register misc device: %d\n", ret);
> + free_test_dev_kmod(test_dev);
> + goto out;
> + }
> +
> + test_dev->dev = test_dev->misc_dev.this_device;
> + list_add_tail(&test_dev->list, &reg_test_devs);
> + dev_info(test_dev->dev, "interface ready\n");
> +
> + num_test_devs++;
> +
> +out:
> + mutex_unlock(&reg_dev_mutex);
> +
> + return test_dev;
> +
> +}
> +
> +static int __init test_kmod_init(void)
> +{
> + struct kmod_test_device *test_dev;
> + int ret;
> +
> + test_dev = register_test_dev_kmod();
> + if (!test_dev) {
> + pr_err("Cannot add first test kmod device\n");
> + return -ENODEV;
> + }
> +
> + /*
> + * With some work we might be able to gracefully enable
> + * testing with this driver built-in, for now this seems
> + * rather risky. For those willing to try have at it,
> + * and enable the below. Good luck! If that works, try
> + * lowering the init level for more fun.
> + */
> + if (force_init_test) {
> + ret = trigger_config_run_driver(test_dev, "tun");
> + if (WARN_ON(ret))
> + return ret;
> + ret = trigger_config_run_fs(test_dev, "btrfs");
> + if (WARN_ON(ret))
> + return ret;
> + }
> +
> + return 0;
> +}
> +late_initcall(test_kmod_init);
> +
> +static
> +void unregister_test_dev_kmod(struct kmod_test_device *test_dev)
> +{
> + mutex_lock(&test_dev->trigger_mutex);
> + mutex_lock(&test_dev->config_mutex);
> +
> + test_dev_kmod_stop_tests(test_dev);
> +
> + dev_info(test_dev->dev, "removing interface\n");
> + misc_deregister(&test_dev->misc_dev);
> +
> + mutex_unlock(&test_dev->config_mutex);
> + mutex_unlock(&test_dev->trigger_mutex);
> +
> + free_test_dev_kmod(test_dev);
> +}
> +
> +static void __exit test_kmod_exit(void)
> +{
> + struct kmod_test_device *test_dev, *tmp;
> +
> + mutex_lock(&reg_dev_mutex);
> + list_for_each_entry_safe(test_dev, tmp, &reg_test_devs, list) {
> + list_del(&test_dev->list);
> + unregister_test_dev_kmod(test_dev);
> + }
> + mutex_unlock(&reg_dev_mutex);
> +}
> +module_exit(test_kmod_exit);
> +
> +MODULE_AUTHOR("Luis R. Rodriguez <mcgrof@xxxxxxxxxx>");
> +MODULE_LICENSE("GPL");
> diff --git a/tools/testing/selftests/kmod/Makefile b/tools/testing/selftests/kmod/Makefile
> new file mode 100644
> index 000000000000..fa2ccc5fb3de
> --- /dev/null
> +++ b/tools/testing/selftests/kmod/Makefile
> @@ -0,0 +1,11 @@
> +# Makefile for kmod loading selftests
> +
> +# No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
> +all:
> +
> +TEST_PROGS := kmod.sh
> +
> +include ../lib.mk
> +
> +# Nothing to clean up.
> +clean:
> diff --git a/tools/testing/selftests/kmod/config b/tools/testing/selftests/kmod/config
> new file mode 100644
> index 000000000000..259f4fd6b5e2
> --- /dev/null
> +++ b/tools/testing/selftests/kmod/config
> @@ -0,0 +1,7 @@
> +CONFIG_TEST_KMOD=m
> +CONFIG_TEST_LKM=m
> +CONFIG_XFS_FS=m
> +
> +# For the module parameter force_init_test is used
> +CONFIG_TUN=m
> +CONFIG_BTRFS_FS=m
> diff --git a/tools/testing/selftests/kmod/kmod.sh b/tools/testing/selftests/kmod/kmod.sh
> new file mode 100755
> index 000000000000..9ea1864d8bae
> --- /dev/null
> +++ b/tools/testing/selftests/kmod/kmod.sh
> @@ -0,0 +1,449 @@
> +#!/bin/bash
> +#
> +# Copyright (C) 2016 Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
> +#
> +# This program is free software; you can redistribute it and/or modify it
> +# under the terms of copyleft-next (version 0.3.1 or later) as published
> +# at http://copyleft-next.org/.
> +
> +# This is a stress test script for kmod, the kernel module loader. It uses
> +# test_kmod which exposes a series of knobs for the API for us so we can
> +# tweak each test in userspace rather than in kernelspace.
> +#
> +# The way kmod works is it uses the kernel's usermode helper API to eventually
> +# call /sbin/modprobe. It has a limit of the number of concurrent calls
> +# possible. The kernel interface to load modules is request_module(), however
> +# mount uses get_fs_type(). Both behave slightly differently, but the
> +# differences are important enough to test each call separately. For this
> +# reason test_kmod starts by providing tests for both calls.
> +#
> +# The test driver test_kmod assumes a series of defaults which you can
> +# override by exporting to your environment prior running this script.
> +# For instance this script assumes you do not have xfs loaded upon boot.
> +# If this is false, export DEFAULT_KMOD_FS="ext4" prior to running this
> +# script if the filesyste module you don't have loaded upon bootup
> +# is ext4 instead. Refer to allow_user_defaults() for a list of user
> +# override variables possible.
> +#
> +# You'll want at least 4096 GiB of RAM to expect to run these tests

4TiB of RAM? I assume this was meant to be 4 GiB not 4096?

> +# without running out of memory on them. For other requirements refer
> +# to test_reqs()
> +
> +set -e
> +
> +TEST_DRIVER="test_kmod"
> +
> +function allow_user_defaults()
> +{
> + if [ -z $DEFAULT_KMOD_DRIVER ]; then
> + DEFAULT_KMOD_DRIVER="test_module"
> + fi
> +
> + if [ -z $DEFAULT_KMOD_FS ]; then
> + DEFAULT_KMOD_FS="xfs"
> + fi
> +
> + if [ -z $PROC_DIR ]; then
> + PROC_DIR="/proc/sys/kernel/"
> + fi
> +
> + if [ -z $MODPROBE_LIMIT ]; then
> + MODPROBE_LIMIT=50
> + fi
> +
> + if [ -z $DIR ]; then
> + DIR="/sys/devices/virtual/misc/${TEST_DRIVER}0/"
> + fi
> +
> + MODPROBE_LIMIT_FILE="${PROC_DIR}/kmod-limit"
> +}
> +
> +test_reqs()
> +{
> + if ! which modprobe 2> /dev/null > /dev/null; then
> + echo "$0: You need modprobe installed"

While not a huge deal, I prefer that error messages end up on stderr,
so adding >&2 to all the failure echos (or providing an err function)
would be nice. (This happens in later places...)

> + exit 1
> + fi
> +
> + if ! which kmod 2> /dev/null > /dev/null; then
> + echo "$0: You need kmod installed"
> + exit 1
> + fi
> +
> + # kmod 19 has a bad bug where it returns 0 when modprobe
> + # gets called *even* if the module was not loaded due to
> + # some bad heuristics. For details see:
> + #
> + # A work around is possible in-kernel but its rather
> + # complex.
> + KMOD_VERSION=$(kmod --version | awk '{print $3}')
> + if [[ $KMOD_VERSION -le 19 ]]; then
> + echo "$0: You need at least kmod 20"
> + echo "kmod <= 19 is buggy, for details see:"
> + echo "http://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/libkmod/libkmod-module.c?id=fd44a98ae2eb5eb32161088954ab21e58e19dfc4";
> + exit 1
> + fi
> +}
> +
> +function load_req_mod()
> +{
> + if [ ! -d $DIR ]; then
> + # Alanis: "Oh isn't it ironic?"
> + modprobe $TEST_DRIVER
> + if [ ! -d $DIR ]; then
> + echo "$0: $DIR not present"
> + echo "You must have the following enabled in your kernel:"
> + cat $PWD/config

I like this (minimum config in the test directory). Are other tests
doing this too?

> + exit 1
> + fi
> + fi
> +}
> +
> +test_finish()
> +{
> + echo "Test completed"
> +}
> +
> +errno_name_to_val()
> +{
> + case "$1" in
> + # kmod calls modprobe and upon of a module not found
> + # modprobe returns just 1... However in the kernel we
> + # *sometimes* see 256...
> + MODULE_NOT_FOUND)
> + echo 256;;
> + SUCCESS)
> + echo 0;;
> + -EPERM)
> + echo -1;;
> + -ENOENT)
> + echo -2;;
> + -EINVAL)
> + echo -22;;
> + -ERR_ANY)
> + echo -123456;;
> + *)
> + echo invalid;;
> + esac
> +}
> +
> +errno_val_to_name()
> + case "$1" in
> + 256)
> + echo MODULE_NOT_FOUND;;
> + 0)
> + echo SUCCESS;;
> + -1)
> + echo -EPERM;;
> + -2)
> + echo -ENOENT;;
> + -22)
> + echo -EINVAL;;
> + -123456)
> + echo -ERR_ANY;;
> + *)
> + echo invalid;;
> + esac
> +
> +config_set_test_case_driver()
> +{
> + if ! echo -n 1 >$DIR/config_test_case; then
> + echo "$0: Unable to set to test case to driver" >&2
> + exit 1
> + fi
> +}
> +
> +config_set_test_case_fs()
> +{
> + if ! echo -n 2 >$DIR/config_test_case; then
> + echo "$0: Unable to set to test case to fs" >&2
> + exit 1
> + fi
> +}
> +
> +config_num_threads()
> +{
> + if ! echo -n $1 >$DIR/config_num_threads; then
> + echo "$0: Unable to set to number of threads" >&2
> + exit 1
> + fi
> +}
> +
> +config_get_modprobe_limit()
> +{
> + if [[ -f ${MODPROBE_LIMIT_FILE} ]] ; then
> + MODPROBE_LIMIT=$(cat $MODPROBE_LIMIT_FILE)
> + fi
> + echo $MODPROBE_LIMIT
> +}
> +
> +config_num_thread_limit_extra()
> +{
> + MODPROBE_LIMIT=$(config_get_modprobe_limit)
> + let EXTRA_LIMIT=$MODPROBE_LIMIT+$1
> + config_num_threads $EXTRA_LIMIT
> +}
> +
> +# For special characters use printf directly,
> +# refer to kmod_test_0001
> +config_set_driver()
> +{
> + if ! echo -n $1 >$DIR/config_test_driver; then
> + echo "$0: Unable to set driver" >&2
> + exit 1
> + fi
> +}
> +
> +config_set_fs()
> +{
> + if ! echo -n $1 >$DIR/config_test_fs; then
> + echo "$0: Unable to set driver" >&2
> + exit 1
> + fi
> +}
> +
> +config_get_driver()
> +{
> + cat $DIR/config_test_driver
> +}
> +
> +config_get_test_result()
> +{
> + cat $DIR/test_result
> +}
> +
> +config_reset()
> +{
> + if ! echo -n "1" >"$DIR"/reset; then
> + echo "$0: reset shuld have worked" >&2
> + exit 1
> + fi
> +}
> +
> +config_show_config()
> +{
> + echo "----------------------------------------------------"
> + cat "$DIR"/config
> + echo "----------------------------------------------------"
> +}
> +
> +config_trigger()
> +{
> + if ! echo -n "1" >"$DIR"/trigger_config 2>/dev/null; then
> + echo "$1: FAIL - loading should have worked"
> + config_show_config
> + exit 1
> + fi
> + echo "$1: OK! - loading kmod test"
> +}
> +
> +config_trigger_want_fail()
> +{
> + if echo "1" > $DIR/trigger_config 2>/dev/null; then
> + echo "$1: FAIL - test case was expected to fail"
> + config_show_config
> + exit 1
> + fi
> + echo "$1: OK! - kmod test case failed as expected"
> +}
> +
> +config_expect_result()
> +{
> + RC=$(config_get_test_result)
> + RC_NAME=$(errno_val_to_name $RC)
> +
> + ERRNO_NAME=$2
> + ERRNO=$(errno_name_to_val $ERRNO_NAME)
> +
> + if [[ $ERRNO_NAME = "-ERR_ANY" ]]; then
> + if [[ $RC -ge 0 ]]; then
> + echo "$1: FAIL, test expects $ERRNO_NAME - got $RC_NAME ($RC)" >&2
> + config_show_config
> + exit 1
> + fi
> + elif [[ $RC != $ERRNO ]]; then
> + echo "$1: FAIL, test expects $ERRNO_NAME ($ERRNO) - got $RC_NAME ($RC)" >&2
> + config_show_config
> + exit 1
> + fi
> + echo "$1: OK! - Return value: $RC ($RC_NAME), expected $ERRNO_NAME"
> +}
> +
> +kmod_defaults_driver()
> +{
> + config_reset
> + modprobe -r $DEFAULT_KMOD_DRIVER
> + config_set_driver $DEFAULT_KMOD_DRIVER
> +}
> +
> +kmod_defaults_fs()
> +{
> + config_reset
> + modprobe -r $DEFAULT_KMOD_FS
> + config_set_fs $DEFAULT_KMOD_FS
> + config_set_test_case_fs
> +}
> +
> +kmod_test_0001_driver()
> +{
> + NAME='\000'
> +
> + kmod_defaults_driver
> + config_num_threads 1
> + printf '\000' >"$DIR"/config_test_driver
> + config_trigger ${FUNCNAME[0]}
> + config_expect_result ${FUNCNAME[0]} MODULE_NOT_FOUND
> +}
> +
> +kmod_test_0001_fs()
> +{
> + NAME='\000'
> +
> + kmod_defaults_fs
> + config_num_threads 1
> + printf '\000' >"$DIR"/config_test_fs
> + config_trigger ${FUNCNAME[0]}
> + config_expect_result ${FUNCNAME[0]} -EINVAL
> +}
> +
> +kmod_test_0001()
> +{
> + kmod_test_0001_driver
> + kmod_test_0001_fs
> +}
> +
> +kmod_test_0002_driver()
> +{
> + NAME="nope-$DEFAULT_KMOD_DRIVER"
> +
> + kmod_defaults_driver
> + config_set_driver $NAME
> + config_num_threads 1
> + config_trigger ${FUNCNAME[0]}
> + config_expect_result ${FUNCNAME[0]} MODULE_NOT_FOUND
> +}
> +
> +kmod_test_0002_fs()
> +{
> + NAME="nope-$DEFAULT_KMOD_FS"
> +
> + kmod_defaults_fs
> + config_set_fs $NAME
> + config_trigger ${FUNCNAME[0]}
> + config_expect_result ${FUNCNAME[0]} -EINVAL
> +}
> +
> +kmod_test_0002()
> +{
> + kmod_test_0002_driver
> + kmod_test_0002_fs
> +}
> +
> +kmod_test_0003()
> +{
> + kmod_defaults_fs
> + config_num_threads 1
> + config_trigger ${FUNCNAME[0]}
> + config_expect_result ${FUNCNAME[0]} SUCCESS
> +}
> +
> +kmod_test_0004()
> +{
> + kmod_defaults_fs
> + config_num_threads 2
> + config_trigger ${FUNCNAME[0]}
> + config_expect_result ${FUNCNAME[0]} SUCCESS
> +}
> +
> +kmod_test_0005()
> +{
> + kmod_defaults_driver
> + config_trigger ${FUNCNAME[0]}
> + config_expect_result ${FUNCNAME[0]} SUCCESS
> +}
> +
> +kmod_test_0006()
> +{
> + kmod_defaults_fs
> + config_trigger ${FUNCNAME[0]}
> + config_expect_result ${FUNCNAME[0]} SUCCESS
> +}
> +
> +kmod_test_0007()
> +{
> + kmod_test_0005
> + kmod_test_0006
> +}
> +
> +kmod_test_0008()
> +{
> + kmod_defaults_driver
> + MODPROBE_LIMIT=$(config_get_modprobe_limit)
> + let EXTRA=$MODPROBE_LIMIT/2
> + config_num_thread_limit_extra $EXTRA
> + config_trigger ${FUNCNAME[0]}
> + config_expect_result ${FUNCNAME[0]} -EINVAL
> +}
> +
> +kmod_test_0009()
> +{
> + kmod_defaults_fs
> + #MODPROBE_LIMIT=$(config_get_modprobe_limit)
> + #let EXTRA=$MODPROBE_LIMIT/3
> + config_num_thread_limit_extra 5
> + config_trigger ${FUNCNAME[0]}
> + config_expect_result ${FUNCNAME[0]} -EINVAL
> +}
> +
> +trap "test_finish" EXIT
> +test_reqs
> +allow_user_defaults
> +load_req_mod
> +
> +usage()
> +{
> + echo "Usage: $0 [ -t <4-number-digit> ]"
> + echo "Valid tests: 0001-0011"
> + echo
> + echo "0001 - Simple test - 1 thread for empty string"
> + echo "0002 - Simple test - 1 thread for modules/filesystems that do not exist"
> + echo "0003 - Simple test - 1 thread for get_fs_type() only"
> + echo "0004 - Simple test - 2 threads for get_fs_type() only"
> + echo "0005 - multithreaded tests with default setup - request_module() only"
> + echo "0006 - multithreaded tests with default setup - get_fs_type() only"
> + echo "0007 - multithreaded tests with default setup test request_module() and get_fs_type()"
> + echo "0008 - multithreaded - push kmod_concurrent over max_modprobes for request_module()"
> + echo "0009 - multithreaded - push kmod_concurrent over max_modprobes for get_fs_type()"
> + exit 1
> +}
> +
> +# You can ask for a specific test:
> +if [[ $# > 0 ]] ; then
> + if [[ $1 != "-t" ]]; then
> + usage
> + fi
> +
> + re='^[0-9]+$'
> + if ! [[ $2 =~ $re ]]; then
> + usage
> + fi
> +
> + RUN_TEST=kmod_test_$2
> + $RUN_TEST
> + exit 0
> +fi
> +
> +# Once tese are enabled please leave them as-is. Write your own test,
> +# we have tons of space.
> +kmod_test_0001
> +kmod_test_0002
> +kmod_test_0003
> +kmod_test_0004
> +kmod_test_0005
> +kmod_test_0006
> +kmod_test_0007
> +
> +#kmod_test_0008
> +#kmod_test_0009

While it's documented in the commit log, I think a short note for each
disabled test should be added here too.

> +
> +exit 0
> --
> 2.10.1
>

-Kees

--
Kees Cook
Nexus Security