[PATCH 1/1] udf: Fix incorrect final NOT_ALLOCATED (hole) extent length

From: Steve Magnani <steve.magnani@digidescorp.com>
To: Jan Kara <jack@suse.com>
Cc: linux-kernel@vger.kernel.org,
	"Steven J . Magnani" <steve@digidescorp.com>
Subject: [PATCH 1/1] udf: Fix incorrect final NOT_ALLOCATED (hole) extent length
Date: Tue,  4 Jun 2019 07:31:58 -0500	[thread overview]
Message-ID: <20190604123158.12741-2-steve@digidescorp.com> (raw)
In-Reply-To: <20190604123158.12741-1-steve@digidescorp.com>

In some cases, using the 'truncate' command to extend a UDF file results
in a mismatch between the length of the file's extents (specifically, due
to incorrect length of the final NOT_ALLOCATED extent) and the information
(file) length. The discrepancy can prevent other operating systems
(i.e., Windows 10) from opening the file.

Two particular errors have been observed when extending a file:

1. The final extent is larger than it should be, having been rounded up
   to a multiple of the block size.

B. The final extent is not shorter than it should be, due to not having
   been updated when the file's information length was increased.

The first case could represent a design error, if coded intentionally
due to a misinterpretation of scantily-documented ECMA-167 "file tail"
rules. The standard specifies that the tail, if present, consists of
a sequence of "unrecorded and allocated" extents (only).

Signed-off-by: Steven J. Magnani <steve@digidescorp.com>

--- a/fs/udf/inode.c	2019-05-24 21:17:33.659704533 -0500
+++ b/fs/udf/inode.c	2019-05-29 20:32:23.730129419 -0500
@@ -474,7 +474,8 @@ static struct buffer_head *udf_getblk(st
 static int udf_do_extend_file(struct inode *inode,
 			      struct extent_position *last_pos,
 			      struct kernel_long_ad *last_ext,
-			      sector_t blocks)
+			      sector_t blocks,
+			      unsigned long partial_final_block)
 {
 	sector_t add;
 	int count = 0, fake = !(last_ext->extLength & UDF_EXTENT_LENGTH_MASK);
@@ -486,7 +487,7 @@ static int udf_do_extend_file(struct ino
 
 	/* The previous extent is fake and we should not extend by anything
 	 * - there's nothing to do... */
-	if (!blocks && fake)
+	if (!blocks && !partial_final_block && fake)
 		return 0;
 
 	iinfo = UDF_I(inode);
@@ -524,6 +525,10 @@ static int udf_do_extend_file(struct ino
 			add = blocks;
 		blocks -= add;
 		last_ext->extLength += add << sb->s_blocksize_bits;
+		if (blocks == 0 && partial_final_block) {
+			last_ext->extLength -= sb->s_blocksize
+				- partial_final_block;
+		}
 	}
 
 	if (fake) {
@@ -566,6 +571,10 @@ static int udf_do_extend_file(struct ino
 	if (blocks) {
 		last_ext->extLength = EXT_NOT_RECORDED_NOT_ALLOCATED |
 			(blocks << sb->s_blocksize_bits);
+		if (partial_final_block) {
+			last_ext->extLength -= sb->s_blocksize
+					- partial_final_block;
+		}
 		err = udf_add_aext(inode, last_pos, &last_ext->extLocation,
 				   last_ext->extLength, 1);
 		if (err)
@@ -605,6 +614,7 @@ static int udf_extend_file(struct inode
 	int8_t etype;
 	struct super_block *sb = inode->i_sb;
 	sector_t first_block = newsize >> sb->s_blocksize_bits, offset;
+	unsigned long partial_final_block;
 	int adsize;
 	struct udf_inode_info *iinfo = UDF_I(inode);
 	struct kernel_long_ad extent;
@@ -619,15 +629,17 @@ static int udf_extend_file(struct inode
 
 	etype = inode_bmap(inode, first_block, &epos, &eloc, &elen, &offset);
 
+	partial_final_block = newsize & (sb->s_blocksize - 1);
+
 	/* File has extent covering the new size (could happen when extending
 	 * inside a block)? */
-	if (etype != -1)
-		return 0;
-	if (newsize & (sb->s_blocksize - 1))
-		offset++;
-	/* Extended file just to the boundary of the last file block? */
-	if (offset == 0)
-		return 0;
+	if (etype == -1) {
+		if (partial_final_block)
+			offset++;
+	} else {
+		/* Extending file within the last file block */
+		offset = 0;  /* Don't add any new blocks */
+	}
 
 	/* Truncate is extending the file by 'offset' blocks */
 	if ((!epos.bh && epos.offset == udf_file_entry_alloc_offset(inode)) ||
@@ -643,7 +655,8 @@ static int udf_extend_file(struct inode
 				      &extent.extLength, 0);
 		extent.extLength |= etype << 30;
 	}
-	err = udf_do_extend_file(inode, &epos, &extent, offset);
+	err = udf_do_extend_file(inode, &epos, &extent, offset,
+				 partial_final_block);
 	if (err < 0)
 		goto out;
 	err = 0;
@@ -760,7 +773,7 @@ static sector_t inode_getblk(struct inod
 			startnum = (offset > 0);
 		}
 		/* Create extents for the hole between EOF and offset */
-		ret = udf_do_extend_file(inode, &prev_epos, laarr, offset);
+		ret = udf_do_extend_file(inode, &prev_epos, laarr, offset, 0);
 		if (ret < 0) {
 			*err = ret;
 			newblock = 0;
