Subject: Re: kern/6224: patch to add syslog'ing of core dumps and core dump failures
To: NetBSD Bugs and PR posting List <netbsd-bugs@NetBSD.ORG>
From: Greg A. Woods <woods@weird.com>
List: netbsd-bugs
Date: 04/07/2000 21:57:34
[ On , February 6, 2000 at 07:34:13 (-0000), fair@netbsd.org wrote: ]
> Subject: Re: kern/6224
>
> State-Changed-From-To: analyzed-feedback
> State-Changed-By: fair
> State-Changed-When: Sat Feb 5 23:31:47 PST 2000
> State-Changed-Why: 
> A patch to implement the requested functionality has been commited, however,
> it differs a bit from the request; it's a bit more spare in its reporting,
> it includes the signal number (the original patch did not), there were no
> changes to the error number returns in the coredump routines, and a new sysctl
> was added to turn this behavior on or off: kern.logsigexit
> 
> Hopefully, all of this answers this PR.

Unfortunately "spareness" in the reporting is exactly contrary to the
requirements that I was trying to fulfil.  While I'm not tied to the
exact format of the process credentials (the group-id isn't really
necessary I suppose and if the real and effective IDs are the same they
could be reported only once -- I was just copying FreeBSD's format at
the time), both the effective and real-uid are indeed necessary to meet
my needs (though I suppose one could argue that particular need could be
met by turning on process accounting -- I just don't want to tie those
features together as if they have anything to do with each other,
because they don't).

The finer control over the level of logging is also necessary to meet my
needs and though it should eliminate the need for a master on/off sysctl
I did maintain compatability with Eric's new kern.logsigexit.

On the other hand even though I don't have any real need to know the
signal number that caused the core dump (attempt), I've also kept it in
the format of the message.

Here's my updated patch that now does something much closer to the
requirements that instigated my initial attempt at offering this
functionality (thanks in no small part to the new kern/vfs_getcwd.c!).

A real-life example of the new kernel message now looks like this:

emacs: pid 25769 [eid 1000:10, rid 1000:10] sent signal 11: dumped core [in /most/big1/home/most/woods/emacs.core]

(btw, that's from a real nasty bug in emacs-20.5 folks -- hopefully
fixed in 20.6, which I'm working on updating to right now....)

This is against a syssrc tree sup'ed Mon Mar 20 09:47:15 EST 2000:

Index: sys/kern/kern_sig.c
===================================================================
RCS file: /cvs/NetBSD/src/sys/kern/kern_sig.c,v
retrieving revision 1.1.1.5
diff -c -r1.1.1.5 kern_sig.c
*** sys/kern/kern_sig.c	2000/03/21 00:58:50	1.1.1.5
--- sys/kern/kern_sig.c	2000/03/30 21:38:10
***************
*** 79,85 ****
  #include <uvm/uvm_extern.h>
  
  void stop __P((struct proc *p));
! void killproc __P((struct proc *, char *));
  static int build_corename __P((char *));
  #if COMPAT_NETBSD32
  static int coredump32 __P((struct proc *, struct vnode *));
--- 79,85 ----
  #include <uvm/uvm_extern.h>
  
  void stop __P((struct proc *p));
! 
  static int build_corename __P((char *));
  #if COMPAT_NETBSD32
  static int coredump32 __P((struct proc *, struct vnode *));
***************
*** 87,92 ****
--- 87,93 ----
  sigset_t contsigmask, stopsigmask, sigcantmask;
  
  struct pool sigacts_pool;	/* memory pool for sigacts structures */
+ static char corename[MAXPATHLEN];	/* the name of a core file */
  
  /*
   * Can process p, with pcred pc, send the signal signum to process q?
***************
*** 1224,1242 ****
  	struct proc *p;
  	char *why;
  {
! 
! 	log(LOG_ERR, "pid %d was killed: %s\n", p->p_pid, why);
! 	uprintf("sorry, pid %d was killed: %s\n", p->p_pid, why);
  	psignal(p, SIGKILL);
  }
  
  /*
!  * Force the current process to exit with the specified signal, dumping core
!  * if appropriate.  We bypass the normal tests for masked and caught signals,
   * allowing unrecoverable failures to terminate the process without changing
!  * signal state.  Mark the accounting record with the signal termination.
!  * If dumping core, save the signal number for the debugger.  Calls exit and
!  * does not return.
   */
  
  #if defined(DEBUG)
--- 1225,1253 ----
  	struct proc *p;
  	char *why;
  {
! 	/*
! 	 * XXX The FreeBSD dudes protected deref of p_cred and p_ucred, but I
! 	 * don't see any other code that does so.
! 	 */
! 	log(LOG_ERR, "%s: pid %d [eid %d:%d, rid %d:%d] was killed: %s\n",
! 	    p->p_comm,
! 	    p->p_pid,
! 	    p->p_ucred->cr_uid,
! 	    p->p_ucred->cr_gid,
! 	    p->p_cred->p_ruid == p->p_ucred->cr_uid ? p->p_cred->p_svuid : p->p_cred->p_ruid,
! 	    p->p_cred->p_rgid == p->p_ucred->cr_gid ? p->p_cred->p_svgid : p->p_cred->p_rgid,
! 	    why);
! 	uprintf("sorry, pid %d (%s) was killed: %s\n", p->p_pid, p->p_comm, why);
  	psignal(p, SIGKILL);
  }
  
  /*
!  * Force the current process to exit with the specified signal, dumping core if
!  * appropriate.  We bypass the normal tests for masked and caught signals,
   * allowing unrecoverable failures to terminate the process without changing
!  * signal state.  Mark the accounting record with the signal termination.  If
!  * dumping core, save the signal number for the debugger.  Logs what happened
!  * to the core dump.  Calls exit and does not return.
   */
  
  #if defined(DEBUG)
***************
*** 1245,1277 ****
  int	kern_logsigexit = 0;	/* not static to make public for sysctl */
  #endif
  
- static	char	*logcoredump =
- 	"pid %d (%s), uid %d: exited on signal %d (core dumped)\n";
- static	char	*lognocoredump =
- 	"pid %d (%s), uid %d: exited on signal %d (core not dumped, err = %d)\n";
- 
  void
  sigexit(p, signum)
  	register struct proc *p;
  	int signum;
  {
  	int	error;
- 	char	*errmsg;
  
  	p->p_acflag |= AXSIG;
  	if (sigprop[signum] & SA_CORE) {
  		p->p_sigacts->ps_sig = signum;
! 		if ((error = coredump(p)) == 0) {
  			signum |= WCOREFLAG;
! 			errmsg = logcoredump;
! 		} else {
! 			errmsg = lognocoredump;
  		}
! 
! 		if (kern_logsigexit)
! 			log(LOG_INFO, errmsg, p->p_pid, p->p_comm,
! 			    p->p_cred && p->p_ucred ? p->p_ucred->cr_uid : -1,
! 			    signum &~ WCOREFLAG, error);
  	}
  
  	exit1(p, W_EXITCODE(0, signum));
--- 1256,1373 ----
  int	kern_logsigexit = 0;	/* not static to make public for sysctl */
  #endif
  
  void
  sigexit(p, signum)
  	register struct proc *p;
  	int signum;
  {
  	int	error;
  
  	p->p_acflag |= AXSIG;
  	if (sigprop[signum] & SA_CORE) {
+ 		char cdcwdbuf[MAXPATHLEN]; /* XXX too bad we don't have asprintf() in the kernel */
+ 		char *cdcwdbp;
+ 		char *cdwhy = "core dump attempted";
+ 		int cdlp = LOG_DEBUG;
+ 
+ 		cdcwdbp = &cdcwdbuf[MAXPATHLEN];
+ 		*(--cdcwdbp) = '\0';
  		p->p_sigacts->ps_sig = signum;
! 		/*
! 		 * XXX FIXME: to pass pointers to cdcwd, cdwhy, and cdlp into
! 		 * coredump() and let it set them as appropriate, right in the
! 		 * code where the message has meaning.
! 		 */
! 		if ((error = coredump(p)) == 0)
  			signum |= WCOREFLAG;
! 		if (*corename && *corename != '/') {
! 			/*
! 			 * getcwd_common() is a bit of a hack -- but it's
! 			 * better than nothing!  It comes from vfs_getcwd.c.
! 			 *
! 			 * The first arg is a pointer to the process' cwd vnode.
! 			 *
! 			 * The second arg is always a NULL vnode pointer.
! 			 *
! 			 * The name is placed in the *end* of the buffer which
! 			 * starts at the pointer passed in the 4th arg.  The
! 			 * pointer passed in by reference in the 3rd arg has to
! 			 * point at the end of the buffer and it is is moved
! 			 * back to point at the beginning of the resulting
! 			 * pathname.  The buffer has to be NUL terminated(?).
! 			 *
! 			 * The 5th argument here is "max number of vnodes to
! 			 * traverse".  Since each entry takes up at least 2
! 			 * bytes in the output buffer, limit it to N/2 vnodes
! 			 * for an N byte buffer.
! 			 * 
! 			 * The 6th arg are flags (always 0 here).
! 			 *
! 			 * The 7th arg is a pointer to the process' proc
! 			 * structure.
! 			 */
! 			if (getcwd_common(p->p_cwdi->cwdi_cdir, (struct vnode *) NULL, &cdcwdbp, cdcwdbuf, sizeof(cdcwdbuf)/2, 0, p))
! 				cdcwdbp = "[error getting process cwd]";
  		}
! 		switch (error) {
! 		case 0:
! 			cdwhy = "dumped core";
! 			cdlp = LOG_DEBUG;
! 			break;
! 		case EAUTH:
! 			cdwhy = "was set-id, core dump not permitted";
! 			cdlp = LOG_ERR;
! 			break;
! 		case EFBIG:
! 			cdwhy = "core dump would exceed rlimit";
! 			cdlp = LOG_NOTICE;
! 			break;
! 		case EINVAL:
! 			cdwhy = "core dump not permitted over non-regular file";
! 			cdlp = LOG_WARNING;
! 			break;
! 		case ENXIO:
! 			cdwhy = "filesystem mount flag prevented core dump";
! 			cdlp = LOG_NOTICE;
! 			break;
! 		case EPERM:
! 			cdwhy = "core dump not permitted";
! 			cdlp = LOG_WARNING;
! 			break;
! 		default:
! 			/* too bad strerror() isn't in the kernel.... */
! 			cdwhy = "core dump failed";
! 			cdlp = LOG_NOTICE;
! 			break;
! 		}
! 		/*
! 		 * XXX The FreeBSD dudes protected the dereference of p_cred
! 		 * and p_ucred, but I don't see any other code that does so.
! 		 */
! 		if (kern_logsigexit) {
! 			log(cdlp, "%s: pid %d [eid %d:%d, rid %d:%d] sent signal %d: %s [in %s%s%s]\n",
! 			    p->p_comm,
! 			    p->p_pid,
! 			    p->p_ucred->cr_uid,
! 			    p->p_ucred->cr_gid,
! 			    p->p_cred->p_ruid == p->p_ucred->cr_uid ? p->p_cred->p_svuid : p->p_cred->p_ruid,
! 			    p->p_cred->p_rgid == p->p_ucred->cr_gid ? p->p_cred->p_svgid : p->p_cred->p_rgid,
! 			    signum & ~WCOREFLAG, /* XXX too bad strsignal() isn't in the kernel.... */
! 			    cdwhy,
! 			    (error != 0 || (*corename && *corename != '/')) ? cdcwdbp : "",
! 			    (error == 0) ? "/" : "",
! 			    (error == 0) ? corename : "");
! 		}
! 		if (cdlp != LOG_DEBUG) { /* tell the user of abnormal events */
! 			uprintf("%s: pid %d sent signal %d: %s [in %s%s%s]\n",
! 				p->p_comm,
! 				p->p_pid,
! 				signum & ~WCOREFLAG, /* XXX strsignal() */
! 				cdwhy,
! 				(error != 0 || (*corename && *corename != '/')) ? cdcwdbp : "",
! 				(error == 0) ? "/" : "",
! 				(error == 0) ? corename : "");
! 		}
  	}
  
  	exit1(p, W_EXITCODE(0, signum));
***************
*** 1279,1286 ****
  }
  
  /*
!  * Dump core, into a file named "progname.core" or "core" (depending on the 
!  * value of shortcorename), unless the process was setuid/setgid.
   */
  int
  coredump(p)
--- 1375,1381 ----
  }
  
  /*
!  * Dump core, unless the process was setuid/setgid.
   */
  int
  coredump(p)
***************
*** 1292,1305 ****
  	struct nameidata nd;
  	struct vattr vattr;
  	int error, error1;
- 	char name[MAXPATHLEN];
  	struct core core;
  
  	/*
  	 * Make sure the process has not set-id, to prevent data leaks.
  	 */
  	if (p->p_flag & P_SUGID)
! 		return (EPERM);
  
  	/*
  	 * Refuse to core if the data + stack + user size is larger than
--- 1387,1400 ----
  	struct nameidata nd;
  	struct vattr vattr;
  	int error, error1;
  	struct core core;
  
+ 	*corename = '\0';
  	/*
  	 * Make sure the process has not set-id, to prevent data leaks.
  	 */
  	if (p->p_flag & P_SUGID)
! 		return (EAUTH);		/* XXX better error code? */
  
  	/*
  	 * Refuse to core if the data + stack + user size is larger than
***************
*** 1318,1330 ****
  	vp = p->p_cwdi->cwdi_cdir;
  	if (vp->v_mount == NULL ||
  	    (vp->v_mount->mnt_flag & MNT_NOCOREDUMP) != 0)
! 		return (EPERM);
  
! 	error = build_corename(name);
  	if (error)
  		return error;
  
! 	NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name, p);
  	error = vn_open(&nd, O_CREAT | FWRITE | FNOSYMLINK, S_IRUSR | S_IWUSR);
  	if (error)
  		return (error);
--- 1413,1425 ----
  	vp = p->p_cwdi->cwdi_cdir;
  	if (vp->v_mount == NULL ||
  	    (vp->v_mount->mnt_flag & MNT_NOCOREDUMP) != 0)
! 		return (ENXIO);		/* XXX is there a better error code? */
  
! 	error = build_corename(corename);
  	if (error)
  		return error;
  
! 	NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, corename, p);
  	error = vn_open(&nd, O_CREAT | FWRITE | FNOSYMLINK, S_IRUSR | S_IWUSR);
  	if (error)
  		return (error);
***************
*** 1505,1510 ****
--- 1600,1614 ----
  	return (ENOSYS);
  }
  
