Re: [LTP] coredump01: New core_pattern specifiers test

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

> > 	ssize_t rval, i;
> > 	int fd, elf;
> >
> > 	if (bytes >= 4 && !memcmp(magic, "\177ELF", 4))
> > 		elf = 1;
> >
> > 	dprintf(fd, "exe=%s pid=%s sig=%s bytes=%lld elf=%d\n",
> > 		argv[1], argv[2], argv[3], bytes, elf);
> 
> Could `elf` be initialized to zero? For short or non-ELF input the
> condition does not assign it, so `dprintf()` reads an indeterminate value.
> A nonzero value can make the test accept a malformed core stream as ELF.

This can be fixed.

> 
> > 	SAFE_PRCTL(PR_GET_DUMPABLE, 1, 0, 0, 0);
> 
> Should this use `PR_SET_DUMPABLE`? `PR_GET_DUMPABLE` only returns the
> current state and ignores arg2, so this call does not ensure that
> `abort()` can produce a core dump.
> 
> > 	/* the kernel spawns the helper asynchronously */
> > 	if (TST_RETRY_FN_EXP_BACKOFF(access(res, F_OK), TST_RETVAL_EQ0, HELPER_TIMEOUT)) {
> > 		tst_res(TFAIL, "%s did not report any core dump", HELPER);
> 
> Could kernels with `CONFIG_STATIC_USERMODEHELPER` be rejected with TCONF
> before this check? In particular, an empty
> `CONFIG_STATIC_USERMODEHELPER_PATH` intentionally disables the helper, so
> this timeout reports TFAIL without testing specifier expansion.

This can be added in needs_kconfig.

> 
> > 	set_pattern("|%s/%s %%e %%p %%s %s/res.%%p", helper_dir, HELPER, cwd);
> >
> > 	len = SAFE_READLINK("/proc/self/exe", helper_dir, sizeof(helper_dir) - 1);
> 
> Could the pipe case require the initial mount namespace, or otherwise use
> paths visible there? The kernel resolves and runs a `core_pattern` pipe
> handler in the initial mount namespace. If LTP runs in another mount
> namespace, these helper and result paths may not exist there, producing a
> spurious TFAIL.
> 
> > /*
> >  * Core dump collector for the piped core_pattern tested by coredump01.
> >  *
> >  * Avoiding the LTP API here is correct, since the kernel spawns the helper
> >  * through ``call_usermodehelper()`` without the LTP IPC environment.
> >  */
> >
> > int main(int argc, char *argv[])
> 
> Could this helper follow the LTP helper-binary convention by defining
> `TST_NO_DEFAULT_MAIN` and including `tst_test.h`? The missing IPC
> environment means it should avoid IPC-backed result calls, but does not
> require avoiding the helper API form itself.

It doesn't make any sense. We can skip this one.

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp