From owner-svn-src-head@FreeBSD.ORG Wed Jun 6 16:30: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 3EFED106566C; Wed, 6 Jun 2012 16:30:17 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 29CF78FC15; Wed, 6 Jun 2012 16:30: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 q56GUHmG025326; Wed, 6 Jun 2012 16:30:17 GMT (envelope-from kib@svn.freebsd.org) Received: (from kib@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id q56GUGH4025324; Wed, 6 Jun 2012 16:30:16 GMT (envelope-from kib@svn.freebsd.org) Message-Id: <201206061630.q56GUGH4025324@svn.freebsd.org> From: Konstantin Belousov Date: Wed, 6 Jun 2012 16:30: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: r236687 - head/sys/fs/nfsclient 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: Wed, 06 Jun 2012 16:30:17 -0000 Author: kib Date: Wed Jun 6 16:30:16 2012 New Revision: 236687 URL: http://svn.freebsd.org/changeset/base/236687 Log: Improve handling of uiomove(9) errors for the NFS client. Do not brelse() the buffer unconditionally with BIO_ERROR set if uiomove() failed. The brelse() treats most buffers with BIO_ERROR as B_INVAL, dropping their content. Instead, if the write request covered the whole buffer, remember the cached state and brelse() with BIO_ERROR set only if the buffer was not cached previously. Update the buffer dirtyoff/dirtyend based on the progress recorded by uiomove() in passed struct uio, even in the presence of error. Otherwise, usermode could see changed data in the backed pages, but later the buffer is destroyed without write-back. If uiomove() failed for IO_UNIT request, try to truncate the vnode back to the pre-write state, and rewind the progress in passed uio accordingly, following the FFS behaviour. Reviewed by: rmacklem (some time ago) Tested by: pho MFC after: 1 month Modified: head/sys/fs/nfsclient/nfs_clbio.c Modified: head/sys/fs/nfsclient/nfs_clbio.c ============================================================================== --- head/sys/fs/nfsclient/nfs_clbio.c Wed Jun 6 16:26:55 2012 (r236686) +++ head/sys/fs/nfsclient/nfs_clbio.c Wed Jun 6 16:30:16 2012 (r236687) @@ -897,8 +897,9 @@ ncl_write(struct vop_write_args *ap) struct nfsmount *nmp = VFSTONFS(vp->v_mount); daddr_t lbn; int bcount; - int n, on, error = 0; - off_t tmp_off; + int bp_cached, n, on, error = 0; + size_t orig_resid, local_resid; + off_t orig_size, tmp_off; KASSERT(uio->uio_rw == UIO_WRITE, ("ncl_write mode")); KASSERT(uio->uio_segflg != UIO_USERSPACE || uio->uio_td == curthread, @@ -950,6 +951,11 @@ flush_and_restart: mtx_unlock(&np->n_mtx); } + orig_resid = uio->uio_resid; + mtx_lock(&np->n_mtx); + orig_size = np->n_size; + mtx_unlock(&np->n_mtx); + /* * If IO_APPEND then load uio_offset. We restart here if we cannot * get the append lock. @@ -1127,7 +1133,10 @@ again: * normally. */ + bp_cached = 1; if (on == 0 && n == bcount) { + if ((bp->b_flags & B_CACHE) == 0) + bp_cached = 0; bp->b_flags |= B_CACHE; bp->b_flags &= ~B_INVAL; bp->b_ioflags &= ~BIO_ERROR; @@ -1193,8 +1202,24 @@ again: goto again; } + local_resid = uio->uio_resid; error = uiomove((char *)bp->b_data + on, n, uio); + if (error != 0 && !bp_cached) { + /* + * This block has no other content then what + * possibly was written by the faulty uiomove. + * Release it, forgetting the data pages, to + * prevent the leak of uninitialized data to + * usermode. + */ + bp->b_ioflags |= BIO_ERROR; + brelse(bp); + uio->uio_offset -= local_resid - uio->uio_resid; + uio->uio_resid = local_resid; + break; + } + /* * Since this block is being modified, it must be written * again and not just committed. Since write clustering does @@ -1203,17 +1228,18 @@ again: */ bp->b_flags &= ~(B_NEEDCOMMIT | B_CLUSTEROK); - if (error) { - bp->b_ioflags |= BIO_ERROR; - brelse(bp); - break; - } + /* + * Get the partial update on the progress made from + * uiomove, if an error occured. + */ + if (error != 0) + n = local_resid - uio->uio_resid; /* * Only update dirtyoff/dirtyend if not a degenerate * condition. */ - if (n) { + if (n > 0) { if (bp->b_dirtyend > 0) { bp->b_dirtyoff = min(on, bp->b_dirtyoff); bp->b_dirtyend = max((on + n), bp->b_dirtyend); @@ -1242,8 +1268,22 @@ again: } else { bdwrite(bp); } + + if (error != 0) + break; } while (uio->uio_resid > 0 && n > 0); + if (error != 0) { + if (ioflag & IO_UNIT) { + VATTR_NULL(&vattr); + vattr.va_size = orig_size; + /* IO_SYNC is handled implicitely */ + (void)VOP_SETATTR(vp, &vattr, cred); + uio->uio_offset -= orig_resid - uio->uio_resid; + uio->uio_resid = orig_resid; + } + } + return (error); }