From owner-svn-src-head@FreeBSD.ORG Thu Feb 9 22:34:17 2012 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 40B931065672; Thu, 9 Feb 2012 22:34:17 +0000 (UTC) (envelope-from mckusick@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 1526F8FC13; Thu, 9 Feb 2012 22:34:17 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id q19MYGFr040873; Thu, 9 Feb 2012 22:34:16 GMT (envelope-from mckusick@svn.freebsd.org) Received: (from mckusick@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id q19MYGEJ040871; Thu, 9 Feb 2012 22:34:16 GMT (envelope-from mckusick@svn.freebsd.org) Message-Id: <201202092234.q19MYGEJ040871@svn.freebsd.org> From: Kirk McKusick Date: Thu, 9 Feb 2012 22:34:16 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r231313 - head/sys/ufs/ffs X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Feb 2012 22:34:17 -0000 Author: mckusick Date: Thu Feb 9 22:34:16 2012 New Revision: 231313 URL: http://svn.freebsd.org/changeset/base/231313 Log: Historically when an application wrote an entire block of a file, the kernel allocated a buffer but did not zero it as it was about to be completely filled by a uiomove() from the user's buffer. However, if the uiomove() failed, the old contents of the buffer could be exposed especially if the file was being mmap'ed. The fix was to always zero the buffer when it was allocated. This change first attempts the uiomove() to the newly allocated (and dirty) buffer and only zeros it if the uiomove() fails. The effect is to eliminate the gratuitous zeroing of the buffer in the usual case where the uiomove() successfully fills it. Reviewed by: kib Tested by: scottl MFC after: 2 weeks (to 9 only) Modified: head/sys/ufs/ffs/ffs_vnops.c Modified: head/sys/ufs/ffs/ffs_vnops.c ============================================================================== --- head/sys/ufs/ffs/ffs_vnops.c Thu Feb 9 22:17:13 2012 (r231312) +++ head/sys/ufs/ffs/ffs_vnops.c Thu Feb 9 22:34:16 2012 (r231313) @@ -718,15 +718,6 @@ ffs_write(ap) vnode_pager_setsize(vp, ip->i_size); break; } - /* - * If the buffer is not valid we have to clear out any - * garbage data from the pages instantiated for the buffer. - * If we do not, a failed uiomove() during a write can leave - * the prior contents of the pages exposed to a userland - * mmap(). XXX deal with uiomove() errors a better way. - */ - if ((bp->b_flags & B_CACHE) == 0 && fs->fs_bsize <= xfersize) - vfs_bio_clrbuf(bp); if (ioflag & IO_DIRECT) bp->b_flags |= B_DIRECT; if ((ioflag & (IO_SYNC|IO_INVAL)) == (IO_SYNC|IO_INVAL)) @@ -743,6 +734,26 @@ ffs_write(ap) error = uiomove((char *)bp->b_data + blkoffset, (int)xfersize, uio); + /* + * If the buffer is not already filled and we encounter an + * error while trying to fill it, we have to clear out any + * garbage data from the pages instantiated for the buffer. + * If we do not, a failed uiomove() during a write can leave + * the prior contents of the pages exposed to a userland mmap. + * + * Note that we need only clear buffers with a transfer size + * equal to the block size because buffers with a shorter + * transfer size were cleared above by the call to UFS_BALLOC() + * with the BA_CLRBUF flag set. + * + * If the source region for uiomove identically mmaps the + * buffer, uiomove() performed the NOP copy, and the buffer + * content remains valid because the page fault handler + * validated the pages. + */ + if (error != 0 && (bp->b_flags & B_CACHE) == 0 && + fs->fs_bsize == xfersize) + vfs_bio_clrbuf(bp); if ((ioflag & (IO_VMIO|IO_DIRECT)) && (LIST_EMPTY(&bp->b_dep))) { bp->b_flags |= B_RELBUF;