Merge tag 'xfs-4.20-fixes-2' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux

Pull xfs fixes from Darrick Wong: "Dave and I have continued our work fixing corruption problems that can be found when running long-term burn-in exercisers on xfs. Here are some patches fixing most of the problems, but there will likely be more. :/ - Numerous corruption fixes for copy on write - Numerous corruption fixes for blocksize < pagesize writes - Don't miscalculate AG reservations for small final AGs - Fix page cache truncation to work properly for reflink and extent shifting - Fix use-after-free when retrying failed inode/dquot buffer logging - Fix corruptions seen when using copy_file_range in directio mode" * tag 'xfs-4.20-fixes-2' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux: iomap: readpages doesn't zero page tail beyond EOF vfs: vfs_dedupe_file_range() doesn't return EOPNOTSUPP iomap: dio data corruption and spurious errors when pipes fill iomap: sub-block dio needs to zeroout beyond EOF iomap: FUA is wrong for DIO O_DSYNC writes into unwritten extents xfs: delalloc -> unwritten COW fork allocation can go wrong xfs: flush removing page cache in xfs_reflink_remap_prep xfs: extent shifting doesn't fully invalidate page cache xfs: finobt AG reserves don't consider last AG can be a runt xfs: fix transient reference count error in xfs_buf_resubmit_failed_buffers xfs: uncached buffer tracing needs to print bno xfs: make xfs_file_remap_range() static xfs: fix shared extent data corruption due to missing cow reservation

Linus Torvalds committed 7 years ago

Commit abe72ff2 parents 7c98a42 8c110d410 files · +104 −46Browse files

Unified · Split

10 files

fs/iomap.c +41 −12
fs/read_write.c +7 −8
fs/xfs/libxfs/xfs_bmap.c +4 −1
fs/xfs/libxfs/xfs_ialloc_btree.c +7 −4
fs/xfs/xfs_bmap_util.c +2 −8
fs/xfs/xfs_bmap_util.h +3 −0
fs/xfs/xfs_buf_item.c +21 −7
fs/xfs/xfs_file.c +1 −1
fs/xfs/xfs_reflink.c +14 −4
fs/xfs/xfs_trace.h +4 −1

fs/iomap.c+41−12

 iomap_adjust_read_range(struct inode *inode, struct iomap_page *iop,
 		loff_t *pos, loff_t length, unsigned *offp, unsigned *lenp)
 {
+	loff_t orig_pos = *pos;
+	loff_t isize = i_size_read(inode);
 	unsigned block_bits = inode->i_blkbits;
 	unsigned block_size = (1 << block_bits);
 	unsigned poff = offset_in_page(*pos);
 	unsigned plen = min_t(loff_t, PAGE_SIZE - poff, length);
 	unsigned first = poff >> block_bits;
 	unsigned last = (poff + plen - 1) >> block_bits;
-	unsigned end = offset_in_page(i_size_read(inode)) >> block_bits;
 	/*
 	 * If the block size is smaller than the page size we need to check the
 	 * handle both halves separately so that we properly zero data in the
 	 * page cache for blocks that are entirely outside of i_size.
 	 */
-	if (first <= end && last > end)
+	if (orig_pos <= isize && orig_pos + length > isize) {
-		plen -= (last - end) * block_size;
+		unsigned end = offset_in_page(isize - 1) >> block_bits;
+		if (first <= end && last > end)
+			plen -= (last - end) * block_size;
+	}
 	*offp = poff;
 	*lenp = plen;
 	struct bio *bio;
 	bool need_zeroout = false;
 	bool use_fua = false;
-	int nr_pages, ret;
+	int nr_pages, ret = 0;
 	size_t copied = 0;
 	if ((pos | length | align) & ((1 << blkbits) - 1))
 	if (iomap->flags & IOMAP_F_NEW) {
 		need_zeroout = true;
-	} else {
+	} else if (iomap->type == IOMAP_MAPPED) {
 		/*
-		 * Use a FUA write if we need datasync semantics, this
+		 * Use a FUA write if we need datasync semantics, this is a pure
-		 * is a pure data IO that doesn't require any metadata
+		 * data IO that doesn't require any metadata updates (including
-		 * updates and the underlying device supports FUA. This
+		 * after IO completion such as unwritten extent conversion) and
-		 * allows us to avoid cache flushes on IO completion.
+		 * the underlying device supports FUA. This allows us to avoid
+		 * cache flushes on IO completion.
 		 */
 		if (!(iomap->flags & (IOMAP_F_SHARED|IOMAP_F_DIRTY)) &&
 		    (dio->flags & IOMAP_DIO_WRITE_FUA) &&
 		ret = bio_iov_iter_get_pages(bio, &iter);
 		if (unlikely(ret)) {
+			/*
+			 * We have to stop part way through an IO. We must fall
+			 * through to the sub-block tail zeroing here, otherwise
+			 * this short IO may expose stale data in the tail of
+			 * the block we haven't written data to.
+			 */
 			bio_put(bio);
-			return copied ? copied : ret;
+			goto zero_tail;
 		}
 		n = bio->bi_iter.bi_size;
 		dio->submit.cookie = submit_bio(bio);
 	} while (nr_pages);
-	if (need_zeroout) {
+	/*
+	 * We need to zeroout the tail of a sub-block write if the extent type
+	 * requires zeroing or the write extends beyond EOF. If we don't zero
+	 * the block tail in the latter case, we can expose stale data via mmap
+	 * reads of the EOF block.
+	 */
+zero_tail:
+	if (need_zeroout ||
+	    ((dio->flags & IOMAP_DIO_WRITE) && pos >= i_size_read(inode))) {
 		/* zero out from the end of the write to the end of the block */
 		pad = pos & (fs_block_size - 1);
 		if (pad)
 			iomap_dio_zero(dio, iomap, pos, fs_block_size - pad);
 	}
-	return copied;
+	return copied ? copied : ret;
 }
 static loff_t
 				dio->wait_for_completion = true;
 				ret = 0;
 			}
+			/*
+			 * Splicing to pipes can fail on a full pipe. We have to
+			 * swallow this to make it look like a short IO
+			 * otherwise the higher splice layers will completely
+			 * mishandle the error and stop moving data.
+			 */
+			if (ret == -EFAULT)
+				ret = 0;
 			break;
 		}
 		pos += ret;

