linux-kernel - Re: overlayfs: BUG in ovl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 06 Oct 2010 18:51:09 +0200
From:	Miklos Szeredi <miklos@...redi.hu>
To:	Andy Whitcroft <apw@...onical.com>
CC:	miklos@...redi.hu, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: overlayfs: BUG in ovl_whiteout

On Wed, 6 Oct 2010, Andy Whitcroft wrote:
> On Mon, Oct 04, 2010 at 10:16:29PM +0200, Miklos Szeredi wrote:
> > 
> > OK, my guess is that it's a 'rename to self' which was not properly
> > implemented.
> > 
> > Does the following patch make a difference?
> 
> That one panic'd immediatly.  Seems that we can have missmatched lowers
> as well.  I applied the below over the top, and that seems to pass
> testing without tripping the WARN_ON.
> 
> -apw
> 
> commit 03375858c62f617846b5dc4968fe8178eda51a7c
> Author: Andy Whitcroft <apw@...onical.com>
> Date:   Tue Oct 5 14:54:08 2010 +0100
> 
>     overlayfs: handle missing lower inodes in ovl_is_same_inode
>     
>     Signed-off-by: Andy Whitcroft <apw@...onical.com>

OK, that's because d1 or d2 is negative, otherwise either the upper or
the lower dentry _must_ exist.  In the version I committed to the
overlayfs.v3 branch vfs_is_same_inode() checks whether the dentries
are positive and only calls ->is_same_inode() if they are.

Can you please try pull from that branch and check if it's still buggy
without your patch?

Thanks,
Miklos



> 
> diff --git a/fs/overlayfs/overlayfs.c b/fs/overlayfs/overlayfs.c
> index c10cc7b..f596222 100644
> --- a/fs/overlayfs/overlayfs.c
> +++ b/fs/overlayfs/overlayfs.c
> @@ -1964,19 +1964,25 @@ static int ovl_statfs(struct dentry *dentry, struct kstatfs *buf)
>  
>  static bool ovl_is_same_inode(struct dentry *d1, struct dentry *d2)
>  {
> -	struct dentry *upperd1;
> -	struct dentry *upperd2;
> +	struct dentry *od1;
> +	struct dentry *od2;
>  
> -	upperd1 = ovl_dentry_upper(d1);
> -	upperd2 = ovl_dentry_upper(d2);
> +	od1 = ovl_dentry_upper(d1);
> +	od2 = ovl_dentry_upper(d2);
>  
> -	if (upperd1 && upperd2)
> -		return vfs_is_same_inode(upperd1, upperd2);
> +	if (od1 && od2)
> +		return vfs_is_same_inode(od1, od2);
>  
> -	if (upperd1 || upperd2)
> +	if (od1 || od2)
>  		return false;
>  
> -	return vfs_is_same_inode(ovl_dentry_lower(d1), ovl_dentry_lower(d2));
> +	od1 = ovl_dentry_lower(d1);
> +	od2 = ovl_dentry_lower(d2);
> +
> +	if (od1 && od2)
> +		return vfs_is_same_inode(od1, od2);
> +
> +	return false;
>  }
>  
>  static const struct super_operations ovl_super_operations = {
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/