hfsplus bugs in linux-2.6.5

hfsplus bugs in linux-2.6.5

[Martin Schaffner]

Here's how to trigger a hfsplus bug in linux-2.6.5:

dd if=/dev/zero of=/test bs=1k count=10k
newfs_hfs /test
mount -t hfsplus /test /mnt/test -o loop
cd /mnt/test
mkdir a; mkdir b
mv a b

The last command doesn't correctly move "a": it complains that it 
cannot find "b/a". Effectively, it creates an entry in "b", but that 
entry doesn't correspond to any actual directory "a". This means I am 
stuck with a directory "b" which I can't delete with "rm -rf b". 
However, I can delete it with:

rmdir b; rm -rf b

If above, after creating, mounting and cd-ing to the hfsplus 
filesystem, I do:

cp -r /whatever/module-init-tools-3.0 .
mkdir m; mv mod* m
rmdir m; rm -rf m
cp -r /whatever/module-init-tools-3.0 .

Then the machine hangs. While I can still change the screen brightness 
with the topleft keys of the keyboard, I can't change virtual consoles 
anymore.


A second, less serious wierdness is that directories created with linux 
are bigger than directories created with Mac OS X: When I do "for 
((i=1;i>0;i++)); do mkdir $i; done" on a new 1MB-HFS+-image on Mac OS 
X, I can create about 6300 directories. With Linux, it's about 3600.
Funny that for both, I can't free up any space afterwards, even if I 
delete everything inside the volume.

Can someone more knowledgeable look at these bugs? Unfortunately, I 
don't have a serial port (or second computer with USB port), so I can't 
do any kernel debugging the usual way.

Thanks,

Martin

Re: hfsplus bugs in linux-2.6.5

[Roman Zippel]

Hi,

On Fri, 14 May 2004, Martin Schaffner wrote:

> Here's how to trigger a hfsplus bug in linux-2.6.5:
>
> dd if=/dev/zero of=/test bs=1k count=10k
> newfs_hfs /test
> mount -t hfsplus /test /mnt/test -o loop
> cd /mnt/test
> mkdir a; mkdir b
> mv a b

Thanks for the report, renames of directories were broken, the patch below
fixes this. Could you please try again, if it solves your problem?

> A second, less serious wierdness is that directories created with linux
> are bigger than directories created with Mac OS X: When I do "for
> ((i=1;i>0;i++)); do mkdir $i; done" on a new 1MB-HFS+-image on Mac OS
> X, I can create about 6300 directories. With Linux, it's about 3600.
> Funny that for both, I can't free up any space afterwards, even if I
> delete everything inside the volume.

The HFS+ catalog file (roughly equivalent to the ext inode table) grows
dynamically and the space is usually not used completely, OS X currently
allocates them better than Linux and doesn't leave as much holes. The
other problem is that the catalog file doesn't shrink, as that would
require online defragmentation. It's possible but not really a priority
right now.

bye, Roman

Index: fs/hfsplus/catalog.c
===================================================================
RCS file: /usr/src/cvsroot/linux-2.6/fs/hfsplus/catalog.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 catalog.c
--- a/fs/hfsplus/catalog.c	11 Mar 2004 18:33:35 -0000	1.1.1.1
+++ b/fs/hfsplus/catalog.c	16 May 2004 19:04:38 -0000
@@ -165,11 +165,11 @@ int hfsplus_create_cat(u32 cnid, struct
 	if (err != -ENOENT) {
 		if (!err)
 			err = -EEXIST;
-		goto out;
+		goto err2;
 	}
 	err = hfs_brec_insert(&fd, &entry, entry_size);
 	if (err)
-		goto out;
+		goto err2;

 	hfsplus_cat_build_key(fd.search_key, dir->i_ino, str);
 	entry_size = hfsplus_cat_build_record(&entry, cnid, inode);
@@ -178,16 +178,23 @@ int hfsplus_create_cat(u32 cnid, struct
 		/* panic? */
 		if (!err)
 			err = -EEXIST;
-		goto out;
+		goto err1;
 	}
 	err = hfs_brec_insert(&fd, &entry, entry_size);
-	if (!err) {
-		dir->i_size++;
-		mark_inode_dirty(dir);
-	}
-out:
+	if (err)
+		goto err1;
+
+	dir->i_size++;
+	mark_inode_dirty(dir);
 	hfs_find_exit(&fd);
+	return 0;

+err1:
+	hfsplus_cat_build_key(fd.search_key, cnid, NULL);
+	if (!hfs_brec_find(&fd))
+		hfs_brec_remove(&fd);
+err2:
+	hfs_find_exit(&fd);
 	return err;
 }