fs/read_write.c+7−8

 	off = same->src_offset;
 	len = same->src_length;
-	ret = -EISDIR;
 	if (S_ISDIR(src->i_mode))
-		goto out;
+		return -EISDIR;
-	ret = -EINVAL;
 	if (!S_ISREG(src->i_mode))
-		goto out;
+		return -EINVAL;
+	if (!file->f_op->remap_file_range)
+		return -EOPNOTSUPP;
 	ret = remap_verify_area(file, off, len, false);
 	if (ret < 0)
-		goto out;
+		return ret;
 	ret = 0;
 	if (off + len > i_size_read(src))
 		fdput(dst_fd);
 next_loop:
 		if (fatal_signal_pending(current))
-			goto out;
+			break;
 	}
-out:
 	return ret;
 }
 EXPORT_SYMBOL(vfs_dedupe_file_range);

fs/xfs/libxfs/xfs_bmap.c+4−1

 	case BMAP_LEFT_FILLING | BMAP_RIGHT_FILLING | BMAP_RIGHT_CONTIG:
 		/*
 		 * Filling in all of a previously delayed allocation extent.
-		 * The right neighbor is contiguous, the left is not.
+		 * The right neighbor is contiguous, the left is not. Take care
+		 * with delay -> unwritten extent allocation here because the
+		 * delalloc record we are overwriting is always written.
 		 */
 		PREV.br_startblock = new->br_startblock;
 		PREV.br_blockcount += RIGHT.br_blockcount;
+		PREV.br_state = new->br_state;
 		xfs_iext_next(ifp, &bma->icur);
 		xfs_iext_remove(bma->ip, &bma->icur, state);

fs/xfs/libxfs/xfs_ialloc_btree.c+7−4

 static xfs_extlen_t
 xfs_inobt_max_size(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	xfs_agnumber_t		agno)
 {
+	xfs_agblock_t		agblocks = xfs_ag_block_count(mp, agno);
 	/* Bail out if we're uninitialized, which can happen in mkfs. */
 	if (mp->m_inobt_mxr[0] == 0)
 		return 0;
 	return xfs_btree_calc_size(mp->m_inobt_mnr,
-		(uint64_t)mp->m_sb.sb_agblocks * mp->m_sb.sb_inopblock /
+				(uint64_t)agblocks * mp->m_sb.sb_inopblock /
-				XFS_INODES_PER_CHUNK);
+					XFS_INODES_PER_CHUNK);
 }
 static int
 	if (error)
 		return error;
-	*ask += xfs_inobt_max_size(mp);
+	*ask += xfs_inobt_max_size(mp, agno);
 	*used += tree_len;
 	return 0;
 }

fs/xfs/xfs_bmap_util.c+2−8

 	goto out_unlock;
 }
