Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1660886 > unrolled thread

[PATCH] ila_xlat: add missing hash secret initialization

Back to article view | Back to linux.kernel

Contents

  [PATCH] ila_xlat: add missing hash secret initialization Arnd Bergmann <arnd@arndb.de> - 2017-06-08 10:00 +0200
    Re: [PATCH] ila_xlat: add missing hash secret initialization David Miller <davem@davemloft.net> - 2017-06-08 21:40 +0200
    Re: [PATCH] ila_xlat: add missing hash secret initialization Tom Herbert <tom@herbertland.com> - 2017-06-08 22:20 +0200

#1660886 — [PATCH] ila_xlat: add missing hash secret initialization

While discussing the possible merits of clang warning about unused initialized
functions, I found one function that was clearly meant to be called but
never actually is.

__ila_hash_secret_init() initializes the hash value for the ila locator,
apparently this is intended to prevent hash collision attacks, but this ends
up being a read-only zero constant since there is no caller. I could find
no indication of why it was never called, the earliest patch submission
for the module already was like this. If my interpretation is right, we
certainly want to backport the patch to stable kernels as well.

I considered adding it to the ila_xlat_init callback, but for best effect
the random data is read as late as possible, just before it is first used.
The underlying net_get_random_once() is already highly optimized to avoid
overhead when called frequently.

Fixes: 7f00feaf1076 ("ila: Add generic ILA translation facility")
Cc: stable@vger.kernel.org
Link: https://www.spinics.net/lists/kernel/msg2527243.html
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/ipv6/ila/ila_xlat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv6/ila/ila_xlat.c b/net/ipv6/ila/ila_xlat.c
index 2fd5ca151dcf..77f7f8c7d93d 100644
--- a/net/ipv6/ila/ila_xlat.c
+++ b/net/ipv6/ila/ila_xlat.c
@@ -62,6 +62,7 @@ static inline u32 ila_locator_hash(struct ila_locator loc)
 {
 	u32 *v = (u32 *)loc.v32;
 
+	__ila_hash_secret_init();
 	return jhash_2words(v[0], v[1], hashrnd);
 }
 
-- 
2.9.0

[toc] | [next] | [standalone]

#1661617

From: Arnd Bergmann <arnd@arndb.de>
Date: Thu,  8 Jun 2017 09:54:24 +0200

> While discussing the possible merits of clang warning about unused initialized
> functions, I found one function that was clearly meant to be called but
> never actually is.
> 
> __ila_hash_secret_init() initializes the hash value for the ila locator,
> apparently this is intended to prevent hash collision attacks, but this ends
> up being a read-only zero constant since there is no caller. I could find
> no indication of why it was never called, the earliest patch submission
> for the module already was like this. If my interpretation is right, we
> certainly want to backport the patch to stable kernels as well.
> 
> I considered adding it to the ila_xlat_init callback, but for best effect
> the random data is read as late as possible, just before it is first used.
> The underlying net_get_random_once() is already highly optimized to avoid
> overhead when called frequently.
> 
> Fixes: 7f00feaf1076 ("ila: Add generic ILA translation facility")
> Cc: stable@vger.kernel.org
> Link: https://www.spinics.net/lists/kernel/msg2527243.html
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Yikes, good catch, applied and queued up for -stable.

[toc] | [prev] | [next] | [standalone]

#1661649

On Thu, Jun 8, 2017 at 12:54 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> While discussing the possible merits of clang warning about unused initialized
> functions, I found one function that was clearly meant to be called but
> never actually is.
>
> __ila_hash_secret_init() initializes the hash value for the ila locator,
> apparently this is intended to prevent hash collision attacks, but this ends
> up being a read-only zero constant since there is no caller. I could find
> no indication of why it was never called, the earliest patch submission
> for the module already was like this. If my interpretation is right, we
> certainly want to backport the patch to stable kernels as well.
>
> I considered adding it to the ila_xlat_init callback, but for best effect
> the random data is read as late as possible, just before it is first used.
> The underlying net_get_random_once() is already highly optimized to avoid
> overhead when called frequently.
>
> Fixes: 7f00feaf1076 ("ila: Add generic ILA translation facility")
> Cc: stable@vger.kernel.org
> Link: https://www.spinics.net/lists/kernel/msg2527243.html
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  net/ipv6/ila/ila_xlat.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv6/ila/ila_xlat.c b/net/ipv6/ila/ila_xlat.c
> index 2fd5ca151dcf..77f7f8c7d93d 100644
> --- a/net/ipv6/ila/ila_xlat.c
> +++ b/net/ipv6/ila/ila_xlat.c
> @@ -62,6 +62,7 @@ static inline u32 ila_locator_hash(struct ila_locator loc)
>  {
>         u32 *v = (u32 *)loc.v32;
>
> +       __ila_hash_secret_init();
>         return jhash_2words(v[0], v[1], hashrnd);
>  }
>
> --
> 2.9.0
>

Thanks Arnd!

Acked-by: Tom Herbert <tom@herbertland.com>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web