Date: Mon, 26 Sep 2005 22:02:05 GMT From: Robert Watson <rwatson@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 84314 for review Message-ID: <200509262202.j8QM25uU032745@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=84314 Change 84314 by rwatson@rwatson_zoo on 2005/09/26 22:01:05 Be more careful about buffer length handling in canon_path(): use strlcpy(), snprintf() to copy and combine strings. This appears to fix some memory corruption problems seen on SMP under high load during large numbers of name space operations. Affected files ... .. //depot/projects/trustedbsd/audit3/sys/security/audit/kern_bsm_klib.c#12 edit Differences ... ==== //depot/projects/trustedbsd/audit3/sys/security/audit/kern_bsm_klib.c#12 (text+ko) ==== @@ -400,16 +400,22 @@ * to obtain the root directoty, but this results in a volfs name * written to the audit log. So we will leave the filename starting * with '/' in the audit log in this case. + * + * XXXRW: Since we combine two paths here, ideally a buffer of size + * MAXPATHLEN * 2 would be passed in. */ void canon_path(struct thread *td, char *path, char *cpath) { + char *bufp; char *retbuf, *freebuf; +#if 0 int len; +#endif struct vnode *vnp; struct filedesc *fdp; - int vfslocked; + int error, vfslocked; fdp = td->td_proc->p_fd; bufp = path; @@ -436,9 +442,6 @@ FILEDESC_UNLOCK(fdp); if (vnp != NULL) { /* - * XXX: Should lock vnode! - */ - /* * XXX: vn_fullpath() on FreeBSD is "less reliable" * than vn_getpath() on Darwin, so this will need more * attention in the future. Also, the question and @@ -447,19 +450,17 @@ */ vfslocked = VFS_LOCK_GIANT(vnp->v_mount); vn_lock(vnp, LK_EXCLUSIVE | LK_RETRY, td); - if (vn_fullpath(td, vnp, &retbuf, &freebuf) == 0) { + error = vn_fullpath(td, vnp, &retbuf, &freebuf); + if (error == 0) { /* Copy and free buffer allocated by vn_fullpath() */ - strlcpy(cpath, retbuf, MAXPATHLEN); + snprintf(cpath, MAXPATHLEN, "%s/%s", retbuf, bufp); free(freebuf, M_TEMP); - } - else { + } else { cpath[0] = '\0'; } vput(vnp); VFS_UNLOCK_GIANT(vfslocked); - len = strlen(cpath); - strncpy(cpath + len-1, bufp, MAXPATHLEN - len); } else { - strncpy(cpath, bufp, MAXPATHLEN); + strlcpy(cpath, bufp, MAXPATHLEN); } }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200509262202.j8QM25uU032745>