Date: Wed, 19 Dec 2012 23:59:48 +0000 (UTC) From: Pawel Jakub Dawidek <pjd@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r244452 - head/sys/kern Message-ID: <201212192359.qBJNxmpa016496@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: pjd Date: Wed Dec 19 23:59:48 2012 New Revision: 244452 URL: http://svnweb.freebsd.org/changeset/base/244452 Log: Replace expand_name() function with corefile_open() function, which not only returns name, but also vnode of corefile to use. This simplifies the code and closes few races, especially in %I handling. Reviewed by: kib Obtained from: WHEEL Systems Modified: head/sys/kern/kern_sig.c Modified: head/sys/kern/kern_sig.c ============================================================================== --- head/sys/kern/kern_sig.c Wed Dec 19 23:40:02 2012 (r244451) +++ head/sys/kern/kern_sig.c Wed Dec 19 23:59:48 2012 (r244452) @@ -106,7 +106,6 @@ SDT_PROBE_ARGTYPE(proc, kernel, , signal SDT_PROBE_ARGTYPE(proc, kernel, , signal_discard, 2, "int"); static int coredump(struct thread *); -static char *expand_name(const char *, uid_t, pid_t, struct thread *, int); static int killpg1(struct thread *td, int sig, int pgid, int all, ksiginfo_t *ksi); static int issignal(struct thread *td, int stop_allowed); @@ -3034,8 +3033,9 @@ SYSCTL_STRING(_kern, OID_AUTO, corefile, sizeof(corefilename), "Process corefile name format string"); /* - * expand_name(name, uid, pid, td, compress) - * Expand the name described in corefilename, using name, uid, and pid. + * corefile_open(comm, uid, pid, td, compress, vpp, namep) + * Expand the name described in corefilename, using name, uid, and pid + * and open/create core file. * corefilename is a printf-like string, with three format specifiers: * %N name of process ("name") * %P process id (pid) @@ -3044,15 +3044,15 @@ SYSCTL_STRING(_kern, OID_AUTO, corefile, * by using "/dev/null", or all core files can be stored in "/cores/%U/%N-%P". * This is controlled by the sysctl variable kern.corefile (see above). */ -static char * -expand_name(const char *comm, uid_t uid, pid_t pid, struct thread *td, - int compress) +static int +corefile_open(const char *comm, uid_t uid, pid_t pid, struct thread *td, + int compress, struct vnode **vpp, char **namep) { + struct nameidata nd; struct sbuf sb; const char *format; - char *name; - int i, indexpos; - char *hostname; + char *hostname, *name; + int indexpos, i, error, cmode, flags, oflags; hostname = NULL; format = corefilename; @@ -3110,11 +3110,14 @@ expand_name(const char *comm, uid_t uid, "long\n", (long)pid, comm, (u_long)uid); sbuf_delete(&sb); free(name, M_TEMP); - return (NULL); + return (ENOMEM); } sbuf_finish(&sb); sbuf_delete(&sb); + cmode = S_IRUSR | S_IWUSR; + oflags = VN_OPEN_NOAUDIT | (capmode_coredump ? VN_OPEN_NOCAPCHECK : 0); + /* * If the core format has a %I in it, then we need to check * for existing corefiles before returning a name. @@ -3122,18 +3125,10 @@ expand_name(const char *comm, uid_t uid, * non-existing core file name to use. */ if (indexpos != -1) { - struct nameidata nd; - int cmode, flags, oflags, error; - - cmode = S_IRUSR | S_IWUSR; - oflags = VN_OPEN_NOAUDIT | - (capmode_coredump ? VN_OPEN_NOCAPCHECK : 0); - for (i = 0; i < num_cores; i++) { flags = O_CREAT | O_EXCL | FWRITE | O_NOFOLLOW; name[indexpos] = '0' + i; - NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, - name, td); + NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name, td); error = vn_open_cred(&nd, &flags, cmode, oflags, td->td_ucred, NULL); if (error) { @@ -3143,25 +3138,26 @@ expand_name(const char *comm, uid_t uid, "pid %d (%s), uid (%u): Path `%s' failed " "on initial open test, error = %d\n", pid, comm, uid, name, error); - free(name, M_TEMP); - return (NULL); - } - NDFREE(&nd, NDF_ONLY_PNBUF); - VOP_UNLOCK(nd.ni_vp, 0); - error = vn_close(nd.ni_vp, FWRITE, td->td_ucred, td); - if (error) { - log(LOG_ERR, - "pid %d (%s), uid (%u): Path `%s' failed " - "on close after initial open test, " - "error = %d\n", - pid, comm, uid, name, error); - free(name, M_TEMP); - return (NULL); } - break; + goto out; } } - return (name); + + flags = O_CREAT | FWRITE | O_NOFOLLOW; + NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name, td); + error = vn_open_cred(&nd, &flags, cmode, oflags, td->td_ucred, NULL); +out: + if (error) { +#ifdef AUDIT + audit_proc_coredump(td, name, error); +#endif + free(name, M_TEMP); + return (error); + } + NDFREE(&nd, NDF_ONLY_PNBUF); + *vpp = nd.ni_vp; + *namep = name; + return (0); } /* @@ -3179,9 +3175,8 @@ coredump(struct thread *td) struct ucred *cred = td->td_ucred; struct vnode *vp; struct flock lf; - struct nameidata nd; struct vattr vattr; - int error, error1, flags, locked; + int error, error1, locked; struct mount *mp; char *name; /* name of corefile */ off_t limit; @@ -3216,25 +3211,11 @@ coredump(struct thread *td) } PROC_UNLOCK(p); - name = expand_name(p->p_comm, cred->cr_uid, p->p_pid, td, compress); - if (name == NULL) - return (EINVAL); - restart: - NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name, td); - flags = O_CREAT | FWRITE | O_NOFOLLOW; - error = vn_open_cred(&nd, &flags, S_IRUSR | S_IWUSR, - VN_OPEN_NOAUDIT | (capmode_coredump ? VN_OPEN_NOCAPCHECK : 0), - cred, NULL); - if (error) { -#ifdef AUDIT - audit_proc_coredump(td, name, error); -#endif - free(name, M_TEMP); + error = corefile_open(p->p_comm, cred->cr_uid, p->p_pid, td, compress, + &vp, &name); + if (error != 0) return (error); - } - NDFREE(&nd, NDF_ONLY_PNBUF); - vp = nd.ni_vp; /* Don't dump to non-regular files or files with links. */ if (vp->v_type != VREG || VOP_GETATTR(vp, &vattr, cred) != 0 ||
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201212192359.qBJNxmpa016496>