-static int
+int
 xfs_flush_unmap_range(
 	struct xfs_inode	*ip,
 	xfs_off_t		offset,
 	 * Writeback and invalidate cache for the remainder of the file as we're
 	 * about to shift down every extent from offset to EOF.
 	 */
-	error = filemap_write_and_wait_range(VFS_I(ip)->i_mapping, offset, -1);
+	error = xfs_flush_unmap_range(ip, offset, XFS_ISIZE(ip));
-	if (error)
-		return error;
-	error = invalidate_inode_pages2_range(VFS_I(ip)->i_mapping,
-					offset >> PAGE_SHIFT, -1);
-	if (error)
-		return error;
 	/*
 	 * Clean out anything hanging around in the cow fork now that

fs/xfs/xfs_bmap_util.h+3−0

@@ -80,4 +80,7 @@ int xfs_bmap_count_blocks(struct xfs_trans tp, struct xfs_inode ip, · 79 unmodified lines
80	int whichfork, xfs_extnum_t *nextents,	80	int whichfork, xfs_extnum_t *nextents,
81	xfs_filblks_t *count);	81	xfs_filblks_t *count);
82		82
		83	int xfs_flush_unmap_range(struct xfs_inode *ip, xfs_off_t offset,
		84	xfs_off_t len);
		85
83	#endif /* __XFS_BMAP_UTIL_H__ */	86	#endif /* __XFS_BMAP_UTIL_H__ */

fs/xfs/xfs_buf_item.c+21−7

 }
 /*
- * Requeue a failed buffer for writeback
+ * Requeue a failed buffer for writeback.
  *
- * Return true if the buffer has been re-queued properly, false otherwise
+ * We clear the log item failed state here as well, but we have to be careful
+ * about reference counts because the only active reference counts on the buffer
+ * may be the failed log items. Hence if we clear the log item failed state
+ * before queuing the buffer for IO we can release all active references to
+ * the buffer and free it, leading to use after free problems in
+ * xfs_buf_delwri_queue. It makes no difference to the buffer or log items which
+ * order we process them in - the buffer is locked, and we own the buffer list
+ * so nothing on them is going to change while we are performing this action.
+ *
+ * Hence we can safely queue the buffer for IO before we clear the failed log
+ * item state, therefore  always having an active reference to the buffer and
+ * avoiding the transient zero-reference state that leads to use-after-free.
+ *
+ * Return true if the buffer was added to the buffer list, false if it was
+ * already on the buffer list.
  */
 bool
 xfs_buf_resubmit_failed_buffers(
 	struct list_head	*buffer_list)
 {
 	struct xfs_log_item	*lip;
+	bool			ret;
+	ret = xfs_buf_delwri_queue(bp, buffer_list);
 	/*
-	 * Clear XFS_LI_FAILED flag from all items before resubmit
+	 * XFS_LI_FAILED set/clear is protected by ail_lock, caller of this
-	 *
-	 * XFS_LI_FAILED set/clear is protected by ail_lock, caller  this
 	 * function already have it acquired
 	 */
 	list_for_each_entry(lip, &bp->b_li_list, li_bio_list)
 		xfs_clear_li_failed(lip);
-	/* Add this buffer back to the delayed write list */
+	return ret;
-	return xfs_buf_delwri_queue(bp, buffer_list);
 }

fs/xfs/xfs_file.c+1−1

@@ -920,7 +920,7 @@ out_unlock: · 919 unmodified lines
920	}	920	}
921		921
922		922
923	loff_t	923	STATIC loff_t
924	xfs_file_remap_range(	924	xfs_file_remap_range(
925	struct file *file_in,	925	struct file *file_in,
926	loff_t pos_in,	926	loff_t pos_in,

fs/xfs/xfs_reflink.c+14−4

 	if (error)
 		return error;
+	xfs_trim_extent(imap, got.br_startoff, got.br_blockcount);
 	trace_xfs_reflink_cow_alloc(ip, &got);
 	return 0;
 }
 	if (ret)
 		goto out_unlock;
-	/* Zap any page cache for the destination file's range. */
+	/*
-	truncate_inode_pages_range(&inode_out->i_data,
+	 * If pos_out > EOF, we may have dirtied blocks between EOF and
-			round_down(pos_out, PAGE_SIZE),
+	 * pos_out. In that case, we need to extend the flush and unmap to cover
-			round_up(pos_out + *len, PAGE_SIZE) - 1);
+	 * from EOF to the end of the copy length.
+	 */
+	if (pos_out > XFS_ISIZE(dest)) {
+		loff_t	flen = *len + (pos_out - XFS_ISIZE(dest));
+		ret = xfs_flush_unmap_range(dest, XFS_ISIZE(dest), flen);
+	} else {
+		ret = xfs_flush_unmap_range(dest, pos_out, *len);
+	}
+	if (ret)
+		goto out_unlock;
 	return 1;
 out_unlock:

fs/xfs/xfs_trace.h+4−1

 	),
 	TP_fast_assign(
 		__entry->dev = bp->b_target->bt_dev;
-		__entry->bno = bp->b_bn;
+		if (bp->b_bn == XFS_BUF_DADDR_NULL)
+			__entry->bno = bp->b_maps[0].bm_bn;
+		else
+			__entry->bno = bp->b_bn;
 		__entry->nblks = bp->b_length;
 		__entry->hold = atomic_read(&bp->b_hold);
 		__entry->pincount = atomic_read(&bp->b_pin_count);

Merge tag 'xfs-4.20-fixes-2' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux

Keyboard shortcuts

General

Issues and pull requests

Issues, pull requests and Inbox

Repository code