kern/45393: core dumps are unilaterally prevented by unmounted cwd or MNT_NOCOREDUMP even if corename will be valid

["Greg A. Woods" <woods%planix.com@localhost>]

>Number:         45393
>Category:       kern
>Synopsis:       core dumps are unilaterally prevented by unmounted cwd or 
>MNT_NOCOREDUMP even if corename will be valid
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Sep 22 22:10:01 +0000 2011
>Originator:     Greg A. Woods
>Release:        netbsd-5
>Organization:
Planix, Inc.; Kelowna, BC; Canada
>Environment:
System: NetBSD 5.2_STABLE
Architecture: 
Machine: 
>Description:

        A question emailed to me about one of the validations done in
        coredump() prompted me to note a potential problem which would
        prevent core dumps from happening in a number of circumstances
        even if those checks were not valid for the ultimate destination
        of the core file.  The conundrum was already mentioned in part
        in a comment in the code, but nothing had been done about it
        then.

>How-To-Repeat:

        by reading the code, and comments

>Fix:

        The following diff is from the netbsd-5 branch and also includes
        some un-related changes to the coredump() API.

        Instead of trying to edit the diff, or trying to re-apply just
        the necessary changes to -current where I cannot test them at
        the moment, I'll just show the whole diff here.

        The changes relevant to this PR are in the 5'th and 6'th hunks.

        The second hunk also updates the opening comment to better
        reflect the current state of the code, something that should
        have been done quite some time ago when the code was first
        changed to make it incompatible with the comment.

--- kern_core.c 24 Apr 2008 11:39:23 -0700      1.12
+++ kern_core.c 22 Sep 2011 11:16:56 -0700      
@@ -55,7 +55,7 @@
 #if !defined(COREDUMP)
 
 int
-coredump(struct lwp *l, const char *pattern)
+coredump(struct lwp *l, const char *pattern, char *corename)
 {
 
        return (ENOSYS);
@@ -73,11 +73,14 @@
 static int     coredump_buildname(struct proc *, char *, const char *, size_t);
 
 /*
- * Dump core, into a file named "progname.core" or "core" (depending on the
- * value of shortcorename), unless the process was setuid/setgid.
+ * Dump core, into a file name created from 'pattern' (unless the process was
+ * setuid/setgid, in which case if security_setidcore_dump is also true then
+ * security_setidcore_path is used as the file name pattern).
+ *
+ * XXX maybe logging should go here instead of all callers? (need better API)
  */
 int
-coredump(struct lwp *l, const char *pattern)
+coredump(struct lwp *l, const char *pattern, char *corename)
 {
        struct vnode            *vp;
        struct proc             *p;
@@ -91,6 +94,10 @@
        char                    *name;
 
        name = PNBUF_GET();
+       if (name == NULL) {
+               error = ENOMEM;
+               goto done;
+       }
 
        p = l->l_proc;
        vm = p->p_vmspace;
@@ -105,7 +112,7 @@
         */
        if (USPACE + ctob(vm->vm_dsize + vm->vm_ssize) >=
            p->p_rlimit[RLIMIT_CORE].rlim_cur) {
-               error = EFBIG;          /* better error code? */
+               error = EFBIG;
                mutex_exit(p->p_lock);
                mutex_exit(proc_lock);
                goto done;
@@ -119,35 +126,17 @@
        cred = p->p_cred;
 
        /*
-        * The core dump will go in the current working directory.  Make
-        * sure that the directory is still there and that the mount flags
-        * allow us to write core dumps there.
-        *
-        * XXX: this is partially bogus, it should be checking the directory
-        * into which the file is actually written - which probably needs
-        * a flag on namei()
-        */
-       vp = p->p_cwdi->cwdi_cdir;
-       if (vp->v_mount == NULL ||
-           (vp->v_mount->mnt_flag & MNT_NOCOREDUMP) != 0) {
-               error = EPERM;
-               mutex_exit(p->p_lock);
-               mutex_exit(proc_lock);
-               goto done;
-       }
-
-       /*
         * Make sure the process has not set-id, to prevent data leaks,
         * unless it was specifically requested to allow set-id coredumps.
         */
        if (p->p_flag & PK_SUGID) {
                if (!security_setidcore_dump) {
-                       error = EPERM;
+                       error = EAUTH;
                        mutex_exit(p->p_lock);
                        mutex_exit(proc_lock);
                        goto done;
                }
-               pattern = security_setidcore_path;
+               pattern = security_setidcore_path; /* is permitted to be NULL */
        }
 
        /* It is (just) possible for p_limit and pl_corename to change */
@@ -155,12 +144,44 @@
        mutex_enter(&lim->pl_lock);
        if (pattern == NULL)
                pattern = lim->pl_corename;
+       KASSERT(pattern);               /* otherwise name is "", and what will 
vn_open() do? */
        error = coredump_buildname(p, name, pattern, MAXPATHLEN);
        mutex_exit(&lim->pl_lock);
+
+       /*
+        * IFF the final name for the core file is relative to the current
+        * working directory, then make sure that the directory is still there
+        * and that the mount flags allow us to write core dumps there.
+        *
+        * Note that there is no check to see if the partition where a fully
+        * qualified core name will be created allows core dumps or not
+        * (i.e. there is no check to make sure that it does not have the
+        * MNT_NOCOREDUMP flag set).
+        *
+        * Presumably the administrator who set the pathname in
+        * lim->pl_corename, or who specified the path in the ptrace() call,
+        * wants to ignore the MNT_NOCOREDUMP flag in this case, and is
+        * authorized to do so.
+        *
+        * This allows core dumps to generally be prevented on a system with
+        * only a single filesystem, unless they are explicitly directed to a
+        * specified location.
+        */
+       if (name[0] != '/') {
+               vp = p->p_cwdi->cwdi_cdir;
+               if (vp->v_mount == NULL ||
+                   (vp->v_mount->mnt_flag & MNT_NOCOREDUMP) != 0) {
+                       error = ENXIO;
+                       mutex_exit(p->p_lock);
+                       mutex_exit(proc_lock);
+                       goto done;
+               }
+       }
        mutex_exit(p->p_lock);
        mutex_exit(proc_lock);
        if (error)
                goto done;
+
        NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name);
        if ((error = vn_open(&nd, O_CREAT | O_NOFOLLOW | FWRITE,
            S_IRUSR | S_IWUSR)) != 0)
@@ -198,8 +219,11 @@
        if (error == 0)
                error = error1;
 done:
-       if (name != NULL)
+       if (name != NULL) {
+               if (corename)
+                       strcpy(corename, name);
                PNBUF_PUT(name);
+       }
        return error;
 }