[Unionfs] kernel BUG at fs/unionfs/fanout.h:128!

[Erez Zadok]

In message <m3myvlns2e.fsf at dynamo.mandriva.com>, Olivier Blin writes:

> I have the same bug, and tried not to inline unionfs_lower_inode_idx
> and unionfs_copy_attr_times to get a more complete trace (the bug
> reads fanout.h:133, but it's fanout.h:128 in the original source code).
> 
> It happens in an union of a squashfs + tmpfs, in which the system
> pivot_roots.
> 
> I can workaround it, but it is probably hidding a more complex issue.
> 
> 
> --=-=-=
> Content-Type: text/x-patch
> Content-Disposition: inline;
> 	filename=MC51-unionfs-2.1.3-do-not-update-mtime-if-no-upper-branch.patch
> Content-Description: do not update mtime if there is no upper branch
> 
> Do not update mtime if there is no upper branch for the inode.
> This prevents from calling unionfs_lower_inode_idx() with a negative
> index, which triggers a bug.
> 
> Signed-off-by: Olivier Blin <blino at mandriva.com>
> 
> diff -urNp linux-2.6.22.orig/fs/unionfs/fanout.h linux-2.6.22/fs/unionfs/fanout.h
> --- linux-2.6.22.orig/fs/unionfs/fanout.h	2007-09-17 15:42:05.000000000 +0200
> +++ linux-2.6.22/fs/unionfs/fanout.h	2007-09-17 16:19:26.000000000 +0200
> @@ -308,7 +308,7 @@ static inline void unionfs_copy_attr_tim
>  	int bindex;
>  	struct inode *lower;
>  
> -	if (!upper)
> +	if (!upper || ibstart(upper) < 0)
>  		return;
>  	for (bindex=ibstart(upper); bindex <= ibend(upper); bindex++) {
>  		lower = unionfs_lower_inode_idx(upper, bindex);

Oliver, I'm pretty certain that your patch is correct.  I've fixed a similar
bug elsewhere in the code not too long ago.

Essentially due to synchrony (which is generally desired to get good
performance), there's a chance that unionfs_copy_attr_times be called on a
unionfs inode that's not fully initialized (ibstart is negative).  This is
ok b/c it relates to cache coherency, and if the upper inode's lower inodes
are being initialized, then any potential cache incoherency (i.e., lower
inodes are newer than upper) will be detected on a need-basis later on.

Thank you.

Erez.