linux-ext4 - Re: [PATCH 5 2/4] Return 32/64-bit dir name hash according to usage type

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120306022838.GA24323@thunk.org>
Date:	Mon, 5 Mar 2012 21:28:38 -0500
From:	Ted Ts'o <tytso@....edu>
To:	Bernd Schubert <bernd.schubert@...m.fraunhofer.de>
Cc:	linux-nfs@...r.kernel.org, linux-ext4@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, Fan Yong <yong.fan@...mcloud.com>,
	bfields@...hat.com, sandeen@...hat.com,
	Andreas Dilger <adilger@...mcloud.com>
Subject: Re: [PATCH 5 2/4] Return 32/64-bit dir name hash according to
 usage type

On Tue, Mar 06, 2012 at 01:40:05AM +0100, Bernd Schubert wrote:
> 
> Yeah, you are right, we also should check for 64-bit EOF. But
> wouldn't be something like this be better?
> 
> 	/* check for hash collision */
> 	if(is_32bit_api() ) {
> 		if (hash == (EXT4_HTREE_EOF_32BIT<<  1))
> 			hash = (EXT4_HTREE_EOF_32BIT - 1)<<  1;
> 	} else {
> 		if (hash == (EXT4_HTREE_EOF_64BIT<<  1))
> 			hash = (EXT4_HTREE_EOF_64BIT - 1)<<  1;
> 	}

Actually, neither change is needed, now that I look at things more
closely.  hash is a __u32, so it could never been
EXT4_HTREE_EOF_64BIT.  But given that we won't let major hash become
larger than 0xfffffffc, that means the largest possible position value
is 0x7ffffffeffffffff.  So using an EOF value of 0x0x7fffffffffffffff
will work fine.

The bigger problem that I found when I looked more closely at the
patch is that the patch uses f_flags in places where f_mode needs to
be used:

static inline loff_t hash2pos(struct file *filp, __u32 major, __u32 minor)
{
	if ((filp->f_flags & FMODE_32BITHASH) ||
                   ^^^^^^^
	    (!(filp->f_flags & FMODE_64BITHASH) && is_32bit_api()))
                     ^^^^^^^
		return major >> 1;
	else
		return ((__u64)(major >> 1) << 32) | (__u64)minor;
}

static inline __u32 pos2maj_hash(struct file *filp, loff_t pos)
{
	if ((filp->f_flags & FMODE_32BITHASH) ||
                   ^^^^^^
	    (!(filp->f_mode & FMODE_64BITHASH) && is_32bit_api()))
                     ^^^^^^
		return (pos << 1) & 0xffffffff;
	else
		return ((pos >> 32) << 1) & 0xffffffff;
}

Which makes me wonder how much this has been tested?

      	       	      	       	    	     - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html