[PATCH 2/2] Fix possible leakage of blocks in UDF

[Jan Kara <jack@suse.cz>]

[-- Attachment #1: Type: text/plain, Size: 352 bytes --]

  Hello,

  attached is a patch that fixes possible leakage of free blocks / use of
free blocks in UDF (which spilled nice assertion failures I've added in my
first round of patches). More details in the changelog. Andrew, please apply.
Both changes have survived some time of fsx and fsstress testing so they
should be reasonably safe.

								Honza

[-- Attachment #2: udf-2.6.22-rc2-2-udf_block_leak.diff --]
[-- Type: text/x-patch, Size: 6795 bytes --]

It is wrong to call udf_discard_prealloc() from udf_clear_inode() as at that time
inode changes won't be written any more which can lead to leakage of blocks, use
of free blocks or improperly aligned extents. Also udf_discard_prealloc() does two
different things - it removes preallocated blocks and truncates the last extent to
exactly match i_size. We move the latter functionality to udf_truncate_tail_extent(),
call udf_discard_prealloc() when last reference to a file is dropped and call
udf_truncate_tail_extent() when inode is being removed from inode cach
(udf_drop_inode() call). We cannot call udf_truncate_tail_extent() earlier as
subsequent open+write would find the last block of the file mapped and happily write
to the end of it, although the last extent says it's shorter.

Signed-off-by: Jan Kara <jack@suse.cz>

diff -rupX /home/jack/.kerndiffexclude linux-2.6.22-rc2-1-udf_data_corruption/fs/udf/inode.c linux-2.6.22-rc2-2-udf_block_leak/fs/udf/inode.c
--- linux-2.6.22-rc2-1-udf_data_corruption/fs/udf/inode.c	2007-05-24 18:16:36.000000000 +0200
+++ linux-2.6.22-rc2-2-udf_block_leak/fs/udf/inode.c	2007-05-24 18:18:54.000000000 +0200
@@ -100,14 +100,18 @@ no_delete:
 	clear_inode(inode);
 }
 
-void udf_clear_inode(struct inode *inode)
+void udf_drop_inode(struct inode *inode)
 {
 	if (!(inode->i_sb->s_flags & MS_RDONLY)) {
 		lock_kernel();
-		udf_discard_prealloc(inode);
+		udf_truncate_tail_extent(inode);
 		unlock_kernel();
 	}
+	generic_drop_inode(inode);
+}
 
+void udf_clear_inode(struct inode *inode)
+{
 	kfree(UDF_I_DATA(inode));
 	UDF_I_DATA(inode) = NULL;
 }
diff -rupX /home/jack/.kerndiffexclude linux-2.6.22-rc2-1-udf_data_corruption/fs/udf/super.c linux-2.6.22-rc2-2-udf_block_leak/fs/udf/super.c
--- linux-2.6.22-rc2-1-udf_data_corruption/fs/udf/super.c	2007-05-24 18:00:05.000000000 +0200
+++ linux-2.6.22-rc2-2-udf_block_leak/fs/udf/super.c	2007-05-24 18:18:54.000000000 +0200
@@ -162,6 +162,7 @@ static const struct super_operations udf
 	.write_inode		= udf_write_inode,
 	.delete_inode		= udf_delete_inode,
 	.clear_inode		= udf_clear_inode,
+	.drop_inode		= udf_drop_inode,
 	.put_super		= udf_put_super,
 	.write_super		= udf_write_super,
 	.statfs			= udf_statfs,
diff -rupX /home/jack/.kerndiffexclude linux-2.6.22-rc2-1-udf_data_corruption/fs/udf/truncate.c linux-2.6.22-rc2-2-udf_block_leak/fs/udf/truncate.c
--- linux-2.6.22-rc2-1-udf_data_corruption/fs/udf/truncate.c	2007-05-24 18:00:05.000000000 +0200
+++ linux-2.6.22-rc2-2-udf_block_leak/fs/udf/truncate.c	2007-05-24 18:18:54.000000000 +0200
@@ -61,7 +61,11 @@ static void extent_trunc(struct inode * 
 	}
 }
 
-void udf_discard_prealloc(struct inode * inode)
+/*
+ * Truncate the last extent to match i_size. This function assumes
+ * that preallocation extent is already truncated.
+ */
+void udf_truncate_tail_extent(struct inode *inode)
 {
 	struct extent_position epos = { NULL, 0, {0, 0}};
 	kernel_lb_addr eloc;
@@ -71,7 +75,7 @@ void udf_discard_prealloc(struct inode *
 	int adsize;
 
 	if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_IN_ICB ||
-		inode->i_size == UDF_I_LENEXTENTS(inode))
+	    inode->i_size == UDF_I_LENEXTENTS(inode))
 		return;
 
 	if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_SHORT)
