[PATCH] Linux-0.97.4 (September 7, 1992)

Linus "dances with patches" Torvalds strikes again: I've already made
patchlevel 4 of 0.97.  It may not be a new record, but it's close :-)

Patch 4 is a very minor patch, but it's pretty important if you want a
stable filesystem (and let's face it: most people seem to prefer a
filesystem that stays up a bit longer).  While patch3 corrected most of
the race-conditions in the minix fs, I overlooked the [f]truncate system
calls, and this new patch corrects that.

[f]truncate is a very race-prone function, and as if that wasn't enough,
there was also a pretty bad error in truncate.c that resulted in the
indirect blocks not being correctly marked dirty when doing partial
truncates.  The latter problem is probably the reason for most of the
filesystem corruptions that have been reported - the race-conditions
were a lot harder to fix, but they also happen a lot less often.

Note that the [f]truncate bug isn't new: it has been in the kernel since
[f]truncate was first implemented (0.95?).  But until now, [f]truncate()
hasn't actually been used very much - only the latest versions of the
binutils have used ftruncate to strip binaries etc.  So the problem
hasn't shown up that much.

So while I consider patch4 to be crucial, you /can/ actually live
without it: I haven't seen the buffer corruption problem at all (until I
actually tested for it after getting good bug-reports), so you can
provably miss it for a long time.  But if you have ever had corruption
problems, I'd suggest upgrading to pl4 as soon as possible.

The corruption problems show up most clearly when using a new "strip"
binary, although they are theoretically possible with other programs
too.  Thanks to "obz@raster.kodak.com" and "jon@robots.ox.ac.uk" for
good bug-reports: thanks to them I was able to pin down the error to
truncate.c, and after that it was pretty easy to get rid of it.

Also note that this patch still hasn't fixed the extended filesystem: I
suspect the same bugs lurk around there.  I'll get it corrected by 0.98
at the latest.

The patch is included at the end of this post (it's very minor - it
contains patches mainly against linux/fs/minix/truncate.c) , and I'll
also update nic.funet.fi (pub/OS/Linux/testing/Linus) to have the new
sources.  Sorry for the inconvenience,

                Linus

diff --git a/Makefile b/Makefile
index e20c98d172ad049afca63596dfea759b3a2b8fe4..26cbba78fc7a5a96c7e81711b283a763dcf8b16b 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -112,7 +112,7 @@ linuxsubdirs: dummy
 
 Version:
        @./makever.sh
-       @echo \#define UTS_RELEASE \"0.97.pl3-`cat .version`\" > tools/version.h
+       @echo \#define UTS_RELEASE \"0.97.pl4-`cat .version`\" > tools/version.h
        @echo \#define UTS_VERSION \"`date +%D`\" >> tools/version.h
        @echo \#define LINUX_COMPILE_TIME \"`date +%T`\" >> tools/version.h
        @echo \#define LINUX_COMPILE_BY \"`whoami`\" >> tools/version.h

diff --git a/fs/minix/bitmap.c b/fs/minix/bitmap.c
index 51082b2bd43574b8fb42cfcd589b20f8d06d07c7..cc66d7fb3febaa6802b63e35ce2b94930c6f8421 100644 (file)
--- a/fs/minix/bitmap.c
+++ b/fs/minix/bitmap.c
@@ -73,7 +73,7 @@ static unsigned long count_used(struct buffer_head *map[], unsigned numblocks,
        return(sum);
 }
 
-int minix_free_block(int dev, int block)
+void minix_free_block(int dev, int block)
 {
        struct super_block * sb;
        struct buffer_head * bh;
@@ -81,32 +81,29 @@ int minix_free_block(int dev, int block)
 
        if (!(sb = get_super(dev))) {
                printk("trying to free block on nonexistent device\n");
-               return 1;
+               return;
        }
        if (block < sb->u.minix_sb.s_firstdatazone ||
            block >= sb->u.minix_sb.s_nzones) {
                printk("trying to free block not in datazone\n");
-               return 1;
+               return;
        }
        bh = get_hash_table(dev,block,BLOCK_SIZE);
-       if (bh) {
-               if (bh->b_count > 1) {
-                       brelse(bh);
-                       return 0;
-               }
+       if (bh)
                bh->b_dirt=0;
-               bh->b_uptodate=0;
-               if (bh->b_count)
-                       brelse(bh);
-       }
+       brelse(bh);
        zone = block - sb->u.minix_sb.s_firstdatazone + 1;
        bit = zone & 8191;
        zone >>= 13;
        bh = sb->u.minix_sb.s_zmap[zone];
+       if (!bh) {
+               printk("minix_free_block: nonexistent bitmap buffer\n");
+               return;
+       }
        if (clear_bit(bit,bh->b_data))
                printk("free_block (%04x:%d): bit already cleared\n",dev,block);
        bh->b_dirt = 1;
-       return 1;
+       return;
 }
 
 int minix_new_block(int dev)

