vxfs fix

Dear Linus, Alan, Al, Christoph, all,

If one mounts without specifying a type, mount will try all available types. After having tried vxfs the next type will cause
set_blocksize: b_count 1 ...
since vxfs forgets to free a block.
The patch below adds the missing brelse().
(In fact there are more resources that are never freed there - maybe the maintainer can have a look some time -
I only added a comment.)

When mount continues to try all types, it may try V7. That always succeeds, there is no test for magic or so, and after garbage has been mounted as a V7 filesystem, the kernel crashes or hangs or fails in other sad ways. Have not tried to debug.

Andries

@@ -178,7 +178,8 @@
 	}
 
 	if (rsbp->vs_version < 2 || rsbp->vs_version > 4) {
-		printk(KERN_NOTICE "vxfs: unsupported VxFS version (%d)\n", rsbp->vs_version);
+		printk(KERN_NOTICE "vxfs: unsupported VxFS version (%d)\n",
+		       rsbp->vs_version);
 		goto out;
 	}
 

@@ -221,6 +222,7 @@
 	if (vxfs_read_fshead(sbp)) {
 		printk(KERN_WARNING "vxfs: unable to read fshead\n");
 		return NULL;
+		/* BUG: lots of gets not matched by puts here */
 	}
 
 	sbp->s_op = &vxfs_super_ops;

@@ -229,6 +231,7 @@
 	
 	printk(KERN_WARNING "vxfs: unable to get root dentry.\n");
 out:
+	brelse(bp);
 	kfree(infp);
 	return NULL;
 }

That's interesting. IIRC, v7 has no magic in superblock. At all. It shouldn't crash the box, though - I'll look into that.

Yes, I already hit that one time but forgot to fix it. I'll update vxfs to free all resources somewhen later this week.

Thanks for the catch!

Maybe some sanity checks should be added?

I'd like to propose:

s_nfree <= V7_NICFREE
s_ninode <= V7_NICINOD
s_time != 0

Christoph

Of course it doesn't work. We've performed a software upgrade.

Even on filesystems that have bad (limited or non-existent) magic numbers, the read_super() function should really be able to do a fair amount of sanity-checking. If nothing else, then things like verifying that the root inode really is a directory with a proper size, for example.

I don't think V7 has a magic number at all. But checking that the size and nr-of-inodes fields make sense, together with verifying that the root inode really is a directory with (size % 512) == 0, and possibly verifying things like "if the root directory is not large enough to have a doubly/triply indirect block, then that doubly/triply indirect blocknumber had better be zero" would catch 99.9% of everything.

Whether those kind of tests are really worth adding, considering the rather limited number of people who actually enable the FS, I don't know.

Linus

Alternatively pass a flag to the mount command saying "this is a guesswork special" then V7 fs can just return 'not me'

Alan