Message ID | 02ade9af1634c4681156dceecfd51721284044e3.1562591065.git.jstancek@redhat.com |
---|---|
State | Accepted |
Delegated to: | Li Wang |
Headers | show |
Series | [v3.1] lib: add tst_no_corefile() | expand |
Hi! > +void tst_no_corefile(int verbose) > +{ > + struct rlimit new_r, old_r; > + > + SAFE_GETRLIMIT(RLIMIT_CORE, &old_r); > + if (old_r.rlim_max >= 1 || geteuid() == 0) { > + /* > + * 1 is a special value, that disables core-to-pipe. > + * At the same time it is small enough value for > + * core-to-file, so it skips creating cores as well. > + */ > + new_r.rlim_cur = 1; > + new_r.rlim_max = 1; > + SAFE_SETRLIMIT(RLIMIT_CORE, &new_r); > + } > + > + if (verbose) > + tst_res(TINFO, "Avoid dumping corefile for process(pid=%d)", getpid()); Should we print the message here only if (old_r.rlim_max <= 1 || geteuid() == 0) because otherwise we will print the mesasge even in cases that the corefile is not in fact limited.
----- Original Message ----- > Hi! > > +void tst_no_corefile(int verbose) > > +{ > > + struct rlimit new_r, old_r; > > + > > + SAFE_GETRLIMIT(RLIMIT_CORE, &old_r); > > + if (old_r.rlim_max >= 1 || geteuid() == 0) { > > + /* > > + * 1 is a special value, that disables core-to-pipe. > > + * At the same time it is small enough value for > > + * core-to-file, so it skips creating cores as well. > > + */ > > + new_r.rlim_cur = 1; > > + new_r.rlim_max = 1; > > + SAFE_SETRLIMIT(RLIMIT_CORE, &new_r); > > + } > > + > > + if (verbose) > > + tst_res(TINFO, "Avoid dumping corefile for process(pid=%d)", getpid()); > > Should we print the message here only > > if (old_r.rlim_max <= 1 || geteuid() == 0) > > because otherwise we will print the mesasge even in cases that the > corefile is not in fact limited. Depends on setup of core_pattern, if it's core-to-file it's already limited. If it's core-to-pipe, then it's no limit, so I agree, we can move it above. > > -- > Cyril Hrubis > chrubis@suse.cz >
On Mon, Jul 8, 2019 at 9:08 PM Jan Stancek <jstancek@redhat.com> wrote: > Li, > > I'd like to avoid generating TWARN when user has done nothing wrong. > What would you tell on this version? Only tst_no_corefile() > is changed compared to your v3. > Agree on this, TWARN seems unnecessary. Feel free to modify&push v3.1 as Cyril suggested. Thank you, Jan!
Hi Jan, On Thu, Jul 11, 2019 at 8:45 PM Jan Stancek <jstancek@redhat.com> wrote: > ... > > > > because otherwise we will print the mesasge even in cases that the > > corefile is not in fact limited. > > Depends on setup of core_pattern, if it's core-to-file it's already > limited. > If it's core-to-pipe, then it's no limit, so I agree, we can move it above. > > I helped to push the v3.1 base on Cyril's suggestion. Since I'm going to apply the pkey testcase. Thanks.
----- Original Message ----- > Hi Jan, > > On Thu, Jul 11, 2019 at 8:45 PM Jan Stancek <jstancek@redhat.com> wrote: > > > ... > > > > > > because otherwise we will print the mesasge even in cases that the > > > corefile is not in fact limited. > > > > Depends on setup of core_pattern, if it's core-to-file it's already > > limited. > > If it's core-to-pipe, then it's no limit, so I agree, we can move it above. > > > > > I helped to push the v3.1 base on Cyril's suggestion. Since I'm going to > apply the pkey testcase. Thanks. Thanks Li. > > -- > Regards, > Li Wang >
diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt index 4b1e7d25b53f..d9d25c659b34 100644 --- a/doc/test-writing-guidelines.txt +++ b/doc/test-writing-guidelines.txt @@ -826,8 +826,8 @@ The 'TST_PROCESS_STATE_WAIT()' waits until process 'pid' is in requested It's mostly used with state 'S' which means that process is sleeping in kernel for example in 'pause()' or any other blocking syscall. -2.2.10 Signal handlers -^^^^^^^^^^^^^^^^^^^^^^ +2.2.10 Signals and signal handlers +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ If you need to use signal handlers, keep the code short and simple. Don't forget that the signal handler is called asynchronously and can interrupt the @@ -859,6 +859,14 @@ type defined in C99 but this one *DOES NOT* imply 'volatile' (it's just a 'typedef' to 'int'). So the correct type for a flag that is changed from a signal handler is either 'volatile int' or 'volatile sig_atomic_t'. +If a crash (e.g. triggered by signal SIGSEGV) is expected in testing, you +can avoid creation of core files by calling tst_no_corefile() function. +This takes effect for process (and its children) which invoked it, unless +they subsequently modify RLIMIT_CORE. + +Note that LTP library will reap any processes that test didn't reap itself, +and report any non-zero exit code as failure. + 2.2.11 Kernel Modules ^^^^^^^^^^^^^^^^^^^^^ diff --git a/include/tst_coredump.h b/include/tst_coredump.h new file mode 100644 index 000000000000..e1f892544aa8 --- /dev/null +++ b/include/tst_coredump.h @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2019 Red Hat, Inc. + */ + +#ifndef TST_COREDUMP__ +#define TST_COREDUMP__ + +/* + * If crash is expected, avoid dumping corefile. + * 1 is a special value, that disables core-to-pipe. + * At the same time it is small enough value for + * core-to-file, so it skips creating cores as well. + */ +void tst_no_corefile(int verbose); + +#endif /* TST_COREDUMP_H */ + diff --git a/include/tst_test.h b/include/tst_test.h index 2e8e3635204e..b50e88b60666 100644 --- a/include/tst_test.h +++ b/include/tst_test.h @@ -33,6 +33,7 @@ #include "tst_get_bad_addr.h" #include "tst_path_has_mnt_flags.h" #include "tst_sys_conf.h" +#include "tst_coredump.h" /* * Reports testcase result. diff --git a/lib/tst_coredump.c b/lib/tst_coredump.c new file mode 100644 index 000000000000..2cae5771fb8e --- /dev/null +++ b/lib/tst_coredump.c @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2019 Red Hat, Inc. + */ + +#define TST_NO_DEFAULT_MAIN + +#include <sys/time.h> +#include <sys/resource.h> + +#include "tst_test.h" +#include "tst_coredump.h" + +void tst_no_corefile(int verbose) +{ + struct rlimit new_r, old_r; + + SAFE_GETRLIMIT(RLIMIT_CORE, &old_r); + if (old_r.rlim_max >= 1 || geteuid() == 0) { + /* + * 1 is a special value, that disables core-to-pipe. + * At the same time it is small enough value for + * core-to-file, so it skips creating cores as well. + */ + new_r.rlim_cur = 1; + new_r.rlim_max = 1; + SAFE_SETRLIMIT(RLIMIT_CORE, &new_r); + } + + if (verbose) + tst_res(TINFO, "Avoid dumping corefile for process(pid=%d)", getpid()); +} diff --git a/testcases/kernel/security/umip/umip_basic_test.c b/testcases/kernel/security/umip/umip_basic_test.c index c34d4a1f618d..37850ef9ff71 100644 --- a/testcases/kernel/security/umip/umip_basic_test.c +++ b/testcases/kernel/security/umip/umip_basic_test.c @@ -86,6 +86,8 @@ static void verify_umip_instruction(unsigned int n) pid = SAFE_FORK(); if (pid == 0) { + tst_no_corefile(0); + switch (n) { case 0: asm_sgdt(); diff --git a/testcases/kernel/syscalls/ipc/shmat/shmat01.c b/testcases/kernel/syscalls/ipc/shmat/shmat01.c index aa9dfd4e5baa..f759147994a2 100644 --- a/testcases/kernel/syscalls/ipc/shmat/shmat01.c +++ b/testcases/kernel/syscalls/ipc/shmat/shmat01.c @@ -59,19 +59,9 @@ static void *expected_addr(void *in_addr, void *out_addr) static void do_child(int *in_addr, int expect_crash) { - if (expect_crash) { - /* - * Crash is expected, avoid dumping corefile. - * 1 is a special value, that disables core-to-pipe. - * At the same time it is small enough value for - * core-to-file, so it skips creating cores as well. - */ - struct rlimit r; - - r.rlim_cur = 1; - r.rlim_max = 1; - SAFE_SETRLIMIT(RLIMIT_CORE, &r); - } + if (expect_crash) + tst_no_corefile(1); + *in_addr = 10; exit(0);