NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

kern/51621: PT_ATTACH from a parent is unreliable



>Number:         51621
>Category:       kern
>Synopsis:       PT_ATTACH from a parent is unreliable
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Nov 11 11:30:00 +0000 2016
>Originator:     Kamil Rytarowski
>Release:        NetBSD 7.99.42 amd64
>Organization:
TNF
>Environment:
NetBSD chieftec 7.99.42 NetBSD 7.99.42 (GENERIC) #0: Wed Nov  9 23:58:23 CET 2016  root@chieftec:/tmp/netbsd-tmp/sys/arch/amd64/compile/GENERIC amd64
>Description:
I want to attach a child from parent with PT_ATTACH & PT_CONTINUE. However I faced the following issues:

- PT_ATTACH seems to work, but waiting for stopped status and signal from the
  child results in getting SIGTRAP, not SIGSTOP like in Linux and FreeBSD. This
  might be by design, I'm unsure. However, so far I was getting SIGSTOP from a
  tracer process that was not the parent. SIGSTOP vs SIGTRAP logic also complicates
  the things up as tracer must check whether is a parent for tracee or not -
  this shouldn't be needed.

- PT_CONTINUE seems to have no effect at all, the child hangs. This operation
  works on Linux and FreeBSD and in the end, test passes correctly.

- Debugging this with gdb(1) results in receiving SIGABRT from the GNU
  debugger (in the moment of raising/receiving SIGTRAP). This is making the things harder in general.

The same code can be debugged on Linux without issues.
>How-To-Repeat:
/* modified src/tests/kernel/t_ptrace_wait.c (c) TNF */

#include <sys/cdefs.h>

#include <sys/param.h>
#include <sys/types.h>
#include <sys/ptrace.h>
#include <sys/resource.h>
#include <sys/sysctl.h>
#include <sys/wait.h>
#include <assert.h>
#include <err.h>
#include <errno.h>
#include <signal.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <strings.h>
#include <unistd.h>

#ifdef __FreeBSD__
#include <sys/user.h>
#endif

#define TWAIT_WAITPID

#ifndef __used
#define __used __attribute__((__used__))
#endif

#ifndef __arraycount
#define __arraycount nitems
#endif

#define atf_utils_fork fork
#define ATF_REQUIRE(a) do{assert((a));}while(0)
#define ATF_REQUIRE_MSG(a,b,...) do{assert((a));}while(0)
#define ATF_REQUIRE_ERRNO(a,b) do{printf("got_errno %d (expected %d)\n", (a), errno);assert((a) == errno);assert((b));}while(0)
#define ATF_REQUIRE_EQ_MSG(a,b,c,...) do{assert((a) == (b));}while(0)

/* Detect plain wait(2) use-case */
#if !defined(TWAIT_WAITPID) && \
    !defined(TWAIT_WAITID) && \
    !defined(TWAIT_WAIT3) && \
    !defined(TWAIT_WAIT4) && \
    !defined(TWAIT_WAIT6)
#define TWAIT_WAIT
#endif

/*
 * There are two classes of wait(2)-like functions:
 * - wait4(2)-like accepting pid_t, optional options parameter, struct rusage*
 * - wait6(2)-like accepting idtype_t, id_t, struct wrusage, mandatory options
 *
 * The TWAIT_FNAME value is to be used for convenience in debug messages.
 *
 * The TWAIT_GENERIC() macro is designed to reuse the same unmodified
 * code with as many wait(2)-like functions as possible.
 *
 * In a common use-case wait4(2) and wait6(2)-like function can work the almost
 * the same way, however there are few important differences:
 * wait6(2) must specify P_PID for idtype to match wpid from wait4(2).
 * To behave like wait4(2), wait6(2) the 'options' to wait must include
 * WEXITED|WTRUNCATED.
 *
 * There are two helper macros (they purpose it to mach more than one
 * wait(2)-like function):
 * The TWAIT_HAVE_STATUS - specifies whether a function can retrieve
 *                         status (as integer value).
 * The TWAIT_HAVE_PID    - specifies whether a function can request
 *                         exact process identifier
 * The TWAIT_HAVE_RUSAGE - specifies whether a function can request
 *                         the struct rusage value
 * 
 */

