2010-02-15 06:15:58

by Mike Frysinger

[permalink] [raw]
Subject: [PATCH] tracehook: add some self tests

At the moment, we stress test the asm/syscall.h functions.

Signed-off-by: Mike Frysinger <[email protected]>
---
arch/Kconfig | 2 +
kernel/Makefile | 1 +
kernel/tracehooktest.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++
lib/Kconfig.debug | 15 +++++
4 files changed, 152 insertions(+), 0 deletions(-)
create mode 100644 kernel/tracehooktest.c

diff --git a/arch/Kconfig b/arch/Kconfig
index 7f418bb..9c2add3 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -105,6 +105,8 @@ config HAVE_KRETPROBES
# TIF_NOTIFY_RESUME calls tracehook_notify_resume()
# signal delivery calls tracehook_signal_handler()
#
+# See CONFIG_TRACEHOOK_SELF_TEST for some test code to help.
+#
config HAVE_ARCH_TRACEHOOK
bool

diff --git a/kernel/Makefile b/kernel/Makefile
index d7c13d2..5b5e4e4 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_FREEZER) += power/
obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
obj-$(CONFIG_KEXEC) += kexec.o
obj-$(CONFIG_BACKTRACE_SELF_TEST) += backtracetest.o
+obj-$(CONFIG_TRACEHOOK_SELF_TEST) += tracehooktest.o
obj-$(CONFIG_COMPAT) += compat.o
obj-$(CONFIG_CGROUPS) += cgroup.o
obj-$(CONFIG_CGROUP_FREEZER) += cgroup_freezer.o
diff --git a/kernel/tracehooktest.c b/kernel/tracehooktest.c
new file mode 100644
index 0000000..3a7d491
--- /dev/null
+++ b/kernel/tracehooktest.c
@@ -0,0 +1,134 @@
+/*
+ * Self tests for the misc pieces of the tracehook code.
+ *
+ * Copyright 2010 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/ptrace.h>
+
+#include <asm/syscall.h>
+
+static int __init syscall_test(void)
+{
+ struct pt_regs _regs, *regs = &_regs;
+ long _args[8];
+ long *args = _args + 1;
+ long *sys_args[6], *sa;
+ unsigned int i, n, s;
+
+ /*
+ * First find each system register in pt_regs. We have to assume
+ * syscall_set_arguments() works with very basic arguments.
+ */
+ pr_info("TEST: asm/syscall.h: arg offsets: { ");
+
+ for (s = 0; s < 6; ++s)
+ args[s] = s;
+ memset(regs, 0xad, sizeof(*regs));
+ syscall_set_arguments(NULL, regs, 0, 6, args);
+
+ for (s = 0; s < 6; ++s) {
+ for (sa = (long *)regs; sa < (long *)(regs + 1); ++sa)
+ if (memcmp(&s, sa, sizeof(s)) == 0)
+ break;
+ if (sa == (long *)(regs + 1)) {
+ pr_cont(" FAIL (couldn't locate sys arg %u)\n", s);
+ return 1;
+ }
+ sys_args[s] = sa;
+ pr_cont("%li ", (unsigned long)sa - (unsigned long)regs);
+ }
+ pr_cont("}: PASS\n");
+
+ /* Make sure syscall_get_arguments() works with basic args */
+ pr_info("TEST: asm/syscall.h: basic syscall_get_arguments(): ");
+ memset(regs, 0xad, sizeof(*regs));
+ for (s = 0; s < 6; ++s)
+ *sys_args[s] = s;
+ syscall_get_arguments(NULL, regs, 0, 6, args);
+ for (s = 0; s < 6; ++s)
+ if (args[s] != s) {
+ pr_cont("FAIL with arg %i (%li)\n", s, args[s]);
+ return 1;
+ }
+ pr_cont("PASS\n");
+
+ /* Now brute force all values of i/n */
+ pr_info("TEST: asm/syscall.h: all i/n get/set combos: ");
+ for (i = 0; i < 6; ++i) {
+ for (n = 0; n < 7 - i; ++n) {
+
+ /* Seed for syscall_get_arguments() test */
+ for (s = 0; s < 6; ++s)
+ *sys_args[s] = -(s + 1);
+ memset(_args, 0, sizeof(_args));
+ syscall_get_arguments(NULL, regs, i, n, args);
+
+ /* Check canaries */
+ if (_args[0] != 0 || _args[7] != 0)
+ goto abort_i_n_get_tests;
+
+ /* Check all system argument values */
+ for (s = i; s < i + n; ++s)
+ if (*sys_args[s] != args[s - i])
+ goto abort_i_n_get_tests;
+
+ /* Check all unused slots */
+ for (s = i + n; s < 7; ++s)
+ if (args[s] != 0)
+ goto abort_i_n_get_tests;
+
+ /* Seed for syscall_set_arguments() test */
+ for (s = 0; s < 6; ++s)
+ args[s] = -(s + 1);
+ memset(regs, 0, sizeof(*regs));
+ syscall_set_arguments(NULL, regs, i, n, args);
+
+ /* Check the entire register set */
+ for (sa = (long *)regs; sa < (long *)(regs + 1); ++sa) {
+ if (*sa == 0)
+ continue;
+ /* Only some args should be set */
+ for (s = i; s < i + n; ++s)
+ if (sa == sys_args[s])
+ break;
+ if (s == i + n)
+ goto abort_i_n_set_tests;
+ }
+
+ /* Check the valid system argument values */
+ for (s = i; s < i + n; ++s)
+ if (*sys_args[s] != -(s + 1 - i))
+ goto abort_i_n_set_tests;
+ }
+ }
+ pr_cont("PASS\n");
+
+ return 0;
+
+ abort_i_n_get_tests:
+ pr_cont("FAIL (get @ i=%u n=%u)\n", i, n);
+ return 1;
+ abort_i_n_set_tests:
+ pr_cont("FAIL (set @ i=%u n=%u)\n", i, n);
+ return 1;
+}
+
+static int __init tracehooktest_init(void)
+{
+ return syscall_test();
+}
+module_init(tracehooktest_init);
+
+static void __exit tracehooktest_exit(void)
+{
+ /* stub to unload */
+}
+module_exit(tracehooktest_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Mike Frysinger <[email protected]>");
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 139fdbc..34b6793 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -788,6 +788,21 @@ config BACKTRACE_SELF_TEST

Say N if you are unsure.

+config TRACEHOOK_SELF_TEST
+ tristate "Self test for the tracehook code"
+ depends on HAVE_ARCH_TRACEHOOK
+ default n
+ help
+ This option provides a kernel module that can be used to test
+ the kernel stack tracehook code. This option is not useful
+ for distributions or general kernels, but only for kernel
+ developers working on architecture code.
+
+ Say N if you are unsure.
+
+ Choose M here to compile this code as a module. The module will
+ be called tracehooktest.
+
config DEBUG_BLOCK_EXT_DEVT
bool "Force extended block device numbers and spread them"
depends on DEBUG_KERNEL
--
1.7.0


2010-02-15 19:40:36

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [PATCH] tracehook: add some self tests

On 02/15, Mike Frysinger wrote:
>
> +static int __init syscall_test(void)
> +{
> + struct pt_regs _regs, *regs = &_regs;
> + long _args[8];
> + long *args = _args + 1;
> + long *sys_args[6], *sa;
> + unsigned int i, n, s;
> +
> + /*
> + * First find each system register in pt_regs. We have to assume
> + * syscall_set_arguments() works with very basic arguments.
> + */
> + pr_info("TEST: asm/syscall.h: arg offsets: { ");
> +
> + for (s = 0; s < 6; ++s)
> + args[s] = s;
> + memset(regs, 0xad, sizeof(*regs));
> + syscall_set_arguments(NULL, regs, 0, 6, args);
^^^^

I am not sure ia64 can tolerate task == NULL. Hmm, even x86 checks
task_thread_info(task)->status.

Probably CONFIG_TRACEHOOK_SELF_TEST needs more attention ?

Otherwise, I convinced myself I understand what this code does, and
it looks good ;)

Oleg.

2010-02-15 20:28:07

by Roland McGrath

[permalink] [raw]
Subject: Re: [PATCH] tracehook: add some self tests

This is something of a misnomer, since asm/syscall.h is the only thing you
are testing.

As Oleg pointed out, not all arch definitions can be used without a proper
task_struct argument. Only ia64 actually needs the task as part of the
actual register access. But several others (including x86) look at the
task to decide whether to use the 32-bit or 64-bit interpretation of the
register values.

I'd make the more general point that this sort of "synthetic" test does not
seem very useful. At best, it can test the asm/syscall.h code for being
internally consistent--but that doesn't test whether it's really correct.
IMHO this is not worth having unless it's an "empirical" test. What I mean
by that is one that really uses the asm/syscall.h calls as specified, and
in the context specified. So, you'd have to fork a user process and use
ptrace on it to get it stopped at a syscall entry. Then you can fetch the
arguments, modify them, and look at the arguments it actually passes in to
the syscall.


Thanks,
Roland

2010-02-15 20:28:51

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] tracehook: add some self tests

