Re: [PATCH RFC 3/3] drivers/misc: add test driver and selftest for proclocal allocator

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Fri, Jun 21, 2024 at 10:15:01PM +0200, Roman Kagan wrote:
> Introduce a simple driver for functional and stress testing of proclocal
> kernel allocator.  The driver exposes a device node /dev/proclocal-test,
> which allows userland programs to request creation of proclocal areas
> and to obtain their addresses as seen by the kernel, and in addition to
> read and write kernel memory at arbitrary address content (simplified
> /dev/kmem good enough to access proclocal allocations under selftest
> responsibility).
> 
> The driver is not meant for use with production kernels, as it exposes
> internal kernel pointers and data.

Then you MUST taint the kernel and throw up huge warnings whenever it is
loaded, otherwise distros will build this in and end up running it.


> 
> Also add a basic selftest that uses this driver.
> 
> Signed-off-by: Roman Kagan <rkagan@amazon.de>
> ---
>  drivers/misc/Makefile                         |   1 +
>  tools/testing/selftests/proclocal/Makefile    |   6 +
>  drivers/misc/proclocal-test.c                 | 200 ++++++++++++++++++
>  .../selftests/proclocal/proclocal-test.c      | 150 +++++++++++++
>  drivers/misc/Kconfig                          |  15 ++
>  tools/testing/selftests/proclocal/.gitignore  |   1 +
>  6 files changed, 373 insertions(+)
>  create mode 100644 tools/testing/selftests/proclocal/Makefile
>  create mode 100644 drivers/misc/proclocal-test.c
>  create mode 100644 tools/testing/selftests/proclocal/proclocal-test.c
>  create mode 100644 tools/testing/selftests/proclocal/.gitignore
> 
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 153a3f4837e8..33c244cee92d 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -69,3 +69,4 @@ obj-$(CONFIG_TMR_INJECT)	+= xilinx_tmr_inject.o
>  obj-$(CONFIG_TPS6594_ESM)	+= tps6594-esm.o
>  obj-$(CONFIG_TPS6594_PFSM)	+= tps6594-pfsm.o
>  obj-$(CONFIG_NSM)		+= nsm.o
> +obj-$(CONFIG_PROCLOCAL_TEST)	+= proclocal-test.o
> diff --git a/tools/testing/selftests/proclocal/Makefile b/tools/testing/selftests/proclocal/Makefile
> new file mode 100644
> index 000000000000..b93baecee762
> --- /dev/null
> +++ b/tools/testing/selftests/proclocal/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +TEST_GEN_PROGS := proclocal-test
> +CFLAGS += -O2 -g -Wall $(KHDR_INCLUDES)
> +
> +include ../lib.mk
> diff --git a/drivers/misc/proclocal-test.c b/drivers/misc/proclocal-test.c
> new file mode 100644
> index 000000000000..9b3d0ed9b2f9
> --- /dev/null
> +++ b/drivers/misc/proclocal-test.c
> @@ -0,0 +1,200 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (C) 2024 Amazon.com, Inc. or its affiliates. All rights reserved.
> + * Author: Roman Kagan <rkagan@amazon.de>
> + *
> + * test driver for proclocal memory allocator
> + */
> +
> +#include <linux/compat.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/workqueue.h>
> +#include <linux/file.h>
> +#include <linux/secretmem.h>
> +
> +struct proclocal_test_alloc {
> +	u64 size;
> +	u64 ptr;

This structure is not defined properly to cross the user/kernel boundry :(

> +};
> +
> +#define PROCLOCAL_TEST_ALLOC _IOWR('A', 0x10, struct proclocal_test_alloc)

ioctl definitions belong in a .h file for userspace to be able to see
them.

> +
> +#define BOUNCE_BUF_SIZE PAGE_SIZE

Then why not just use PAGE_SIZE everywhere?  Less characters and we all
know what that is.

Stopped reviewing here, sorry.

greg k-h