Message ID | 20190510132400.28963-1-camann@suse.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [v1] syscalls/acct02: add functional testcase | expand |
Hi! > diff --git a/testcases/kernel/syscalls/acct/.gitignore b/testcases/kernel/syscalls/acct/.gitignore > index c2fb15611..ed68d202c 100644 > --- a/testcases/kernel/syscalls/acct/.gitignore > +++ b/testcases/kernel/syscalls/acct/.gitignore > @@ -1 +1,2 @@ > /acct01 > +/acct02 > diff --git a/testcases/kernel/syscalls/acct/acct02.c b/testcases/kernel/syscalls/acct/acct02.c > new file mode 100644 > index 000000000..4ddc482c3 > --- /dev/null > +++ b/testcases/kernel/syscalls/acct/acct02.c > @@ -0,0 +1,166 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > + > +/* > + * Copyright (c) SUSE LLC, 2019 > + * Author: Christian Amann <camann@suse.com> > + */ > + > +/* > + * This tests if the kernel writes correct data to the > + * process accounting file. > + * > + * First, system-wide process accounting is turned on and the output gets > + * directed to a defined file. After that a simple command is issued in order > + * to generate data and the process accounting gets turned off again. > + * > + * To verify the written data, the entries of the accounting file get > + * parsed into the corresponding acct structure. Since it cannot be guaranteed > + * that only the command issued by this test gets written into the accounting > + * file, the contents get parsed until the correct entry is found, or EOF > + * is reached. > + */ > + > +#include <sys/stat.h> > +#include <sys/types.h> > +#include <errno.h> > +#include <string.h> > +#include <time.h> > +#include <unistd.h> > +#include "config.h" > +#include "tst_kconfig.h" > +#include "tst_test.h" > + > +#ifdef HAVE_STRUCT_ACCT_V3 ^ Can't we, rather than ifdefing the test out, add a fallback definition? > +#include <sys/acct.h> > + > +#define COMMAND "echo" ^ Can we rather than this create a dummy program that has more or less unique name? Bonus points for sleeping/burning cpu time for a second in the test program and then checking that the utime and etime are within resonable bounds. We can probably check different exit values as well. > +#define OUTPUT_FILE "acct_file" > +#define SIZE sizeof(struct acct_v3) ^ Shouldn't this depend on the return value from the acct_version_is_3() ? > + > +static int fd; > +static int v3; > +static unsigned int rc; > +static unsigned int start_time; > + > +static union acct_union { > + struct acct v0; > + struct acct_v3 v3; > +} ACCT; ^ Uppercase identifiers are mostly used for macros. > +static int acct_version_is_3(void) > +{ > + const char *kconfig_acct_v3[] = { > + "CONFIG_BSD_PROCESS_ACCT_V3", > + NULL > + }; > + > + struct tst_kconfig_res results[1]; > + > + tst_kconfig_read(kconfig_acct_v3, results, 1); > + > + return results[0].match == 'y'; > +} > + > +static void run_command(void) > +{ > + const char *const cmd[] = {COMMAND, NULL}; > + > + rc = tst_run_cmd(cmd, NULL, NULL, 1); > +} > + > +static int verify_acct(struct acct *acc) > +{ > + if (strcmp(acc->ac_comm, COMMAND) || > + acc->ac_btime - start_time > 1 || ^ This will also pass if there is a garbage in the ac_btime that happens to be <= start_time e.g. for ac_btime == 0. And it may also fail when we executed the command right before the kernel timer wheel increments. We should rather check that ac_btime >= start_time and that ac_btime - start_time <= 1. > + acc->ac_uid != getuid() || > + acc->ac_gid != getgid() || > + acc->ac_exitcode != rc) > + return 0; > + return 1; > +} > + > +static int verify_acct_v3(struct acct_v3 *acc) > +{ > + if (strcmp(acc->ac_comm, COMMAND) || > + acc->ac_btime - start_time > 1 || Here as well. > + acc->ac_uid != getuid() || > + acc->ac_gid != getgid() || > + acc->ac_exitcode != rc || > + acc->ac_version != 3 || > + acc->ac_pid < 1) > + return 0; > + return 1; > +} > + > +static void run(void) > +{ > + int read_bytes, ret; > + > + fd = SAFE_OPEN(OUTPUT_FILE, O_RDWR | O_CREAT, 0644); > + > + TEST(acct(OUTPUT_FILE)); > + if (TST_RET == -1) > + tst_brk(TBROK | TTERRNO, "Could not set acct output file"); > + > + start_time = time(NULL); > + run_command(); > + acct(NULL); > + > + do { > + read_bytes = SAFE_READ(0, fd, &ACCT, SIZE); > + > + if (v3) > + ret = verify_acct_v3(&ACCT.v3); > + else > + ret = verify_acct(&ACCT.v0); > + > + if (!ret) > + tst_res(TINFO, "acct file entry did not match; " > + "trying to check next entry..."); ^ This will probably flood the test output, can't we just increment counter here and print how many entries we processed? > + } while (read_bytes == SIZE && !ret); > + > + if (ret) > + tst_res(TPASS, "acct() wrote correct file contents!"); > + else > + tst_res(TFAIL, "acct() wrote incorrect file contents!"); > +} > + > +static void setup(void) > +{ > + TEST(acct(NULL)); > + if (TST_RET == -1) > + tst_brk(TBROK | TTERRNO, > + "acct() system call returned with error"); > + > + v3 = acct_version_is_3(); > + if (v3) > + tst_res(TINFO, "Verifying using 'struct acct_v3'"); > + else > + tst_res(TINFO, "Verifying using 'struct acct'"); > +} > + > +static void cleanup(void) > +{ > + if (fd > 0) > + SAFE_CLOSE(fd); > + acct(NULL); > +} > + > +static const char *kconfigs[] = { > + "CONFIG_BSD_PROCESS_ACCT", > + NULL > +}; > + > +static struct tst_test test = { > + .test_all = run, > + .needs_kconfigs = kconfigs, > + .setup = setup, > + .cleanup = cleanup, > + .needs_tmpdir = 1, > + .needs_root = 1, > +}; > + > +#else > + TST_TEST_TCONF("This system does not support acct_v3 structure"); > +#endif /* HAVE_STRUCT_ACCT_V3 */ Otherwise it looks good.
diff --git a/configure.ac b/configure.ac index fad8f8396..8d1e74b0c 100644 --- a/configure.ac +++ b/configure.ac @@ -191,6 +191,7 @@ AC_CONFIG_SUBDIRS([utils/ffsb-6.0-rc2]) AC_CONFIG_COMMANDS([syscalls.h], [cd ${ac_top_srcdir}/include/lapi/syscalls; ./regen.sh]) # custom functions +LTP_CHECK_ACCT LTP_CHECK_ACL_SUPPORT LTP_CHECK_ATOMIC_MEMORY_MODEL LTP_CHECK_BUILTIN_CLEAR_CACHE diff --git a/m4/ltp-acct.m4 b/m4/ltp-acct.m4 new file mode 100644 index 000000000..61bc01947 --- /dev/null +++ b/m4/ltp-acct.m4 @@ -0,0 +1,7 @@ +dnl SPDX-License-Identifier: GPL-2.0-or-later +dnl Copyright (c) 2019 SUSE LLC +dnl Author: Christian Amann <camann@suse.com> + +AC_DEFUN([LTP_CHECK_ACCT],[ +AC_CHECK_TYPES([struct acct_v3],,,[#include <sys/acct.h>]) +]) diff --git a/runtest/syscalls b/runtest/syscalls index 2b8ca719b..253055de3 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -10,6 +10,7 @@ access03 access03 access04 access04 acct01 acct01 +acct02 acct02 add_key01 add_key01 add_key02 add_key02 diff --git a/testcases/kernel/syscalls/acct/.gitignore b/testcases/kernel/syscalls/acct/.gitignore index c2fb15611..ed68d202c 100644 --- a/testcases/kernel/syscalls/acct/.gitignore +++ b/testcases/kernel/syscalls/acct/.gitignore @@ -1 +1,2 @@ /acct01 +/acct02 diff --git a/testcases/kernel/syscalls/acct/acct02.c b/testcases/kernel/syscalls/acct/acct02.c new file mode 100644 index 000000000..4ddc482c3 --- /dev/null +++ b/testcases/kernel/syscalls/acct/acct02.c @@ -0,0 +1,166 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +/* + * Copyright (c) SUSE LLC, 2019 + * Author: Christian Amann <camann@suse.com> + */ + +/* + * This tests if the kernel writes correct data to the + * process accounting file. + * + * First, system-wide process accounting is turned on and the output gets + * directed to a defined file. After that a simple command is issued in order + * to generate data and the process accounting gets turned off again. + * + * To verify the written data, the entries of the accounting file get + * parsed into the corresponding acct structure. Since it cannot be guaranteed + * that only the command issued by this test gets written into the accounting + * file, the contents get parsed until the correct entry is found, or EOF + * is reached. + */ + +#include <sys/stat.h> +#include <sys/types.h> +#include <errno.h> +#include <string.h> +#include <time.h> +#include <unistd.h> +#include "config.h" +#include "tst_kconfig.h" +#include "tst_test.h" + +#ifdef HAVE_STRUCT_ACCT_V3 +#include <sys/acct.h> + +#define COMMAND "echo" +#define OUTPUT_FILE "acct_file" +#define SIZE sizeof(struct acct_v3) + +static int fd; +static int v3; +static unsigned int rc; +static unsigned int start_time; + +static union acct_union { + struct acct v0; + struct acct_v3 v3; +} ACCT; + +static int acct_version_is_3(void) +{ + const char *kconfig_acct_v3[] = { + "CONFIG_BSD_PROCESS_ACCT_V3", + NULL + }; + + struct tst_kconfig_res results[1]; + + tst_kconfig_read(kconfig_acct_v3, results, 1); + + return results[0].match == 'y'; +} + +static void run_command(void) +{ + const char *const cmd[] = {COMMAND, NULL}; + + rc = tst_run_cmd(cmd, NULL, NULL, 1); +} + +static int verify_acct(struct acct *acc) +{ + if (strcmp(acc->ac_comm, COMMAND) || + acc->ac_btime - start_time > 1 || + acc->ac_uid != getuid() || + acc->ac_gid != getgid() || + acc->ac_exitcode != rc) + return 0; + return 1; +} + +static int verify_acct_v3(struct acct_v3 *acc) +{ + if (strcmp(acc->ac_comm, COMMAND) || + acc->ac_btime - start_time > 1 || + acc->ac_uid != getuid() || + acc->ac_gid != getgid() || + acc->ac_exitcode != rc || + acc->ac_version != 3 || + acc->ac_pid < 1) + return 0; + return 1; +} + +static void run(void) +{ + int read_bytes, ret; + + fd = SAFE_OPEN(OUTPUT_FILE, O_RDWR | O_CREAT, 0644); + + TEST(acct(OUTPUT_FILE)); + if (TST_RET == -1) + tst_brk(TBROK | TTERRNO, "Could not set acct output file"); + + start_time = time(NULL); + run_command(); + acct(NULL); + + do { + read_bytes = SAFE_READ(0, fd, &ACCT, SIZE); + + if (v3) + ret = verify_acct_v3(&ACCT.v3); + else + ret = verify_acct(&ACCT.v0); + + if (!ret) + tst_res(TINFO, "acct file entry did not match; " + "trying to check next entry..."); + } while (read_bytes == SIZE && !ret); + + if (ret) + tst_res(TPASS, "acct() wrote correct file contents!"); + else + tst_res(TFAIL, "acct() wrote incorrect file contents!"); +} + +static void setup(void) +{ + TEST(acct(NULL)); + if (TST_RET == -1) + tst_brk(TBROK | TTERRNO, + "acct() system call returned with error"); + + v3 = acct_version_is_3(); + if (v3) + tst_res(TINFO, "Verifying using 'struct acct_v3'"); + else + tst_res(TINFO, "Verifying using 'struct acct'"); +} + +static void cleanup(void) +{ + if (fd > 0) + SAFE_CLOSE(fd); + acct(NULL); +} + +static const char *kconfigs[] = { + "CONFIG_BSD_PROCESS_ACCT", + NULL +}; + +static struct tst_test test = { + .test_all = run, + .needs_kconfigs = kconfigs, + .setup = setup, + .cleanup = cleanup, + .needs_tmpdir = 1, + .needs_root = 1, +}; + +#else + TST_TEST_TCONF("This system does not support acct_v3 structure"); +#endif /* HAVE_STRUCT_ACCT_V3 */ +
This patch adds a functional test to verify that the file contents of the process accounting file contains valid and correct data when triggered using the acct() syscall. Signed-off-by: Christian Amann <camann@suse.com> --- configure.ac | 1 + m4/ltp-acct.m4 | 7 ++ runtest/syscalls | 1 + testcases/kernel/syscalls/acct/.gitignore | 1 + testcases/kernel/syscalls/acct/acct02.c | 166 ++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+) create mode 100644 m4/ltp-acct.m4 create mode 100644 testcases/kernel/syscalls/acct/acct02.c