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
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
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.
On Mon, 3 Nov 2003 [email protected] 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
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