#if defined(TWAIT_WAIT)
#	define TWAIT_FNAME			"wait"
#	define TWAIT_WAIT4TYPE(a,b,c,d)		wait((b))
#	define TWAIT_GENERIC(a,b,c)		wait((b))
#	define TWAIT_HAVE_STATUS		1
#elif defined(TWAIT_WAITPID)
#	define TWAIT_FNAME			"waitpid"
#	define TWAIT_WAIT4TYPE(a,b,c,d)		waitpid((a),(b),(c))
#	define TWAIT_GENERIC(a,b,c)		waitpid((a),(b),(c))
#	define TWAIT_HAVE_PID			1
#	define TWAIT_HAVE_STATUS		1
#elif defined(TWAIT_WAITID)
#	define TWAIT_FNAME			"waitid"
#	define TWAIT_GENERIC(a,b,c)		\
		waitid(P_PID,(a),NULL,(c)|WEXITED|WTRAPPED)
#	define TWAIT_WAIT6TYPE(a,b,c,d,e,f)	waitid((a),(b),(f),(d))
#	define TWAIT_HAVE_PID			1
#elif defined(TWAIT_WAIT3)
#	define TWAIT_FNAME			"wait3"
#	define TWAIT_WAIT4TYPE(a,b,c,d)		wait3((b),(c),(d))
#	define TWAIT_GENERIC(a,b,c)		wait3((b),(c),NULL)
#	define TWAIT_HAVE_STATUS		1
#	define TWAIT_HAVE_RUSAGE		1
#elif defined(TWAIT_WAIT4)
#	define TWAIT_FNAME			"wait4"
#	define TWAIT_WAIT4TYPE(a,b,c,d)		wait4((a),(b),(c),(d))
#	define TWAIT_GENERIC(a,b,c)		wait4((a),(b),(c),NULL)
#	define TWAIT_HAVE_PID			1
#	define TWAIT_HAVE_STATUS		1
#	define TWAIT_HAVE_RUSAGE		1
#elif defined(TWAIT_WAIT6)
#	define TWAIT_FNAME			"wait6"
#	define TWAIT_WAIT6TYPE(a,b,c,d,e,f)	wait6((a),(b),(c),(d),(e),(f))
#	define TWAIT_GENERIC(a,b,c)		\
		wait6(P_PID,(a),(b),(c)|WEXITED|WTRAPPED,NULL,NULL)
#	define TWAIT_HAVE_PID			1
#	define TWAIT_HAVE_STATUS		1
#endif

/*
 * There are 3 groups of tests:
 * - TWAIT_GENERIC()	(wait, wait2, waitpid, wait3, wait4, wait6)
 * - TWAIT_WAIT4TYPE()	(wait2, waitpid, wait3, wait4)
 * - TWAIT_WAIT6TYPE()	(waitid, wait6)
 *
 * Tests only in the above categories are allowed. However some tests are not
 * possible in the context requested functionality to be verified, therefore
 * there are helper macros:
 * - TWAIT_HAVE_PID	(wait2, waitpid, waitid, wait4, wait6)
 * - TWAIT_HAVE_STATUS	(wait, wait2, waitpid, wait3, wait4, wait6)
 * - TWAIT_HAVE_RUSAGE	(wait3, wait4)
 * - TWAIT_HAVE_RETPID	(wait, wait2, waitpid, wait3, wait4, wait6)
 *
 * If there is an intention to test e.g. wait6(2) specific features in the
 * ptrace(2) context, find the most matching group and with #ifdefs reduce
 * functionality of less featured than wait6(2) interface (TWAIT_WAIT6TYPE).
 *
 * For clarity never use negative preprocessor checks, like:
 *     #if !defined(TWAIT_WAIT4)
 * always refer to checks for positive values.
 */