diff --git a/fs/minix/truncate.c b/fs/minix/truncate.c
index 767dd3d08a01548a5307849f0c2fd1f37cc88ccc..34c20a6e4a48960219e3b77196b7fd9cc53de637 100644 (file)
--- a/fs/minix/truncate.c
+++ b/fs/minix/truncate.c
@@ -26,111 +26,159 @@
 
 static int trunc_direct(struct inode * inode)
 {
-       int i;
-       int result = 0;
+       struct buffer_head * bh;
+       int i, tmp;
+       int retry = 0;
 #define DIRECT_BLOCK ((inode->i_size + 1023) >> 10)
 
 repeat:
        for (i = DIRECT_BLOCK ; i < 7 ; i++) {
-               if (i < DIRECT_BLOCK)
+               tmp = inode->i_data[i];
+               if (!tmp)
+                       continue;
+               bh = getblk(inode->i_dev,tmp,BLOCK_SIZE);
+               if (i < DIRECT_BLOCK) {
+                       brelse(bh);
                        goto repeat;
-               if (!inode->i_data[i])
+               }
+               if ((bh && bh->b_count != 1) || tmp != inode->i_data[i]) {
+                       retry = 1;
+                       brelse(bh);
                        continue;
-               result = 1;
-               if (minix_free_block(inode->i_dev,inode->i_data[i]))
-                       inode->i_data[i] = 0;
+               }
+               inode->i_data[i] = 0;
+               inode->i_dirt = 1;
+               brelse(bh);
+               minix_free_block(inode->i_dev,tmp);
        }
-       return result;
+       return retry;
 }
 
 static int trunc_indirect(struct inode * inode, int offset, unsigned short * p)
 {
-       int i;
-       struct buffer_head * bh = NULL;
+       struct buffer_head * bh;
+       int i, tmp;
+       struct buffer_head * ind_bh;
        unsigned short * ind;
-       int result = 0;
+       int retry = 0;
 #define INDIRECT_BLOCK (DIRECT_BLOCK-offset)
 
-       if (*p)
-               bh = bread(inode->i_dev, *p, BLOCK_SIZE);
-       if (!bh)
+       tmp = *p;
+       if (!tmp)
+               return 0;
+       ind_bh = bread(inode->i_dev, tmp, BLOCK_SIZE);
+       if (tmp != *p) {
+               brelse(ind_bh);
+               return 1;
+       }
+       if (!ind_bh) {
+               *p = 0;
                return 0;
+       }
 repeat:
        for (i = INDIRECT_BLOCK ; i < 512 ; i++) {
                if (i < 0)
                        i = 0;
                if (i < INDIRECT_BLOCK)
                        goto repeat;
-               ind = i+(unsigned short *) bh->b_data;
-               if (!*ind)
+               ind = i+(unsigned short *) ind_bh->b_data;
+               tmp = *ind;
+               if (!tmp)
                        continue;
-               result = 1;
-               if (minix_free_block(inode->i_dev,*ind))
-                       *ind = 0;
+               bh = getblk(inode->i_dev,tmp,BLOCK_SIZE);
+               if (i < INDIRECT_BLOCK) {
+                       brelse(bh);
+                       goto repeat;
+               }
+               if ((bh && bh->b_count != 1) || tmp != *ind) {
+                       retry = 1;
+                       brelse(bh);
+                       continue;
+               }
+               *ind = 0;
+               ind_bh->b_dirt = 1;
+               brelse(bh);
+               minix_free_block(inode->i_dev,tmp);
        }
-       ind = (unsigned short *) bh->b_data;
+       ind = (unsigned short *) ind_bh->b_data;
        for (i = 0; i < 512; i++)
                if (*(ind++))
                        break;
-       brelse(bh);
-       if (i >= 512) {
-               result = 1;
-               if (minix_free_block(inode->i_dev,*p))
+       if (i >= 512)
+               if (ind_bh->b_count != 1)
+                       retry = 1;
+               else {
+                       tmp = *p;
                        *p = 0;
-       }
-       return result;
+                       minix_free_block(inode->i_dev,tmp);
+               }
+       brelse(ind_bh);
+       return retry;
 }
                
 static int trunc_dindirect(struct inode * inode)
 {
-       int i;
-       struct buffer_head * bh = NULL;
+       int i, tmp;
+       struct buffer_head * dind_bh;
        unsigned short * dind;
-       int result = 0;
+       int retry = 0;
 #define DINDIRECT_BLOCK ((DIRECT_BLOCK-(512+7))>>9)
 
-       if (inode->i_data[8])
-               bh = bread(inode->i_dev, inode->i_data[8], BLOCK_SIZE);
-       if (!bh)
+       tmp = inode->i_data[8];
+       if (!tmp)
                return 0;
+       dind_bh = bread(inode->i_dev, tmp, BLOCK_SIZE);
+       if (tmp != inode->i_data[8]) {
+               brelse(dind_bh);
+               return 1;
+       }
+       if (!dind_bh) {
+               inode->i_data[8] = 0;
+               return 0;
+       }
 repeat:
        for (i = DINDIRECT_BLOCK ; i < 512 ; i ++) {
                if (i < 0)
                        i = 0;
                if (i < DINDIRECT_BLOCK)
                        goto repeat;
-               dind = i+(unsigned short *) bh->b_data;
-               if (!*dind)
-                       continue;
-               result |= trunc_indirect(inode,7+512+(i<<9),dind);
+               dind = i+(unsigned short *) dind_bh->b_data;
+               retry |= trunc_indirect(inode,7+512+(i<<9),dind);
+               dind_bh->b_dirt = 1;
        }
-       dind = (unsigned short *) bh->b_data;
+       dind = (unsigned short *) dind_bh->b_data;
        for (i = 0; i < 512; i++)
                if (*(dind++))
                        break;
-       brelse(bh);
-       if (i >= 512) {
-               result = 1;
-               if (minix_free_block(inode->i_dev,inode->i_data[8]))
+       if (i >= 512)
+               if (dind_bh->b_count != 1)
+                       retry = 1;
+               else {
+                       tmp = inode->i_data[8];
                        inode->i_data[8] = 0;
-       }
-       return result;
+                       inode->i_dirt = 1;
+                       minix_free_block(inode->i_dev,tmp);
+               }
+       brelse(dind_bh);
+       return retry;
 }
                
 void minix_truncate(struct inode * inode)
 {
-       int flag;
+       int retry;
 
        if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
             S_ISLNK(inode->i_mode)))
                return;
