PATCH: udf fs corruption on linux-2.6

2007-06-13 Thread Rich Coe
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
[0139 - 0159]   [0161 - 0191]   [0193 - 0223]   
[4464 - 4475]   [4485 - 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 for14336 bytes in logical partion 0 flags 1 ] 

-- 
Rich Coe[EMAIL PROTECTED]
Virtual Principle Engineer  General Electric Healthcare Technologies
Global Software Platforms, Computer Technology Team

Signed-off-by: Rich Coe <[EMAIL PROTECTED]>
---
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.c2007-02-04 12:44:54.0 -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.c2007-02-04 12:44:54.0 -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,
+   .put_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.0 -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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-06-14 Thread 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 <[EMAIL PROTECTED]> 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
>   [0139 - 0159]   [0161 - 0191]   [0193 - 0223]   
>   [4464 - 4475]   [4485 - 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 for14336 bytes in logical partion 0 flags 1 
> ] 
> 
> -- 
> Rich Coe  [EMAIL PROTECTED]
> Virtual Principle Engineer  General Electric Healthcare Technologies
> Global Software Platforms, Computer Technology Team

Signed-off-by: Rich Coe <[EMAIL PROTECTED]>
---
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.c2007-02-04 12:44:54.0 -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.c2007-02-04 12:44:54.0 -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.0 -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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/