/*
 * A child process cannot call atf functions and expect them to magically
 * work like in the parent.
 * The printf(3) messaging from a child will not work out of the box as well
 * without estabilishing a communication protocol with its parent. To not
 * overcomplicate the tests - do not log from a child and use err(3)/errx(3)
 * wrapped with FORKEE_ASSERT()/FORKEE_ASSERTX() as that is guaranteed to work.
 */
#define FORKEE_ASSERT(x) do{assert((x));}while(0)
#define FORKEE_ASSERTX(x) do{assert((x));}while(0)

/*
 * If waitid(2) returns because one or more processes have a state change to
 * report, 0 is returned.  If an error is detected, a value of -1 is returned
 * and errno is set to indicate the error. If WNOHANG is specified and there
 * are no stopped, continued or exited children, 0 is returned.
 */
#if defined(TWAIT_WAITID)
#define TWAIT_REQUIRE_SUCCESS(a,b)	ATF_REQUIRE((a) == 0)
#define TWAIT_REQUIRE_FAILURE(a,b)	ATF_REQUIRE_ERRNO((a),(b) == -1)
#define FORKEE_REQUIRE_SUCCESS(a,b)	FORKEE_ASSERTX((a) == 0)
#define FORKEE_REQUIRE_FAILURE(a,b)	\
	FORKEE_ASSERTX(((a) == errno) && ((b) == -1))
#else
#define TWAIT_REQUIRE_SUCCESS(a,b)	ATF_REQUIRE((a) == (b))
#define TWAIT_REQUIRE_FAILURE(a,b)	ATF_REQUIRE_ERRNO((a),(b) == -1)
#define FORKEE_REQUIRE_SUCCESS(a,b)	FORKEE_ASSERTX((a) == (b))
#define FORKEE_REQUIRE_FAILURE(a,b)	\
	FORKEE_ASSERTX(((a) == errno) && ((b) == -1))
#endif

/*
 * Helper tools to verify whether status reports exited value
 */
#if TWAIT_HAVE_STATUS
static void __used
validate_status_exited(int status, int expected)
{
        ATF_REQUIRE_MSG(WIFEXITED(status), "Reported exited process");
        ATF_REQUIRE_MSG(!WIFCONTINUED(status), "Reported continued process");
        ATF_REQUIRE_MSG(!WIFSIGNALED(status), "Reported signaled process");
        ATF_REQUIRE_MSG(!WIFSTOPPED(status), "Reported stopped process");

	ATF_REQUIRE_EQ_MSG(WEXITSTATUS(status), expected,
	    "The process has exited with invalid value");
}

static void __used
forkee_status_exited(int status, int expected)
{
	FORKEE_ASSERTX(WIFEXITED(status));
	FORKEE_ASSERTX(!WIFCONTINUED(status));
	FORKEE_ASSERTX(!WIFSIGNALED(status));
	FORKEE_ASSERTX(!WIFSTOPPED(status));

	FORKEE_ASSERTX(WEXITSTATUS(status) == expected);
}

static void __used
validate_status_continued(int status)
{
	ATF_REQUIRE_MSG(!WIFEXITED(status), "Reported exited process");
	ATF_REQUIRE_MSG(WIFCONTINUED(status), "Reported continued process");
	ATF_REQUIRE_MSG(!WIFSIGNALED(status), "Reported signaled process");
	ATF_REQUIRE_MSG(!WIFSTOPPED(status), "Reported stopped process");
}

static void __used
forkee_status_continued(int status)
{
	FORKEE_ASSERTX(!WIFEXITED(status));
	FORKEE_ASSERTX(WIFCONTINUED(status));
	FORKEE_ASSERTX(!WIFSIGNALED(status));
	FORKEE_ASSERTX(!WIFSTOPPED(status));
}

static void __used
validate_status_signaled(int status, int expected_termsig, int expected_core)
{
	ATF_REQUIRE_MSG(!WIFEXITED(status), "Reported exited process");
	ATF_REQUIRE_MSG(!WIFCONTINUED(status), "Reported continued process");
	ATF_REQUIRE_MSG(WIFSIGNALED(status), "Reported signaled process");
	ATF_REQUIRE_MSG(!WIFSTOPPED(status), "Reported stopped process");

	ATF_REQUIRE_EQ_MSG(WTERMSIG(status), expected_termsig,
	    "Unexpected signal received");

	ATF_REQUIRE_EQ_MSG(WCOREDUMP(status), expected_core,
	    "Unexpectedly core file %s generated", expected_core ? "not" : "");
}