-       if (inode->i_data[7] & 0xffff0000)
+       if (inode->i_data[7] & 0xffff0000) {
                printk("BAD! minix inode has 16 high bits set\n");
+               inode->i_data[7] = 0;
+       }
        while (1) {
-               flag = trunc_direct(inode);
-               flag |= trunc_indirect(inode,7,(unsigned short *)&inode->i_data[7]);
-               flag |= trunc_dindirect(inode);
-               if (!flag)
+               retry = trunc_direct(inode);
+               retry |= trunc_indirect(inode,7,(unsigned short *)&inode->i_data[7]);
+               retry |= trunc_dindirect(inode);
+               if (!retry)
                        break;
                current->counter = 0;
                schedule();

diff --git a/include/linux/minix_fs.h b/include/linux/minix_fs.h
index 614070e21180ca266e606edcd9fda838dd45af3a..d97df3dc9ee7d7f997996a547f00729324b42696 100644 (file)
--- a/include/linux/minix_fs.h
+++ b/include/linux/minix_fs.h
@@ -63,7 +63,7 @@ extern struct inode * minix_new_inode(int dev);
 extern void minix_free_inode(struct inode * inode);
 extern unsigned long minix_count_free_inodes(struct super_block *sb);
 extern int minix_new_block(int dev);
-extern int minix_free_block(int dev, int block);
+extern void minix_free_block(int dev, int block);
 extern unsigned long minix_count_free_blocks(struct super_block *sb);
 
 extern int minix_bmap(struct inode *,int);

diff --git a/tools/build.c b/tools/build.c
index 5169ce3c19d3f108bc1637f9d6f19698081fc4e3..3573df91abcf6ffd8b975d72bb39bc4333c096fc 100644 (file)
--- a/tools/build.c
+++ b/tools/build.c
@@ -80,12 +80,6 @@ int main(int argc, char ** argv)
                minor_root = DEFAULT_MINOR_ROOT;
        }
        fprintf(stderr, "Root device is (%d, %d)\n", major_root, minor_root);
-       if ((major_root != 2) && (major_root != 3) &&
-           (major_root != 8) && (major_root != 0)) {
-               fprintf(stderr, "Illegal root device (major = %d)\n",
-                       major_root);
-               die("Bad root device --- major #");
-       }
        for (i=0;i<sizeof buf; i++) buf[i]=0;
        if ((id=open(argv[1],O_RDONLY,0))<0)
                die("Unable to open 'boot'");