diff mbox series

[RESEND,v2,3/6] ima: Fix ima digest hash table key calculation

Message ID 20200428073010.25631-1-roberto.sassu@huawei.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Roberto Sassu April 28, 2020, 7:30 a.m. UTC
From: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>

Function hash_long() accepts unsigned long, while currently only one byte
is passed from ima_hash_key(), which calculates a key for ima_htable.

Given that hashing the digest does not give clear benefits compared to
using the digest itself, remove hash_long() and return the modulus
calculated on the first two bytes of the digest with the number of slots.
Also reduce the depth of the hash table by doubling the number of slots.

Changelog

v2: directly access the first two bytes of the digest to avoid memory
    access issues on big endian systems (suggested by David Laight)

Cc: stable@vger.kernel.org
Fixes: 3323eec921ef ("integrity: IMA as an integrity service provider")
Co-developed-by: Roberto Sassu <roberto.sassu@huawei.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Signed-off-by: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>
---
 security/integrity/ima/ima.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

David Laight April 30, 2020, 8:03 a.m. UTC | #1
From: Roberto Sassu
> Sent: 28 April 2020 08:30
> From: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>
> 
> Function hash_long() accepts unsigned long, while currently only one byte
> is passed from ima_hash_key(), which calculates a key for ima_htable.
> 
> Given that hashing the digest does not give clear benefits compared to
> using the digest itself, remove hash_long() and return the modulus
> calculated on the first two bytes of the digest with the number of slots.
> Also reduce the depth of the hash table by doubling the number of slots.
> 
> Changelog
> 
> v2: directly access the first two bytes of the digest to avoid memory
>     access issues on big endian systems (suggested by David Laight)
> 
> Cc: stable@vger.kernel.org
> Fixes: 3323eec921ef ("integrity: IMA as an integrity service provider")
> Co-developed-by: Roberto Sassu <roberto.sassu@huawei.com>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Signed-off-by: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>

Acked-by: David.Laight@aculab.com

> ---
>  security/integrity/ima/ima.h | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 467dfdbea25c..02796473238b 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -36,7 +36,7 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
>  #define IMA_DIGEST_SIZE		SHA1_DIGEST_SIZE
>  #define IMA_EVENT_NAME_LEN_MAX	255
> 
> -#define IMA_HASH_BITS 9
> +#define IMA_HASH_BITS 10
>  #define IMA_MEASURE_HTABLE_SIZE (1 << IMA_HASH_BITS)
> 
>  #define IMA_TEMPLATE_FIELD_ID_MAX_LEN	16
> @@ -179,9 +179,10 @@ struct ima_h_table {
>  };
>  extern struct ima_h_table ima_htable;
> 
> -static inline unsigned long ima_hash_key(u8 *digest)
> +static inline unsigned int ima_hash_key(u8 *digest)
>  {
> -	return hash_long(*digest, IMA_HASH_BITS);
> +	/* there is no point in taking a hash of part of a digest */
> +	return (digest[0] | digest[1] << 8) % IMA_MEASURE_HTABLE_SIZE;
>  }
> 
>  #define __ima_hooks(hook)		\
> --
> 2.17.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
diff mbox series

Patch

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 467dfdbea25c..02796473238b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -36,7 +36,7 @@  enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
 #define IMA_DIGEST_SIZE		SHA1_DIGEST_SIZE
 #define IMA_EVENT_NAME_LEN_MAX	255
 
-#define IMA_HASH_BITS 9
+#define IMA_HASH_BITS 10
 #define IMA_MEASURE_HTABLE_SIZE (1 << IMA_HASH_BITS)
 
 #define IMA_TEMPLATE_FIELD_ID_MAX_LEN	16
@@ -179,9 +179,10 @@  struct ima_h_table {
 };
 extern struct ima_h_table ima_htable;
 
-static inline unsigned long ima_hash_key(u8 *digest)
+static inline unsigned int ima_hash_key(u8 *digest)
 {
-	return hash_long(*digest, IMA_HASH_BITS);
+	/* there is no point in taking a hash of part of a digest */
+	return (digest[0] | digest[1] << 8) % IMA_MEASURE_HTABLE_SIZE;
 }
 
 #define __ima_hooks(hook)		\