static void __used
forkee_status_signaled(int status, int expected_termsig, int expected_core)
{
	FORKEE_ASSERTX(!WIFEXITED(status));
	FORKEE_ASSERTX(!WIFCONTINUED(status));
	FORKEE_ASSERTX(WIFSIGNALED(status));
	FORKEE_ASSERTX(!WIFSTOPPED(status));

	FORKEE_ASSERTX(WTERMSIG(status) == expected_termsig);
	FORKEE_ASSERTX(WCOREDUMP(status) == expected_core);
}

static void __used
validate_status_stopped(int status, int expected)
{
	printf("WIFSTOPPED(x)=%d\n", WIFSTOPPED(status));
	printf("WIFCONTINUED(x)=%d\n", WIFCONTINUED(status));
	printf("WIFSIGNALED(x)=%d\n", WIFSIGNALED(status));
	printf("WIFEXITED(x)=%d\n", WIFEXITED(status));
	printf("WSTOPSIG(status)=0x%x\n", WSTOPSIG(status));
	ATF_REQUIRE_MSG(!WIFEXITED(status), "Reported exited process");
	ATF_REQUIRE_MSG(!WIFCONTINUED(status), "Reported continued process");
	ATF_REQUIRE_MSG(!WIFSIGNALED(status), "Reported signaled process");
	ATF_REQUIRE_MSG(WIFSTOPPED(status), "Reported stopped process");

	ATF_REQUIRE_EQ_MSG(WSTOPSIG(status), expected,
	    "Unexpected stop signal received");
}

static void __used
forkee_status_stopped(int status, int expected)
{
	FORKEE_ASSERTX(!WIFEXITED(status));
	FORKEE_ASSERTX(!WIFCONTINUED(status));
	FORKEE_ASSERTX(!WIFSIGNALED(status));
	FORKEE_ASSERTX(WIFSTOPPED(status));

	FORKEE_ASSERTX(WSTOPSIG(status) == expected);
}
#else
#define validate_status_exited(a,b)
#define forkee_status_exited(a,b)
#define validate_status_continued(a,b)
#define forkee_status_continued(a,b)
#define validate_status_signaled(a,b,c)
#define forkee_status_signaled(a,b,c)
#define validate_status_stopped(a,b)
#define forkee_status_stopped(a,b)
#endif

#if defined(TWAIT_HAVE_PID)
/* This function is currently designed to be run in the main/parent process */
static void
await_zombie(pid_t process)
{
#if defined(__NetBSD__)
	struct kinfo_proc2 p;
	size_t len = sizeof(p);

	const int name[] = {
		[0] = CTL_KERN,
		[1] = KERN_PROC2,
		[2] = KERN_PROC_PID,
		[3] = process,
		[4] = sizeof(p),
		[5] = 1
	};

	const size_t namelen = __arraycount(name);

	/* Await the process becoming a zombie */
	while(1) {
		ATF_REQUIRE(sysctl(name, namelen, &p, &len, NULL, 0) == 0);

		if (p.p_stat == LSZOMB)
			break;

		ATF_REQUIRE(usleep(1000) == 0);
	}
#elif defined(__FreeBSD__)
	struct kinfo_proc p;
	size_t len = sizeof(p);

	const int name[] = {
		[0] = CTL_KERN,
		[1] = KERN_PROC,
		[2] = KERN_PROC_PID,
		[3] = process
	};

	const size_t namelen = __arraycount(name);

	/* Await the process becoming a zombie */
	while(1) {
		if(sysctl(name, namelen, &p, &len, NULL, 0) == -1) {
			if (errno == ESRCH)
				break;
			err(EXIT_FAILURE, "sysctl");
		}

		ATF_REQUIRE(usleep(1000) == 0);
	}
#elif defined(linux)
	int iszombie = 0;
	char pbuf[60];
	snprintf(pbuf, sizeof(pbuf), "/proc/%d/stat", (int)process);

	/* Detect that tracee is zombie */
	while(1) {
		FILE* fpstat = fopen(pbuf, "r");
		if (!fpstat) {
			err(EXIT_FAILURE, "fopen");
		};
		int rpid =0; char rcmd[32]; char rstatc = 0;
		fscanf(fpstat, "%d %30s %c", &rpid, rcmd, &rstatc);
		iszombie = rstatc == 'Z';

		fclose(fpstat);
		if (iszombie == 1) {
			break;
		}

		usleep(1000);
	}
#endif
}
#endif

