Message ID | 0279beda4660ab1b401164e369c79475633bd067.1608189944.git.viresh.kumar@linaro.org |
---|---|
State | Changes Requested |
Headers | show |
Series | [V2,1/3] tst_module: Add separate declarations of helpers for new tests framework | expand |
Hi! This finit_module02 fails for me: tst_test.c:1261: TINFO: Timeout per run is 0h 05m 00s finit_module02.c:66: TPASS: finit_module(*tc->fd, tc->param, tc->flags): EINVAL (22) finit_module02.c:66: TFAIL: finit_module(*tc->fd, tc->param, tc->flags) expected ENOEXEC: EBADF (9) finit_module02.c:66: TPASS: finit_module(*tc->fd, tc->param, tc->flags): EFAULT (14) finit_module02.c:66: TPASS: finit_module(*tc->fd, tc->param, tc->flags): EINVAL (22) finit_module02.c:66: TPASS: finit_module(*tc->fd, tc->param, tc->flags): EINVAL (22) tst_capability.c:29: TINFO: Dropping CAP_SYS_MODULE(16) finit_module02.c:66: TPASS: finit_module(*tc->fd, tc->param, tc->flags): EPERM (1) tst_capability.c:41: TINFO: Permitting CAP_SYS_MODULE(16) finit_module02.c:59: TPASS: finit_module(*tc->fd, tc->param, tc->flags) passed finit_module02.c:63: TPASS: finit_module(*tc->fd, tc->param, tc->flags): EEXIST (17) finit_module02.c:66: TFAIL: finit_module(*tc->fd, tc->param, tc->flags) expected EBADF: ETXTBSY (26) Linux 5.9.12 Also it looks to me like EBADF is more reasonable error for fd set to -1 also ETXTBSY sounds more reasonable for a file opened for writing. I guess that someone cleaned up the kernel implementation but forget to update the manual pages?
On 17-12-20, 11:09, Cyril Hrubis wrote: > Hi! > This finit_module02 fails for me: > > tst_test.c:1261: TINFO: Timeout per run is 0h 05m 00s > finit_module02.c:66: TPASS: finit_module(*tc->fd, tc->param, tc->flags): EINVAL (22) > finit_module02.c:66: TFAIL: finit_module(*tc->fd, tc->param, tc->flags) expected ENOEXEC: EBADF (9) > finit_module02.c:66: TPASS: finit_module(*tc->fd, tc->param, tc->flags): EFAULT (14) > finit_module02.c:66: TPASS: finit_module(*tc->fd, tc->param, tc->flags): EINVAL (22) > finit_module02.c:66: TPASS: finit_module(*tc->fd, tc->param, tc->flags): EINVAL (22) > tst_capability.c:29: TINFO: Dropping CAP_SYS_MODULE(16) > finit_module02.c:66: TPASS: finit_module(*tc->fd, tc->param, tc->flags): EPERM (1) > tst_capability.c:41: TINFO: Permitting CAP_SYS_MODULE(16) > finit_module02.c:59: TPASS: finit_module(*tc->fd, tc->param, tc->flags) passed > finit_module02.c:63: TPASS: finit_module(*tc->fd, tc->param, tc->flags): EEXIST (17) > finit_module02.c:66: TFAIL: finit_module(*tc->fd, tc->param, tc->flags) expected EBADF: ETXTBSY (26) > > Linux 5.9.12 > > Also it looks to me like EBADF is more reasonable error for fd set to -1 > also ETXTBSY sounds more reasonable for a file opened for writing. > > I guess that someone cleaned up the kernel implementation but forget to > update the manual pages? Merged in v4.6 and I am on 4.4 :( commit a1db74209483a24c861c848b4bb79a4d945ef6fa Author: Mimi Zohar <zohar@linux.vnet.ibm.com> Date: Wed Dec 30 07:35:30 2015 -0500 module: replace copy_module_from_fd with kernel version Replace copy_module_from_fd() with kernel_read_file_from_fd(). Although none of the upstreamed LSMs define a kernel_module_from_file hook, IMA is called, based on policy, to prevent unsigned kernel modules from being loaded by the original kernel module syscall and to measure/appraise signed kernel modules. The security function security_kernel_module_from_file() was called prior to reading a kernel module. Preventing unsigned kernel modules from being loaded by the original kernel module syscall remains on the pre-read kernel_read_file() security hook. Instead of reading the kernel module twice, once for measuring/appraising and again for loading the kernel module, the signature validation is moved to the kernel_post_read_file() security hook. This patch removes the security_kernel_module_from_file() hook and security call. Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Acked-by: Kees Cook <keescook@chromium.org> Acked-by: Luis R. Rodriguez <mcgrof@kernel.org> Cc: Rusty Russell <rusty@rustcorp.com.au>
diff --git a/include/lapi/init_module.h b/include/lapi/init_module.h index 65ff70356c60..9e79e11b8557 100644 --- a/include/lapi/init_module.h +++ b/include/lapi/init_module.h @@ -16,4 +16,20 @@ static inline int init_module(void *module_image, unsigned long len, { return tst_syscall(__NR_init_module, module_image, len, param_values); } + +static inline int finit_module(int fd, const char *param_values, int flags) +{ + return tst_syscall(__NR_finit_module, fd, param_values, flags); +} + +void finit_module_supported_by_kernel(void) +{ + if ((tst_kvercmp(3, 8, 0)) < 0) { + /* Check if the syscall is backported on an older kernel */ + TEST(syscall(__NR_finit_module, 0, "", 0)); + if (TST_RET == -1 && TST_ERR == ENOSYS) + tst_brk(TCONF, "Test not supported on kernel version < v3.8"); + } +} + #endif /* INIT_MODULE_H__ */ diff --git a/runtest/syscalls b/runtest/syscalls index 28174dddd716..961545e73834 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -327,6 +327,9 @@ fgetxattr01 fgetxattr01 fgetxattr02 fgetxattr02 fgetxattr03 fgetxattr03 +finit_module01 finit_module01 +finit_module02 finit_module02 + flistxattr01 flistxattr01 flistxattr02 flistxattr02 flistxattr03 flistxattr03 diff --git a/testcases/kernel/syscalls/finit_module/.gitignore b/testcases/kernel/syscalls/finit_module/.gitignore new file mode 100644 index 000000000000..5fcafdb3736d --- /dev/null +++ b/testcases/kernel/syscalls/finit_module/.gitignore @@ -0,0 +1,3 @@ +/finit_module01 +/finit_module02 +/*.ko diff --git a/testcases/kernel/syscalls/finit_module/Makefile b/testcases/kernel/syscalls/finit_module/Makefile new file mode 100644 index 000000000000..a529695d23cc --- /dev/null +++ b/testcases/kernel/syscalls/finit_module/Makefile @@ -0,0 +1,21 @@ +# SPDX-License-Identifier: GPL-2.0-or-later + +ifneq ($(KERNELRELEASE),) + +obj-m := finit_module.o + +else + +top_srcdir ?= ../../../.. + +include $(top_srcdir)/include/mk/testcases.mk + +REQ_VERSION_MAJOR := 3 +REQ_VERSION_PATCH := 8 + +MAKE_TARGETS := finit_module01 finit_module02 finit_module.ko + +include $(top_srcdir)/include/mk/module.mk +include $(top_srcdir)/include/mk/generic_leaf_target.mk + +endif diff --git a/testcases/kernel/syscalls/finit_module/finit_module.c b/testcases/kernel/syscalls/finit_module/finit_module.c new file mode 100644 index 000000000000..78d03b899a7e --- /dev/null +++ b/testcases/kernel/syscalls/finit_module/finit_module.c @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org> + * + * Dummy test module. + * + * The module accepts a single argument named "status" and it fails + * initialization if the status is set to "invalid". + */ + +#include <linux/module.h> +#include <linux/init.h> +#include <linux/proc_fs.h> +#include <linux/kernel.h> + +static char status[20]; +module_param_string(status, status, 20, 0444); + +static int dummy_init(void) +{ + struct proc_dir_entry *proc_dummy; + + if (!strcmp(status, "invalid")) + return -EINVAL; + + proc_dummy = proc_mkdir("dummy", 0); + return 0; +} +module_init(dummy_init); + +static void dummy_exit(void) +{ + remove_proc_entry("dummy", 0); +} +module_exit(dummy_exit); + +MODULE_LICENSE("GPL"); diff --git a/testcases/kernel/syscalls/finit_module/finit_module01.c b/testcases/kernel/syscalls/finit_module/finit_module01.c new file mode 100644 index 000000000000..c31c0c2dce4c --- /dev/null +++ b/testcases/kernel/syscalls/finit_module/finit_module01.c @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org> + */ + +/*\ + * [DESCRIPTION] + * + * Basic finit_module() tests. + * + * [ALGORITHM] + * + * Inserts a simple module after opening and mmaping the module file. +\*/ + +#include <errno.h> +#include "lapi/init_module.h" +#include "tst_module.h" + +#define MODULE_NAME "finit_module.ko" + +int fd; + +static void setup(void) +{ + finit_module_supported_by_kernel(); + fd = SAFE_OPEN(MODULE_NAME, O_RDONLY|O_CLOEXEC); +} + +static void run(void) +{ + TST_EXP_PASS(finit_module(fd, "status=valid", 0)); + if (!TST_PASS) + return; + + tst_module_unload(MODULE_NAME); +} + +static void cleanup(void) +{ + SAFE_CLOSE(fd); +} + +static struct tst_test test = { + .test_all = run, + .setup = setup, + .cleanup = cleanup, + .needs_root = 1, +}; diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c new file mode 100644 index 000000000000..220c83fd4d1c --- /dev/null +++ b/testcases/kernel/syscalls/finit_module/finit_module02.c @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org> + */ + +/*\ + * [DESCRIPTION] + * + * Basic finit_module() failure tests. + * + * [ALGORITHM] + * + * Tests various failure scenarios for finit_module(). +\*/ + +#include <linux/capability.h> +#include <errno.h> +#include "lapi/init_module.h" +#include "tst_module.h" +#include "tst_capability.h" + +#define MODULE_NAME "finit_module.ko" + +static int fd, fd_zero, fd_invalid = -1; + +static struct tst_cap cap_req = TST_CAP(TST_CAP_REQ, CAP_SYS_MODULE); +static struct tst_cap cap_drop = TST_CAP(TST_CAP_DROP, CAP_SYS_MODULE); + +static struct tcase { + const char *name; + int *fd; + const char *param; + int open_flags; + int flags; + int cap; + int exp_errno; +} tcases[] = { + {"zero-fd", &fd_zero, "", O_RDONLY | O_CLOEXEC, 0, 0, EINVAL}, + {"invalid-fd", &fd_invalid, "", O_RDONLY | O_CLOEXEC, 0, 0, ENOEXEC}, + {"null-param", &fd, NULL, O_RDONLY | O_CLOEXEC, 0, 0, EFAULT}, + {"invalid-param", &fd, "status=invalid", O_RDONLY | O_CLOEXEC, 0, 0, EINVAL}, + {"invalid-flags", &fd, "", O_RDONLY | O_CLOEXEC, -1, 0, EINVAL}, + {"no-perm", &fd, "", O_RDONLY | O_CLOEXEC, 0, 1, EPERM}, + {"module-exists", &fd, "", O_RDONLY | O_CLOEXEC, 0, 0, EEXIST}, + {"file-not-readable", &fd, "", O_WRONLY | O_CLOEXEC, 0, 0, EBADF}, +}; + +static void run(unsigned int n) +{ + struct tcase *tc = &tcases[n]; + + fd = SAFE_OPEN(MODULE_NAME, tc->open_flags); + + if (tc->cap) + tst_cap_action(&cap_drop); + + /* Insert module twice */ + if (tc->exp_errno == EEXIST) { + TST_EXP_PASS(finit_module(*tc->fd, tc->param, tc->flags)); + if (!TST_PASS) + goto out; + + TST_EXP_FAIL(finit_module(*tc->fd, tc->param, tc->flags), tc->exp_errno); + tst_module_unload(MODULE_NAME); + } else { + TST_EXP_FAIL(finit_module(*tc->fd, tc->param, tc->flags), tc->exp_errno); + } + + if (!TST_PASS && !TST_RET) + tst_module_unload(MODULE_NAME); + +out: + if (tc->cap) + tst_cap_action(&cap_req); + + SAFE_CLOSE(fd); +} + +static struct tst_test test = { + .test = run, + .tcnt = ARRAY_SIZE(tcases), + .setup = finit_module_supported_by_kernel, + .needs_root = 1, +};
This patch adds success and failure tests for finit_module() syscall. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- V2: - Remove unwanted header - Use TST_EXP_*() macros - Rename module. include/lapi/init_module.h | 16 ++++ runtest/syscalls | 3 + .../kernel/syscalls/finit_module/.gitignore | 3 + .../kernel/syscalls/finit_module/Makefile | 21 +++++ .../syscalls/finit_module/finit_module.c | 37 ++++++++ .../syscalls/finit_module/finit_module01.c | 49 +++++++++++ .../syscalls/finit_module/finit_module02.c | 84 +++++++++++++++++++ 7 files changed, 213 insertions(+) create mode 100644 testcases/kernel/syscalls/finit_module/.gitignore create mode 100644 testcases/kernel/syscalls/finit_module/Makefile create mode 100644 testcases/kernel/syscalls/finit_module/finit_module.c create mode 100644 testcases/kernel/syscalls/finit_module/finit_module01.c create mode 100644 testcases/kernel/syscalls/finit_module/finit_module02.c