@@ -79,25 +83,63 @@ void udf_discard_prealloc(struct inode *
 	else if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_LONG)
 		adsize = sizeof(long_ad);
 	else
-		adsize = 0;
-
-	epos.block = UDF_I_LOCATION(inode);
+		BUG();
 
 	/* Find the last extent in the file */
 	while ((netype = udf_next_aext(inode, &epos, &eloc, &elen, 1)) != -1)
 	{
 		etype = netype;
 		lbcount += elen;
-		if (lbcount > inode->i_size && lbcount - elen < inode->i_size)
-		{
-			WARN_ON(lbcount - inode->i_size >= inode->i_sb->s_blocksize);
+		if (lbcount > inode->i_size) {
+			if (lbcount - inode->i_size >= inode->i_sb->s_blocksize)
+				printk(KERN_WARNING "udf_truncate_tail_extent():\
+ Too long extent after EOF in inode %u: i_size: %Ld lbcount: %Ld extent %u+%u\n",
+(unsigned)inode->i_ino, (long long)inode->i_size, (long long)lbcount,
+(unsigned)eloc.logicalBlockNum, (unsigned)elen);
 			nelen = elen - (lbcount - inode->i_size);
 			epos.offset -= adsize;
 			extent_trunc(inode, &epos, eloc, etype, elen, nelen);
 			epos.offset += adsize;
-			lbcount = inode->i_size;
+			if (udf_next_aext(inode, &epos, &eloc, &elen, 1) != -1)
+				printk(KERN_ERR "udf_truncate_tail_extent(): \
+Extent after EOF in inode %u.\n", (unsigned)inode->i_ino);
+			break;
 		}
 	}
+	/* This inode entry is in-memory only and thus we don't have to mark
+	 * the inode dirty */
+	UDF_I_LENEXTENTS(inode) = inode->i_size;
+	brelse(epos.bh);
+}
+
+void udf_discard_prealloc(struct inode * inode)
+{
+	struct extent_position epos = { NULL, 0, {0, 0}};
+	kernel_lb_addr eloc;
+	uint32_t elen;
+	uint64_t lbcount = 0;
+	int8_t etype = -1, netype;
+	int adsize;
+
+	if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_IN_ICB ||
+		inode->i_size == UDF_I_LENEXTENTS(inode))
+		return;
+
+	if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_SHORT)
+		adsize = sizeof(short_ad); 
+	else if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_LONG)
+		adsize = sizeof(long_ad);
+	else
+		adsize = 0;
+
+	epos.block = UDF_I_LOCATION(inode);
+
+	/* Find the last extent in the file */
+	while ((netype = udf_next_aext(inode, &epos, &eloc, &elen, 1)) != -1)
+	{
+		etype = netype;
+		lbcount += elen;
+	}
 	if (etype == (EXT_NOT_RECORDED_ALLOCATED >> 30)) {
 		epos.offset -= adsize;
 		lbcount -= elen;
@@ -118,9 +160,9 @@ void udf_discard_prealloc(struct inode *
 			mark_buffer_dirty_inode(epos.bh, inode);
 		}
 	}
+	/* This inode entry is in-memory only and thus we don't have to mark
+	 * the inode dirty */
 	UDF_I_LENEXTENTS(inode) = lbcount;
-
-	WARN_ON(lbcount != inode->i_size);
 	brelse(epos.bh);
 }
 
diff -rupX /home/jack/.kerndiffexclude linux-2.6.22-rc2-1-udf_data_corruption/fs/udf/udfdecl.h linux-2.6.22-rc2-2-udf_block_leak/fs/udf/udfdecl.h
--- linux-2.6.22-rc2-1-udf_data_corruption/fs/udf/udfdecl.h	2007-05-24 18:00:05.000000000 +0200
+++ linux-2.6.22-rc2-2-udf_block_leak/fs/udf/udfdecl.h	2007-05-24 18:18:54.000000000 +0200
@@ -103,6 +103,7 @@ extern struct buffer_head * udf_bread(st
 extern void udf_truncate(struct inode *);
 extern void udf_read_inode(struct inode *);
 extern void udf_delete_inode(struct inode *);
+extern void udf_drop_inode(struct inode *);
 extern void udf_clear_inode(struct inode *);
 extern int udf_write_inode(struct inode *, int);
 extern long udf_block_map(struct inode *, sector_t);
@@ -146,6 +147,7 @@ extern void udf_free_inode(struct inode 
 extern struct inode * udf_new_inode (struct inode *, int, int *);
 
 /* truncate.c */
+extern void udf_truncate_tail_extent(struct inode *);
 extern void udf_discard_prealloc(struct inode *);
 extern void udf_truncate_extents(struct inode *);