[PATCH v2 4/6] ntfs: do not map a vcn as a hole when its runlist lookup failed

[Matthias Goergens]

ntfs_attr_vcn_to_rl() retries ntfs_map_runlist_nolock() for any lcn up
to LCN_RL_NOT_MAPPED, which includes LCN_ENOENT, but turns a failed
retry into an error only for LCN_RL_NOT_MAPPED.  For LCN_ENOENT the
error is dropped and the read path maps the range as a hole.

An LCN_ENOENT below allocated_size comes from a base extent with a
highest_vcn of 0, which ntfs_mapping_pairs_decompress() takes to map the
whole attribute, so the runlist ends after its last mapping pair.  If
the pairs end early, the retry finds the same extent and fails with
-ENOENT.  On a crafted volume with 4 KiB clusters, a 64-cluster file
whose mapping pairs stop after 16 clusters reads 48 clusters of zeros,
with no error.

The same layout gets a crafted $MFT past the check from "ntfs: fail the
mount when $MFT needs its own extent records".  With 512-byte clusters
and $MFT's mapping pairs ending at vcn 4, an unpatched kernel hangs on
the folio lock reading records 0-3.  With the check alone, the -EIO is
dropped, records 2 and 3 read as zeros and the mount carries on until
check_mft_mirror() finds the zeroed record 2.

Fail the lookup whenever the retry leaves @vcn unmapped, -ENOENT
included.  At or beyond allocated_size nothing is mapped, so do not
retry there: the runlist ends with LCN_ENOENT, or with LCN_RL_NOT_MAPPED
when only the last extent is mapped, as after a write into it, and with
clusters smaller than a page every read of a file's last folio looks up
such vcns.

A failed expansion in ntfs_non_resident_attr_expand() or
ntfs_attrlist_repack() truncates the runlist under the runlist lock but
restores allocated_size only after dropping it.  A lookup in between
would now fail, so restore allocated_size under the lock in both.

The crafted file now fails from vcn 16 on with -EIO, and the crafted
volume fails to mount with the check's message.

Fixes: 495e90fa3348 ("ntfs: update attrib operations")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Matthias Goergens <matthias.goergens@xxxxxxxxx>
---
 fs/ntfs/attrib.c   | 38 +++++++++++++++++++++++++++++++-------
 fs/ntfs/attrlist.c |  8 ++++++--
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index eab4d8d32132f..30d3d2eb5ef3c 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -344,6 +344,23 @@ struct runlist_element *ntfs_attr_vcn_to_rl(struct ntfs_inode *ni, s64 vcn, s64
 		rl++;
 	*lcn = ntfs_rl_vcn_to_lcn(rl, vcn);
 
+	/*
+	 * Nothing is mapped at or beyond the allocated size: the runlist ends
+	 * there with LCN_ENOENT, or with LCN_RL_NOT_MAPPED if only a later
+	 * extent has been mapped.  Return that end as it is.  Below the
+	 * allocated size, an unmapped vcn is worth a retry.
+	 */
+	if (*lcn <= LCN_RL_NOT_MAPPED && !is_retry) {
+		unsigned long flags;
+		s64 allocated_vcn;
+
+		read_lock_irqsave(&ni->size_lock, flags);
+		allocated_vcn = ntfs_bytes_to_cluster(ni->vol, ni->allocated_size);
+		read_unlock_irqrestore(&ni->size_lock, flags);
+		if (vcn >= allocated_vcn)
+			return rl;
+	}
+
 	if (*lcn <= LCN_RL_NOT_MAPPED && is_retry == false) {
 		is_retry = true;
 		err = ntfs_map_runlist_nolock(ni, vcn, NULL);
@@ -354,11 +371,14 @@ struct runlist_element *ntfs_attr_vcn_to_rl(struct ntfs_inode *ni, s64 vcn, s64
 	}
 
 	/*
-	 * The runlist fragment containing @vcn could not be mapped, e.g.
-	 * because the extent mft record holding it is corrupt.  Do not hand
-	 * LCN_RL_NOT_MAPPED back to callers, which would treat it as a hole.
+	 * Neither the runlist nor the retry mapped @vcn, which lies below the
+	 * allocated size, e.g. because the extent mft record holding it is
+	 * corrupt or because the mapping pairs end too soon.
+	 * ntfs_map_runlist_nolock() reports the latter as -ENOENT, as @vcn
+	 * lies past the extent it found.  Callers would treat
+	 * LCN_RL_NOT_MAPPED or LCN_ENOENT here as a hole, so fail instead.
 	 */
-	if (*lcn == LCN_RL_NOT_MAPPED)
+	if (*lcn <= LCN_RL_NOT_MAPPED)
 		return ERR_PTR(err == -ENOMEM ? -ENOMEM : -EIO);
 
 	return rl;
@@ -4703,11 +4723,17 @@ static int ntfs_non_resident_attr_expand(struct ntfs_inode *ni, const s64 newsiz
 	if (err2)
 		ntfs_debug("Leaking clusters");
 
-	/* Now, truncate the runlist itself. */
+	/*
+	 * Now, truncate the runlist itself.  Restore allocated_size before
+	 * dropping the lock: ntfs_attr_vcn_to_rl() fails a lookup below the
+	 * allocated size that falls past the end of the runlist.
+	 */
 	if (ni != locked_ni)
 		down_write(&ni->runlist.lock);
 	err2 = ntfs_rl_truncate_nolock(vol, &ni->runlist,
 			ntfs_bytes_to_cluster(vol, org_alloc_size));
+	if (!err2)
+		ni->allocated_size = org_alloc_size;
 	if (ni != locked_ni)
 		up_write(&ni->runlist.lock);
 	if (err2) {
@@ -4719,8 +4745,6 @@ static int ntfs_non_resident_attr_expand(struct ntfs_inode *ni, const s64 newsiz
 		ni->runlist.rl = NULL;
 		ntfs_error(sb, "Couldn't truncate runlist. Rollback failed");
 	} else {
-		/* Prepare to mapping pairs update. */
-		ni->allocated_size = org_alloc_size;
 		/* Restore mapping pairs. */
 		if (ni != locked_ni)
 			down_read(&ni->runlist.lock);
diff --git a/fs/ntfs/attrlist.c b/fs/ntfs/attrlist.c
index 1bbd2bc62c582..3660e7fd24b13 100644
--- a/fs/ntfs/attrlist.c
+++ b/fs/ntfs/attrlist.c
@@ -168,14 +168,18 @@ static int ntfs_attrlist_repack(struct inode *attr_vi,
 	return 0;
 
 restore_old_runlist:
+	/*
+	 * Restore allocated_size before dropping the runlist lock:
+	 * ntfs_attr_vcn_to_rl() fails a lookup below the allocated size that
+	 * falls past the end of the runlist.
+	 */
 	down_write(&attr_ni->runlist.lock);
 	attr_ni->runlist.rl = old_rl;
 	attr_ni->runlist.count = old_rl_count;
-	up_write(&attr_ni->runlist.lock);
-
 	write_lock_irqsave(&attr_ni->size_lock, flags);
 	attr_ni->allocated_size = old_alloc_size;
 	write_unlock_irqrestore(&attr_ni->size_lock, flags);
+	up_write(&attr_ni->runlist.lock);
 
 	restore_err = ntfs_attr_update_mapping_pairs_locked(
 			attr_ni, 0, locked_ni);
-- 
2.55.0