From 9f15e7b3660a0698d6bb068e277796ac37efa93c Mon Sep 17 00:00:00 2001
From: David van Moolenbroek <david@minix3.org>
Date: Mon, 31 Aug 2015 12:01:26 +0000
Subject: [PATCH] VFS: tweak coredump wrapper code

Change-Id: I6c1f50910d906b25f6df2b48f2cbfb899850924e
---
 minix/servers/vfs/main.c |  8 ++++++++
 minix/servers/vfs/misc.c | 29 +++++++++++++++++------------
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/minix/servers/vfs/main.c b/minix/servers/vfs/main.c
index 44fd1d451..adbb85ce7 100644
--- a/minix/servers/vfs/main.c
+++ b/minix/servers/vfs/main.c
@@ -620,6 +620,14 @@ void service_pm_postponed(void)
 	term_signal = job_m_in.VFS_PM_TERM_SIG;
 	core_path = (vir_bytes) job_m_in.VFS_PM_PATH;
 
+	/* A zero signal used to indicate that a coredump should be generated
+	 * without terminating the target process, but this was broken in so
+	 * many ways that we no longer support this. Userland should implement
+	 * this functionality itself, for example through ptrace(2).
+	 */
+	if (term_signal == 0)
+		panic("no termination signal given for coredump!");
+
 	assert(proc_e == fp->fp_endpoint);
 
 	r = pm_dumpcore(term_signal, core_path);
diff --git a/minix/servers/vfs/misc.c b/minix/servers/vfs/misc.c
index 876686a9b..8a88fa863 100644
--- a/minix/servers/vfs/misc.c
+++ b/minix/servers/vfs/misc.c
@@ -879,36 +879,41 @@ int do_svrctl(void)
  *===========================================================================*/
 int pm_dumpcore(int csig, vir_bytes exe_name)
 {
-  int r = OK, core_fd;
+  int r, core_fd;
   struct filp *f;
   char core_path[PATH_MAX];
   char proc_name[PROC_NAME_LEN];
 
-  /* if a process is blocked, fp->fp_fd holds the fd it's blocked on.
-   * free it up for use by common_open().
+  /* If a process is blocked, fp->fp_fd holds the fd it's blocked on. Free it
+   * up for use by common_open(). This step is the reason we cannot use this
+   * function to generate a core dump of a process while it is still running
+   * (i.e., without terminating it), as it changes the state of the process.
    */
   if (fp_is_blocked(fp))
           unpause();
 
   /* open core file */
   snprintf(core_path, PATH_MAX, "%s.%d", CORE_NAME, fp->fp_pid);
-  core_fd = common_open(core_path, O_WRONLY | O_CREAT | O_TRUNC, CORE_MODE);
-  if (core_fd < 0) { r = core_fd; goto core_exit; }
+  r = core_fd = common_open(core_path, O_WRONLY | O_CREAT | O_TRUNC,
+	CORE_MODE);
+  if (r < 0) goto core_exit;
 
-  /* get process' name */
-  r = sys_datacopy_wrapper(PM_PROC_NR, exe_name, VFS_PROC_NR, (vir_bytes) proc_name,
-			PROC_NAME_LEN);
+  /* get process name */
+  r = sys_datacopy_wrapper(PM_PROC_NR, exe_name, VFS_PROC_NR,
+	(vir_bytes) proc_name, PROC_NAME_LEN);
   if (r != OK) goto core_exit;
   proc_name[PROC_NAME_LEN - 1] = '\0';
 
-  if ((f = get_filp(core_fd, VNODE_WRITE)) == NULL) { r=EBADF; goto core_exit; }
+  /* write the core dump */
+  f = get_filp(core_fd, VNODE_WRITE);
+  assert(f != NULL);
   write_elf_core_file(f, csig, proc_name);
   unlock_filp(f);
-  (void) close_fd(fp, core_fd);	        /* ignore failure, we're exiting anyway */
 
 core_exit:
-  if(csig)
-	  free_proc(FP_EXITING);
+  /* The core file descriptor will be closed as part of the process exit. */
+  free_proc(FP_EXITING);
+
   return(r);
 }