minix fs corruption fix for 2.4

minix fs corruption fix for 2.4

[Konstantin Boldyshev]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1098 bytes --]

Hello,

Enclosed is a simple patch to fix corruption of minix filesystem
when deleting character and block device nodes (special files).
>From what I've found out the bug was introduced somehwere in 2.3
and is present in all 2.4 versions, and I guess also goes into 2.6.
Older 2.0 and 2.2 kernels do not have it - it seems that one who
was rewriting fs code for 2.4 just forgot to add the needed check.

Note that other filesystems that are rarely used nowdays may have
the same bug, especially if they used minix code as a template.


diff -urN linux-2.4.22/fs/minix/itree_common.c linux/fs/minix/itree_common.c
--- linux-2.4.22/fs/minix/itree_common.c	Thu Oct 16 11:30:27 2003
+++ linux/fs/minix/itree_common.c	Mon Nov  3 12:25:20 2003
@@ -301,6 +301,12 @@
 	int first_whole;
 	long iblock;

+	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
+	    S_ISLNK(inode->i_mode)))
+		return;
+	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
+		return;
+
 	iblock = (inode->i_size + BLOCK_SIZE-1) >> 10;
 	block_truncate_page(inode->i_mapping, inode->i_size, get_block);


-- 
Regards,
Konstantin

[-- Attachment #2: Type: TEXT/PLAIN, Size: 555 bytes --]

diff -urN linux-2.4.22/fs/minix/itree_common.c linux/fs/minix/itree_common.c
--- linux-2.4.22/fs/minix/itree_common.c	Thu Oct 16 11:30:27 2003
+++ linux/fs/minix/itree_common.c	Mon Nov  3 12:25:20 2003
@@ -301,6 +301,12 @@
 	int first_whole;
 	long iblock;
 
+	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
+	    S_ISLNK(inode->i_mode)))
+		return;
+	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
+		return;
+
 	iblock = (inode->i_size + BLOCK_SIZE-1) >> 10;
 	block_truncate_page(inode->i_mapping, inode->i_size, get_block);
 

Re: minix fs corruption fix for 2.4

[Linus Torvalds]

On Mon, 3 Nov 2003, Konstantin Boldyshev wrote:
> 
> Enclosed is a simple patch to fix corruption of minix filesystem
> when deleting character and block device nodes (special files).
> From what I've found out the bug was introduced somehwere in 2.3
> and is present in all 2.4 versions, and I guess also goes into 2.6.

Oops, yes.

The problem is that block and character devices put not a block number but 
a _device_ number in the place where other files put their block 
allocations.

Your patch is wrong, though - you shouldn't test for APPEND and IMMUTABLE 
here. That should be done at higher layers. 

I'd also prefer to do the test the other way around: test for CHRDEV and 
BLKDEV in inode.c the same way the other functions do. Something like the 
appended..

Al, can you verify? I think this crept in when you did the block lookup 
cleanups. I also worry whether anybody else got the bug?

		Linus

===== fs/minix/inode.c 1.38 vs edited =====
--- 1.38/fs/minix/inode.c	Fri Sep  5 04:31:53 2003
+++ edited/fs/minix/inode.c	Mon Nov  3 08:51:01 2003
@@ -547,6 +547,8 @@
  */
 void minix_truncate(struct inode * inode)
 {
+	if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode))
+		return;
 	if (INODE_VERSION(inode) == MINIX_V1)
 		V1_minix_truncate(inode);
 	else

Re: minix fs corruption fix for 2.4

[viro]

On Mon, Nov 03, 2003 at 08:55:42AM -0800, Linus Torvalds wrote:
> I'd also prefer to do the test the other way around: test for CHRDEV and 
> BLKDEV in inode.c the same way the other functions do. Something like the 
> appended..
> 
> Al, can you verify? I think this crept in when you did the block lookup 
> cleanups. I also worry whether anybody else got the bug?
> 
> 		Linus

Hmm...  I would rather check for regular|directory|symlink explicitly -
note that FIFO and socket can have junk in i_data.

Looks like that fsckup had happened only in fs/minix - fs/sysv/itree.c
does it right.

Re: minix fs corruption fix for 2.4

[Linus Torvalds]

On Mon, 3 Nov 2003 viro@parcelfarce.linux.theplanet.co.uk wrote:
> 
> Hmm...  I would rather check for regular|directory|symlink explicitly -
> note that FIFO and socket can have junk in i_data.

Fair enough, that's how sysvfs does it too.

Committed and pushed out.

Thanks Konstantin,

		Linus

Re: minix fs corruption fix for 2.4

[Konstantin Boldyshev]

On Mon, 3 Nov 2003, Linus Torvalds wrote:

> > Enclosed is a simple patch to fix corruption of minix filesystem
> > when deleting character and block device nodes (special files).
> > From what I've found out the bug was introduced somehwere in 2.3
> > and is present in all 2.4 versions, and I guess also goes into 2.6.
>
> Oops, yes.
>
> The problem is that block and character devices put not a block
> number but a _device_ number in the place where other files put
> their block allocations.

Yes, it took some time to find out why particular blocks
are being freed when I delete particular device nodes.

> Your patch is wrong, though - you shouldn't test for APPEND
> and IMMUTABLE here. That should be done at higher layers.

Perhaps. I just added the same check as ext2 code does in ext2_truncate().

-- 
Regards,
Konstantin