On Sep 23, 2014, at 12:16, "George Spelvin" <linux@xxxxxxxxxxx> wrote:
>
> This uses SipHash-2-4 for directory hashing.
>
> Signed-off-by: George Spelvin <linux@xxxxxxxxxxx>
> ---
> The actual feature. Pretty simple, no?
Simple, except that it introduces a serious bug in the code. It duplicates
the previous on-disk encoding of:
DX_HASH_LEGACY | DX_HASH_UNSIGNED_DELTA = 3
with:
DX_HASH_SIPHASH24 3
and that will mean old filesystems would be considered corrupted.
Due to the changes of your previous patch, it is not at all clear at the
definition of DX_HASH_* that the values 3, 4, 5 are already used.
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index cda8dd9c..13420bd4 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1737,6 +1737,7 @@ static inline __le16 ext4_rec_len_to_disk(unsigned len, unsigned blocksize)
> #define DX_HASH_LEGACY 0
> #define DX_HASH_HALF_MD4 1
> #define DX_HASH_TEA 2
> +#define DX_HASH_SIPHASH24 3
This should still explicitly #define the DX_HASH_LEGACY_UNSIGNED
values so that it is clear the values are already in use, otherwise we
expose ourselves to this bug repeatedly in the future.
Cheers, Andreas
> /*
> * For historical reasons, the first three hash algorithms
> @@ -1744,7 +1745,7 @@ static inline __le16 ext4_rec_len_to_disk(unsigned len, unsigned blocksize)
> * use only, define some extra values outside the range of
> * what's allowed on disk.
> */
> -#define DX_HASH_UNSIGNED_DELTA 3
> +#define DX_HASH_UNSIGNED_DELTA 4
>
> #define DX_HASH_LEGACY_UNSIGNED (DX_HASH_LEGACY + DX_HASH_UNSIGNED_DELTA)
> #define DX_HASH_HALF_MD4_UNSIGNED (DX_HASH_HALF_MD4 + DX_HASH_UNSIGNED_DELTA)
> diff --git a/fs/ext4/hash.c b/fs/ext4/hash.c
> index 5186e09a..584639f4 100644
> --- a/fs/ext4/hash.c
> +++ b/fs/ext4/hash.c
> @@ -140,6 +140,7 @@ int ext4fs_dirhash(const char *name, int len, struct dx_hash_info *hinfo)
> {
> __u32 hash;
> __u32 minor_hash = 0;
> + __u64 hash64;
> int i;
> __u32 in[8], buf[4];
> void (*str2hashbuf)(const char *, int, __u32 *, int) =
> @@ -192,6 +193,11 @@ int ext4fs_dirhash(const char *name, int len, struct dx_hash_info *hinfo)
> hash = buf[0];
> minor_hash = buf[1];
> break;
> + case DX_HASH_SIPHASH24:
> + hash64 = siphash24(name, len, hinfo->seed);
> + hash = (__u32)hash64;
> + minor_hash = hash64 >> 32;
> + break;
> default:
> hinfo->hash = 0;
> return -1;
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 9e6eced1..922d6b48 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -685,7 +685,8 @@ dx_probe(const struct qstr *d_name, struct inode *dir,
> goto fail;
> }
> root = (struct dx_root *) bh->b_data;
> - if (root->info.hash_version != DX_HASH_TEA &&
> + if (root->info.hash_version != DX_HASH_SIPHASH24 &&
> + root->info.hash_version != DX_HASH_TEA &&
> root->info.hash_version != DX_HASH_HALF_MD4 &&
> root->info.hash_version != DX_HASH_LEGACY) {
> ext4_warning(dir->i_sb, "Unrecognised inode hash code %d",
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