Index: fs/hfsplus/dir.c
===================================================================
RCS file: /usr/src/cvsroot/linux-2.6/fs/hfsplus/dir.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 dir.c
--- a/fs/hfsplus/dir.c	11 Mar 2004 18:33:35 -0000	1.1.1.1
+++ b/fs/hfsplus/dir.c	16 May 2004 17:36:26 -0000
@@ -18,6 +18,13 @@
 #include "hfsplus_fs.h"
 #include "hfsplus_raw.h"

+static inline void hfsplus_instantiate(struct dentry *dentry,
+				       struct inode *inode, u32 cnid)
+{
+	dentry->d_fsdata = (void *)(unsigned long)cnid;
+	d_instantiate(dentry, inode);
+}
+
 /* Find the entry inside dir named dentry->d_name */
 static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry,
 				     struct nameidata *nd)
@@ -52,6 +69,7 @@ again:
 			goto fail;
 		}
 		cnid = be32_to_cpu(entry.folder.id);
+		dentry->d_fsdata = (void *)(unsigned long)cnid;
 	} else if (type == HFSPLUS_FILE) {
 		if (fd.entrylength < sizeof(struct hfsplus_cat_file)) {
 			err = -EIO;
@@ -233,11 +251,11 @@ int hfsplus_create(struct inode *dir, st
 	res = hfsplus_create_cat(inode->i_ino, dir, &dentry->d_name, inode);
 	if (res) {
 		inode->i_nlink = 0;
+		hfsplus_delete_inode(inode);
 		iput(inode);
 		return res;
 	}
-	dentry->d_fsdata = (void *)inode->i_ino;
-	d_instantiate(dentry, inode);
+	hfsplus_instantiate(dentry, inode, inode->i_ino);
 	mark_inode_dirty(inode);
 	return 0;
 }
@@ -284,8 +302,7 @@ int hfsplus_link(struct dentry *src_dent
 		return res;

 	inode->i_nlink++;
-	dst_dentry->d_fsdata = (void *)(unsigned long)cnid;
-	d_instantiate(dst_dentry, inode);
+	hfsplus_instantiate(dst_dentry, inode, cnid);
 	atomic_inc(&inode->i_count);
 	inode->i_ctime = CURRENT_TIME;
 	mark_inode_dirty(inode);
@@ -351,10 +368,11 @@ int hfsplus_mkdir(struct inode *dir, str
 	res = hfsplus_create_cat(inode->i_ino, dir, &dentry->d_name, inode);
 	if (res) {
 		inode->i_nlink = 0;
+		hfsplus_delete_inode(inode);
 		iput(inode);
 		return res;
 	}
-	d_instantiate(dentry, inode);
+	hfsplus_instantiate(dentry, inode, inode->i_ino);
 	mark_inode_dirty(inode);
 	return 0;
 }
@@ -391,7 +409,8 @@ int hfsplus_symlink(struct inode *dir, s
 	res = page_symlink(inode, symname, strlen(symname) + 1);
 	if (res) {
 		inode->i_nlink = 0;
-		iput (inode);
+		hfsplus_delete_inode(inode);
+		iput(inode);
 		return res;
 	}

@@ -399,8 +418,7 @@ int hfsplus_symlink(struct inode *dir, s
 	res = hfsplus_create_cat(inode->i_ino, dir, &dentry->d_name, inode);

 	if (!res) {
-		dentry->d_fsdata = (void *)inode->i_ino;
-		d_instantiate(dentry, inode);
+		hfsplus_instantiate(dentry, inode, inode->i_ino);
 		mark_inode_dirty(inode);
 	}

@@ -421,12 +439,12 @@ int hfsplus_mknod(struct inode *dir, str
 	res = hfsplus_create_cat(inode->i_ino, dir, &dentry->d_name, inode);
 	if (res) {
 		inode->i_nlink = 0;
+		hfsplus_delete_inode(inode);
 		iput(inode);
 		return res;
 	}
 	init_special_inode(inode, mode, rdev);
-	dentry->d_fsdata = (void *)inode->i_ino;
-	d_instantiate(dentry, inode);
+	hfsplus_instantiate(dentry, inode, inode->i_ino);
 	mark_inode_dirty(inode);

 	return 0;

Re: hfsplus bugs in linux-2.6.5

[Martin Schaffner]

On 17.05.2004, at 00:19, Roman Zippel wrote:

>> Here's how to trigger a hfsplus bug in linux-2.6.5:
>> [...]
>> mkdir a; mkdir b
>> mv a b
>
> Thanks for the report, renames of directories were broken, the patch 
> below
> fixes this. Could you please try again, if it solves your problem?

Your patch fixes the above problem, thanks.

Unfortunately, the first bug I encountered (which lead me to 
stress-test hfsplus support in the first place) is still present.

I still wasn't able to reproduce it on another partition than my Mac OS 
X root partition. :-(

The symptoms are as follows: Whenever I try to write a sufficently 
large file (always larger than 512k), or try to read a sufficiently 
large file (say a 4 MB file) with any program, I get:

   HFS+-fs: request for non-existent node 1929183232 in B*Tree

after which the I/O operation fails like this:

   dd: writing 'bla': Cannot allocate memory

The number of the non-existent node will not change until I do a fsck.

After linux handled the partition, Apple's fsck_hfs will always report 
errors, which it usually can't fix, but TechTool Deluxe can.

In the past, hpfsck (from ftp.penguinppc.org/users/hasi/) would report 
lots of cases of:

   Backpointers in Node 239 index 70 out of order (0x1001e982 >= 
0x1002298e)

with differing indices but constant node number (until the next fsck).

However, lately, all I get is:

   [...]
   *** Checking Backup Volume Header:
   Unexpected Volume signature '  ' expected 'H+'
   hpfsck: hpfsck: This is not a HFS+ volume (Unknown error 4294967295)


However, I think I have narrowed it down somewhat: If I apply:

diff -ur linux-2.6.6/fs/hfsp.bak/bfind.c linux-2.6.6/fs/hfsplus/bfind.c
--- linux-2.6.6/fs/hfsp.bak/bfind.c     Fri May 21 11:38:33 2004
+++ linux-2.6.6/fs/hfsplus/bfind.c      Fri May 21 15:31:16 2004
@@ -64,7 +64,7 @@
                 else
                         e = rec - 1;
         } while (b <= e);
-       //printk("%d: %d,%d,%d\n", bnode->this, b, e, rec);
+       printk("problem is here: %d: %d,%d,%d\n", bnode->this, b, e, 
rec);
         if (rec != e && e >= 0) {
                 len = hfs_brec_lenoff(bnode, e, &off);
                 keylen = hfs_brec_keylen(bnode, e);

then, this new printk happens often. More importantly, it happens 
ALWAYS before the "request for non-existent node" message and 
subsequent failure.


I guess that sometimes, the bnodes aren't ordered correctly, and 
therefore we don't find the correct one. If the while loop is 
terminated without "goto done;", shouldn't we fall back to checking all 
bnodes for a matching key, in case the inodes aren't ordered correctly? 
I've tried to implement exactly that, but this resulted in lots of 
Oopses - I guess I'd have to dig deeper, guess less, and understand 
more to find out more. Unfortunately, I can't debug the problem while 
it's happening, because my ibook doesn't have a serial port...


How could I get to the root of the problem?

Thanks,

Martin

Re: hfsplus bugs in linux-2.6.5

[Roman Zippel]

Hi,

On Sat, 22 May 2004, Martin Schaffner wrote:

> I still wasn't able to reproduce it on another partition than my Mac OS 
> X root partition. :-(
> 
> The symptoms are as follows: Whenever I try to write a sufficently 
> large file (always larger than 512k), or try to read a sufficiently 
> large file (say a 4 MB file) with any program, I get:
> 
>    HFS+-fs: request for non-existent node 1929183232 in B*Tree

It seems you have a very fragmented volume and it goes wrong when the 
driver tries to access the extent file. I tested this with HFS, but it 
seems not all fixes made it to the HFS+ driver.
Fix is below.

> In the past, hpfsck (from ftp.penguinppc.org/users/hasi/) would report 
> lots of cases of:
> 
>    Backpointers in Node 239 index 70 out of order (0x1001e982 >= 
> 0x1002298e)

hpfsck has quite some problems with large volumes and last time I checked 
this was usually a bug in hpfsck.

bye, Roman

Index: fs/hfsplus/brec.c
===================================================================
RCS file: /usr/src/cvsroot/linux-2.6/fs/hfsplus/brec.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 brec.c
--- a/fs/hfsplus/brec.c	11 Mar 2004 18:33:35 -0000	1.1.1.1
+++ b/fs/hfsplus/brec.c	22 May 2004 12:10:18 -0000
@@ -33,7 +33,7 @@ u16 hfs_brec_keylen(struct hfs_bnode *no
 
 	if ((node->type == HFS_NODE_INDEX) &&
 	   !(node->tree->attributes & HFS_TREE_VARIDXKEYS)) {
-		retval = node->tree->max_key_len;
+		retval = node->tree->max_key_len + 2;
 	} else {
 		recoff = hfs_bnode_read_u16(node, node->tree->node_size - (rec + 1) * 2);
 		if (!recoff)
@@ -144,7 +144,7 @@ skip:
 		if (tree->attributes & HFS_TREE_VARIDXKEYS)
 			key_len = be16_to_cpu(fd->search_key->key_len) + 2;
 		else {
-			fd->search_key->key_len = tree->max_key_len;
+			fd->search_key->key_len = be16_to_cpu(tree->max_key_len);
 			key_len = tree->max_key_len + 2;
 		}
 		goto again;

Re: hfsplus bugs in linux-2.6.5

[Martin Schaffner]

On 22.05.2004, at 14:32, Roman Zippel wrote:

>> I still wasn't able to reproduce it on another partition than my Mac 
>> OS
>> X root partition. :-(
>>
>> The symptoms are as follows: Whenever I try to write a sufficently
>> large file (always larger than 512k), or try to read a sufficiently
>> large file (say a 4 MB file) with any program, I get:
>>
>>    HFS+-fs: request for non-existent node 1929183232 in B*Tree
>
> It seems you have a very fragmented volume and it goes wrong when the
> driver tries to access the extent file. I tested this with HFS, but it
> seems not all fixes made it to the HFS+ driver.
> Fix is below.

Yes, that solved the problem! Thanks a lot for that fast fix! I'm glad 
that's finally resolved.


There is one more annoying thing with the hfsplus driver: the handling 
of HFSPLUS_VOL_INCNSTNT.
It seem that both Mac OS X's mount/unmount routines and fsck_hfs don't 
ever read or write this bit, in violation of Apple's own 
specifications. This means that when linux crashes, a mounted hfs+ 
volumes will from then on have the INCONSTNT bit set and linux will 
refuse to mount it read-write, even after Mac OS X fscked it and 
mounted it read/write. This situation can be resolved by running 
hpmount on that volume, which clears the INCONSTNT bit, but that's not 
user-friendly. I therefore propose this simple patch:

--- linux-2.6.6/fs/hfsplus/super.c.bak  Sat May 22 20:32:05 2004
+++ linux-2.6.6/fs/hfsplus/super.c      Sat May 22 20:34:48 2004
@@ -245,8 +245,7 @@
         if (!(*flags & MS_RDONLY)) {
                 struct hfsplus_vh *vhdr = HFSPLUS_SB(sb).s_vhdr;

-               if ((vhdr->attributes & 
cpu_to_be32(HFSPLUS_VOL_INCNSTNT)) ||
-                   !(vhdr->attributes & 
cpu_to_be32(HFSPLUS_VOL_UNMNT))) {
+               if (!(vhdr->attributes & 
cpu_to_be32(HFSPLUS_VOL_UNMNT))) {
                         printk("HFS+-fs warning: Filesystem was not 
cleanly unmounted, "
                                "running fsck.hfsplus is recommended.  
leaving read-only.\n");
                         sb->s_flags |= MS_RDONLY;
@@ -331,8 +330,7 @@
         sb->s_op = &hfsplus_sops;
         sb->s_maxbytes = MAX_LFS_FILESIZE;

-       if ((vhdr->attributes & cpu_to_be32(HFSPLUS_VOL_INCNSTNT)) ||
-           !(vhdr->attributes & cpu_to_be32(HFSPLUS_VOL_UNMNT))) {
+       if (!(vhdr->attributes & cpu_to_be32(HFSPLUS_VOL_UNMNT))) {
                 if (!silent)
                         printk("HFS+-fs warning: Filesystem was not 
cleanly unmounted, "
                                "running fsck.hfsplus is recommended.  
mounting read-only.\n");