[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

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

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)

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();

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);

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'");