+ /*
+  * Generate an appropriate name for the core file.  This will normally be
+  * derived by expanding the value of the process' desired corename, which is
+  * normally the value of the system-wide kern.defcorename sysctl() variable.
+  * The core name can be changed per-process by setting the proc.PID.corename
+  * sysctl() variable (this value is inherited by child processes).  The
+  * corename may be an absolute pathname such as "/var/tmp/%u/%n.core" or just a
+  * plain filename such as "%n.core".
+  */
  static int
  build_corename(dst)
  	char *dst;
***************
*** 1513,1537 ****
  	char *d;
  	int len, i;
  
! 	for (s = curproc->p_limit->pl_corename, len = 0, d = dst;
! 	    *s != '\0'; s++) {
  		if (*s == '%') {
  			switch (*(s+1)) {
  			case 'n':
! 				i = snprintf(d,MAXPATHLEN - 1 - len, "%s",
! 				    curproc->p_comm);
  				break;
  			case 'p':
  				i = snprintf(d, MAXPATHLEN - 1 - len, "%d",
! 				    curproc->p_pid);
  				break;
  			case 'u':
  				i = snprintf(d, MAXPATHLEN - 1 - len, "%s",
! 				    curproc->p_pgrp->pg_session->s_login);
  				break;
  			case 't':
  				i = snprintf(d, MAXPATHLEN - 1 - len, "%ld",
! 				    curproc->p_stats->p_start.tv_sec);
  				break;
  			default:
  				goto copy;
--- 1617,1640 ----
  	char *d;
  	int len, i;
  
! 	for (s = curproc->p_limit->pl_corename, len = 0, d = dst; *s != '\0'; s++) {
  		if (*s == '%') {
  			switch (*(s+1)) {
  			case 'n':
! 				i = snprintf(d, MAXPATHLEN - 1 - len, "%s",
! 					     curproc->p_comm);
  				break;
  			case 'p':
  				i = snprintf(d, MAXPATHLEN - 1 - len, "%d",
! 					     curproc->p_pid);
  				break;
  			case 'u':
  				i = snprintf(d, MAXPATHLEN - 1 - len, "%s",
! 					     curproc->p_pgrp->pg_session->s_login);
  				break;
  			case 't':
  				i = snprintf(d, MAXPATHLEN - 1 - len, "%ld",
! 					     curproc->p_stats->p_start.tv_sec);
  				break;
  			default:
  				goto copy;
Index: sys/kern/vfs_getcwd.c
===================================================================
RCS file: /cvs/NetBSD/src/sys/kern/vfs_getcwd.c,v
retrieving revision 1.1.1.1
diff -c -r1.1.1.1 vfs_getcwd.c
*** sys/kern/vfs_getcwd.c	2000/03/21 00:59:11	1.1.1.1
--- sys/kern/vfs_getcwd.c	2000/03/30 03:30:58
***************
*** 59,67 ****
  static int
  getcwd_getcache __P((struct vnode **, struct vnode **,
      char **, char *));
- int
- getcwd_common __P((struct vnode *, struct vnode *,
- 		   char **, char *, int, int, struct proc *));
  
  #define DIRENT_MINSIZE (sizeof(struct dirent) - (MAXNAMLEN+1) + 4)
  
--- 59,64 ----
Index: sys/sys/signalvar.h
===================================================================
RCS file: /cvs/NetBSD/src/sys/sys/signalvar.h,v
retrieving revision 1.1.1.4
diff -c -r1.1.1.4 signalvar.h
*** sys/sys/signalvar.h	2000/03/21 01:02:48	1.1.1.4
--- sys/sys/signalvar.h	2000/03/21 04:02:38
***************
*** 150,155 ****
--- 150,156 ----
  void	psignal __P((struct proc *p, int sig));
  void	siginit __P((struct proc *p));
  void	trapsignal __P((struct proc *p, int sig, u_long code));
+ void	killproc __P((struct proc *, char *));
  void	sigexit __P((struct proc *, int));
  void	setsigvec __P((struct proc *, int, struct sigaction *));
  int	killpg1 __P((struct proc *, int, int, int));
Index: sys/sys/vnode.h
===================================================================
RCS file: /cvs/NetBSD/src/sys/sys/vnode.h,v
retrieving revision 1.1.1.5
diff -c -r1.1.1.5 vnode.h
*** sys/sys/vnode.h	2000/03/21 01:02:57	1.1.1.5
--- sys/sys/vnode.h	2000/03/30 03:33:18
***************
*** 536,541 ****
--- 536,546 ----
  void 	vrele __P((struct vnode *vp));
  int	vaccess __P((enum vtype type, mode_t file_mode, uid_t uid, gid_t gid,
  		     mode_t acc_mode, struct ucred *cred));
+ 
+ /* sys/kern/vfs_getcwd.c */
+ int	getcwd_common __P((struct vnode *, struct vnode *,
+ 			   char **, char *, int, int, struct proc *));
+ 
  #endif /* _KERNEL */
  
  #endif /* !_SYS_VNODE_H_ */


-- 
							Greg A. Woods

+1 416 218-0098      VE3TCP      <gwoods@acm.org>      <robohack!woods>
Planix, Inc. <woods@planix.com>; Secrets of the Weird <woods@weird.com>