On Mon, Feb 15, 2010 at 14:39, Oleg Nesterov wrote:
> On 02/15, Mike Frysinger wrote:
>> +static int __init syscall_test(void)
>> +{
>> +     struct pt_regs _regs, *regs = &_regs;
>> +     long _args[8];
>> +     long *args = _args + 1;
>> +     long *sys_args[6], *sa;
>> +     unsigned int i, n, s;
>> +
>> +     /*
>> +      * First find each system register in pt_regs.  We have to assume
>> +      * syscall_set_arguments() works with very basic arguments.
>> +      */
>> +     pr_info("TEST: asm/syscall.h: arg offsets: { ");
>> +
>> +     for (s = 0; s < 6; ++s)
>> +             args[s] = s;
>> +     memset(regs, 0xad, sizeof(*regs));
>> +     syscall_set_arguments(NULL, regs, 0, 6, args);
>                              ^^^^
>
> I am not sure ia64 can tolerate task == NULL. Hmm, even x86 checks
> task_thread_info(task)->status.

is there some task available while the kernel is initializing ? i.e.
the test is compiled in ?

if current is valid, i'll just [ab]use that ...
-mike

2010-02-15 20:33:27

by Roland McGrath

[permalink] [raw]
Subject: Re: [PATCH] tracehook: add some self tests

> is there some task available while the kernel is initializing ? i.e.
> the test is compiled in ?
>
> if current is valid, i'll just [ab]use that ...

I don't think you can win with any kind of abuse on ia64. (For other
machines that only test the compat state of the task, any task will
do--though perhaps only test half the code.) AFAICT, on ia64 only a real
empirical test can work at all. That is, you must really be referring to a
task and its task_pt_regs() when that task really has entered the kernel
for a syscall.


Thanks,
Roland

2010-02-15 21:11:23

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] tracehook: add some self tests

On Mon, Feb 15, 2010 at 15:28, Roland McGrath wrote:
> This is something of a misnomer, since asm/syscall.h is the only thing you
> are testing.

it's meant as a starting point, not the end point. so overtime people
can easily extend it.

> I'd make the more general point that this sort of "synthetic" test does not
> seem very useful.  At best, it can test the asm/syscall.h code for being
> internally consistent--but that doesn't test whether it's really correct.

i wrote it because i needed it when trying to make sure the various
i/n values worked correctly. writing a bit of static code based on
just "n" is trivial, but avoiding a nest of code with i/n is a lot
harder. and as i noted earlier, there is on code in the kernel that
ever calls with i being non-zero.

> IMHO this is not worth having unless it's an "empirical" test.  What I mean
> by that is one that really uses the asm/syscall.h calls as specified, and
> in the context specified.  So, you'd have to fork a user process and use
> ptrace on it to get it stopped at a syscall entry.  Then you can fetch the
> arguments, modify them, and look at the arguments it actually passes in to
> the syscall.

that would indeed be useful as a next point
-mike