RFC: Re: PATCH: udf fs corruption on linux-2.6

[Rich Coe]

I've updated the patch below to use drop_inode rather than put_inode.

drop_inode is only called when the last iput() reference to the inode is
released, where put_inode is called for every iput().

Rich

On Wed, 13 Jun 2007 15:48:03 -0500
Rich Coe <Richard.Coe@xxxxxxxxxx> wrote:
> Hi Linus, 
> 
> This patch fixes directory and missing files corruption in fs/udf which
> occurs on all known 2.6 releases.
> 
> The corruption occurs because blocks which were pre-alloc'd for a directory 
> are released back to the fs freelist, but the inode's alloc block information
> is not updated to reflect this.
> 
> You would not see corruption if the number of files in any directory is 
> less than 41, because the pre-alloc routine does not allocate blocks for the
> directory until the number of files is over 40.
> 
> The problem occurs during unmounting because fs/udf incorrectly calls 
> udf_discard_prealloc() from udf_clear_inode().  udf_discard_prealloc() will
> update the inode and schedule it for write, but no write will ever occur 
> because the fs is in the process of being umount'd. 
> 
> The solution is to add a put_inode routine to update the inode contents
> and release the pre-alloc'd blocks to disk prior to clearing the inode
> from the kernel.
> 
> Test case:
>     mkuddfs /dev/scd0
>     mount -o sync /dev/scd0 /mnt/cdrom
>     mkdir /mnt/cdrom/A /mnt/cdrom/B
>     cp A/* /mnt/cdrom/A		[ A contains 90 files of various sizes ]
>     cp B/* /mnt/cdrom/B 	[ B contains 20 or fewer files ]
>     umount /mnt/cdrom
> 
> Here you can see how 7 blocks starting at sector 139 are free and listed in the
> directory entry for 'A'.  I used udfdump to get the following information:
>     [ ... ]
>     Free space found on this partition
> 	[00000139 - 00000159]   [00000161 - 00000191]   [00000193 - 00000223]   
> 	[00004464 - 00004475]   [00004485 - 00524286]   [00524287 - 01048573]
> 	[01048574 - 01572860]   [01572861 - 02097147]   [02097148 - 02235039]   
>     [ ... ]
>     Filename `A`
>     [ ... ]
>     [ blob at sector     2038 for     2048 bytes in logical partion 0 ] 
>     [ blob at sector      137 for     4096 bytes in logical partion 0 ] 
>     [ blob at sector      139 for    14336 bytes in logical partion 0 flags 1 ] 
> 
> -- 
> Rich Coe		richard.coe@xxxxxxxxxx
> Virtual Principle Engineer  General Electric Healthcare Technologies
> Global Software Platforms, Computer Technology Team

Signed-off-by: Rich Coe <richard.coe@xxxxxxxxxx>
---
diff -urNp linux-2.6.20.orig/fs/udf/inode.c linux-2.6.20/fs/udf/inode.c
--- linux-2.6.20.orig/fs/udf/inode.c	2007-02-04 12:44:54.000000000 -0600
+++ linux-2.6.20/fs/udf/inode.c	2007-06-13 11:32:41.930983471 -0500
@@ -102,14 +102,17 @@ no_delete:
 
 void udf_clear_inode(struct inode *inode)
 {
+	kfree(UDF_I_DATA(inode));
+	UDF_I_DATA(inode) = NULL;
+}
+
+void udf_put_inode(struct inode *inode)
+{
 	if (!(inode->i_sb->s_flags & MS_RDONLY)) {
 		lock_kernel();
 		udf_discard_prealloc(inode);
 		unlock_kernel();
 	}
-
-	kfree(UDF_I_DATA(inode));
-	UDF_I_DATA(inode) = NULL;
 }
 
 static int udf_writepage(struct page *page, struct writeback_control *wbc)
diff -urNp linux-2.6.20.orig/fs/udf/super.c linux-2.6.20/fs/udf/super.c
--- linux-2.6.20.orig/fs/udf/super.c	2007-02-04 12:44:54.000000000 -0600
+++ linux-2.6.20/fs/udf/super.c	2007-06-13 11:31:23.793017185 -0500
@@ -166,6 +166,7 @@ static struct super_operations udf_sb_op
 	.write_inode		= udf_write_inode,
 	.delete_inode		= udf_delete_inode,
 	.clear_inode		= udf_clear_inode,
+	.drop_inode		= udf_put_inode,
 	.put_super		= udf_put_super,
 	.write_super		= udf_write_super,
 	.statfs			= udf_statfs,
diff -urNp linux-2.6.20.orig/fs/udf/udfdecl.h linux-2.6.20/fs/udf/udfdecl.h
--- linux-2.6.20.orig/fs/udf/udfdecl.h	2007-02-04 12:44:54.000000000 -0600
+++ linux-2.6.20/fs/udf/udfdecl.h	2007-06-13 11:31:16.684293910 -0500
@@ -97,6 +97,7 @@ extern void udf_truncate(struct inode *)
 extern void udf_read_inode(struct inode *);
 extern void udf_delete_inode(struct inode *);
 extern void udf_clear_inode(struct inode *);
+extern void udf_put_inode(struct inode *);
 extern int udf_write_inode(struct inode *, int);
 extern long udf_block_map(struct inode *, long);
 extern int8_t inode_bmap(struct inode *, int, kernel_lb_addr *, uint32_t *, kernel_lb_addr *, uint32_t *, uint32_t *, struct buffer_head **);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/