int
main(int argc, char **argv)
{
	int fds_totracee[2], fds_fromtracee[2];
	int rv;
	const int exitval_tracee = 5;
	pid_t tracee, wpid;
	uint8_t msg = 0xde; /* dummy message for IPC based on pipe(2) */
#if defined(TWAIT_HAVE_STATUS)
	int status;
#endif

	printf("Spawn tracee\n");
	ATF_REQUIRE(pipe(fds_totracee) == 0);
	ATF_REQUIRE(pipe(fds_fromtracee) == 0);
	tracee = atf_utils_fork();
	if (tracee == 0) {
		FORKEE_ASSERT(close(fds_totracee[1]) == 0);
		FORKEE_ASSERT(close(fds_fromtracee[0]) == 0);

		/* Wait for message from the parent */
		rv = read(fds_totracee[0], &msg, sizeof(msg));
		FORKEE_ASSERT(rv == sizeof(msg));

		/* Wait for message from the parent */
		rv = write(fds_fromtracee[1], &msg, sizeof(msg));
		FORKEE_ASSERT(rv == sizeof(msg));

		printf("ccc!\n");fflush(stdout);

		_exit(exitval_tracee);
	}
	ATF_REQUIRE(close(fds_totracee[0]) == 0);
	ATF_REQUIRE(close(fds_fromtracee[1]) == 0);

	printf("Wait for the tracee to become ready\n");
	rv = write(fds_totracee[1], &msg, sizeof(msg));
	ATF_REQUIRE(rv == sizeof(msg));

	printf("Before calling PT_ATTACH for tracee %d\n", tracee);
	ATF_REQUIRE(ptrace(PT_ATTACH, tracee, NULL, 0) != -1);

	printf("Wait for the stopped tracee process with %s()\n",
	    TWAIT_FNAME);
	TWAIT_REQUIRE_SUCCESS(
	    wpid = TWAIT_GENERIC(tracee, &status, 0), tracee);

#ifdef __NetBSD__
	validate_status_stopped(status, SIGTRAP);
#else
	validate_status_stopped(status, SIGSTOP);
#endif

	printf("Resume tracee with PT_CONTINUE\n");
	ATF_REQUIRE(ptrace(PT_CONTINUE, tracee, (void *)1, 0) != -1);

	printf("Let the tracee exit now\n");
	rv = read(fds_fromtracee[0], &msg, sizeof(msg));
	ATF_REQUIRE(rv == sizeof(msg));

	printf("Wait for tracee to exit with %s()\n", TWAIT_FNAME);
	TWAIT_REQUIRE_SUCCESS(
	    wpid = TWAIT_GENERIC(tracee, &status, 0), tracee);

	validate_status_exited(status, exitval_tracee);

	printf("Before calling %s() for tracee\n", TWAIT_FNAME);
	wpid = TWAIT_GENERIC(tracee, &status, 0);
	assert(wpid == -1);
	assert(errno == ECHILD);

	printf("fds_fromtracee is no longer needed - close it\n");
	ATF_REQUIRE(close(fds_fromtracee[0]) == 0);

	printf("fds_totracee is no longer needed - close it\n");
	ATF_REQUIRE(close(fds_totracee[1]) == 0);
}

>Fix:
N/A



Home | Main Index | Thread Index | Old Index