Commit a408e4a86b36 ("ima: open a new file instance if no read permissions") tries to create a new file descriptor to calculate a file digest if the file has not been opened with O_RDONLY flag. However, if a new file descriptor cannot be obtained, it sets the FMODE_READ flag to file->f_flags instead of file->f_mode.
This patch fixes this issue by replacing f_flags with f_mode as it was before that commit.
Changelog
v1: - fix comment for f_mode change (suggested by Mimi) - rename modified_flags variable to modified_mode (suggested by Mimi)
Cc: stable@vger.kernel.org # 4.20.x Fixes: a408e4a86b36 ("ima: open a new file instance if no read permissions") Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/ima_crypto.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 5201f5ec2ce4..f3a7f4eb1fc1 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -537,7 +537,7 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash) loff_t i_size; int rc; struct file *f = file; - bool new_file_instance = false, modified_flags = false; + bool new_file_instance = false, modified_mode = false;
/* * For consistency, fail file's opened with the O_DIRECT flag on @@ -557,13 +557,13 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash) f = dentry_open(&file->f_path, flags, file->f_cred); if (IS_ERR(f)) { /* - * Cannot open the file again, lets modify f_flags + * Cannot open the file again, lets modify f_mode * of original and continue */ pr_info_ratelimited("Unable to reopen file for reading.\n"); f = file; - f->f_flags |= FMODE_READ; - modified_flags = true; + f->f_mode |= FMODE_READ; + modified_mode = true; } else { new_file_instance = true; } @@ -581,8 +581,8 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash) out: if (new_file_instance) fput(f); - else if (modified_flags) - f->f_flags &= ~FMODE_READ; + else if (modified_mode) + f->f_mode &= ~FMODE_READ; return rc; }
This patch avoids a kernel panic due to accessing an error pointer set by crypto_alloc_shash(). It occurs especially when there are many files that require an unsupported algorithm, as it would increase the likelihood of the following race condition:
Task A: *tfm = crypto_alloc_shash() <= error pointer Task B: if (*tfm == NULL) <= *tfm is not NULL, use it Task B: rc = crypto_shash_init(desc) <= panic Task A: *tfm = NULL
This patch uses the IS_ERR_OR_NULL macro to determine whether or not a new crypto context must be created.
Cc: stable@vger.kernel.org Fixes: d46eb3699502b ("evm: crypto hash replaced by shash") Co-developed-by: Krzysztof Struczynski krzysztof.struczynski@huawei.com Signed-off-by: Krzysztof Struczynski krzysztof.struczynski@huawei.com Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/evm/evm_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 35682852ddea..77ad1e5a93e4 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -91,7 +91,7 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo) algo = hash_algo_name[hash_algo]; }
- if (*tfm == NULL) { + if (IS_ERR_OR_NULL(*tfm)) { mutex_lock(&mutex); if (*tfm) goto out;
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 beginning of the digest with the number of slots. Also reduce the depth of the hash table by doubling the number of slots.
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 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 467dfdbea25c..6ee458cf124a 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,9 @@ 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); + return (*(unsigned int *)digest % IMA_MEASURE_HTABLE_SIZE); }
#define __ima_hooks(hook) \
From: Roberto Sassu
Sent: 27 April 2020 11:29 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 beginning of the digest with the number of slots. Also reduce the depth of the hash table by doubling the number of slots.
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 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 467dfdbea25c..6ee458cf124a 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,9 @@ 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);
- return (*(unsigned int *)digest % IMA_MEASURE_HTABLE_SIZE);
That almost certainly isn't right. It falls foul of the *(integer_type *)ptr being almost always wrong.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
-----Original Message----- From: David Laight [mailto:David.Laight@ACULAB.COM] Sent: Monday, April 27, 2020 1:00 PM To: Roberto Sassu roberto.sassu@huawei.com; zohar@linux.ibm.com; rgoldwyn@suse.de Cc: linux-integrity@vger.kernel.org; linux-security-module@vger.kernel.org; linux-kernel@vger.kernel.org; Silviu Vlasceanu Silviu.Vlasceanu@huawei.com; Krzysztof Struczynski krzysztof.struczynski@huawei.com; stable@vger.kernel.org Subject: RE: [PATCH v2 3/6] ima: Fix ima digest hash table key calculation
From: Roberto Sassu
Sent: 27 April 2020 11:29 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 beginning of the digest with the number of slots. Also reduce the depth of the hash table by doubling the number of slots.
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 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 467dfdbea25c..6ee458cf124a 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,9 @@ 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);
- return (*(unsigned int *)digest % IMA_MEASURE_HTABLE_SIZE);
That almost certainly isn't right. It falls foul of the *(integer_type *)ptr being almost always wrong.
I didn't find the problem. Can you please explain?
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli
David
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
From: Roberto Sassu
Sent: 27 April 2020 13:51
...
-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);
- return (*(unsigned int *)digest % IMA_MEASURE_HTABLE_SIZE);
That almost certainly isn't right. It falls foul of the *(integer_type *)ptr being almost always wrong.
I didn't find the problem. Can you please explain?
The general problem with *(int_type *)ptr is that it does completely the wrong thing if 'ptr' is the address of a larger integer type on a big-endian system. You may also get a misaligned access trap.
In this case I guess that digest is actually u8[SHA1_DIGEST_SIZE]. Maybe what you should return is: (digest[0] | digest[1] << 8) % IMA_MEASURE_HTABLE_SIZE; and comment that there is no point taking a hash of part of a SHA1 digest.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
-----Original Message----- From: David Laight [mailto:David.Laight@ACULAB.COM] Sent: Monday, April 27, 2020 4:28 PM To: Roberto Sassu roberto.sassu@huawei.com; zohar@linux.ibm.com; rgoldwyn@suse.de Cc: linux-integrity@vger.kernel.org; linux-security-module@vger.kernel.org; linux-kernel@vger.kernel.org; Silviu Vlasceanu Silviu.Vlasceanu@huawei.com; Krzysztof Struczynski krzysztof.struczynski@huawei.com; stable@vger.kernel.org Subject: RE: [PATCH v2 3/6] ima: Fix ima digest hash table key calculation
From: Roberto Sassu
Sent: 27 April 2020 13:51
...
-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);
- return (*(unsigned int *)digest % IMA_MEASURE_HTABLE_SIZE);
That almost certainly isn't right. It falls foul of the *(integer_type *)ptr being almost always wrong.
I didn't find the problem. Can you please explain?
The general problem with *(int_type *)ptr is that it does completely the wrong thing if 'ptr' is the address of a larger integer type on a big-endian system. You may also get a misaligned access trap.
In this case I guess that digest is actually u8[SHA1_DIGEST_SIZE]. Maybe what you should return is: (digest[0] | digest[1] << 8) % IMA_MEASURE_HTABLE_SIZE; and comment that there is no point taking a hash of part of a SHA1 digest.
Ok, thanks.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli
David
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
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(-)
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) \
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)
From: Krzysztof Struczynski krzysztof.struczynski@huawei.com
After adding the new add_rule() function in commit c52657d93b05 ("ima: refactor ima_init_policy()"), all appraisal flags are added to the temp_ima_appraise variable. Revert to the previous behavior instead of removing build_ima_appraise, to benefit from the protection offered by __ro_after_init.
The mentioned commit introduced a bug, as it makes all the flags modifiable, while build_ima_appraise flags can be protected with __ro_after_init.
Changelog
v1: - set build_ima_appraise instead of removing it (suggested by Mimi)
Cc: stable@vger.kernel.org # 5.0.x Fixes: c52657d93b05 ("ima: refactor ima_init_policy()") 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_policy.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index ea9b991f0232..ef7f68cc935e 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -643,8 +643,14 @@ static void add_rules(struct ima_rule_entry *entries, int count,
list_add_tail(&entry->list, &ima_policy_rules); } - if (entries[i].action == APPRAISE) - temp_ima_appraise |= ima_appraise_flag(entries[i].func); + if (entries[i].action == APPRAISE) { + if (entries != build_appraise_rules) + temp_ima_appraise |= + ima_appraise_flag(entries[i].func); + else + build_ima_appraise |= + ima_appraise_flag(entries[i].func); + } } }
This patch fixes the return value of ima_write_policy() when a new policy is directly passed to IMA and the current policy requires appraisal of the file containing the policy. Currently, if appraisal is not in ENFORCE mode, ima_write_policy() returns 0 and leads user space applications to an endless loop. Fix this issue by denying the operation regardless of the appraisal mode.
Changelog
v1: - deny the operation in all cases (suggested by Mimi, Krzysztof)
Cc: stable@vger.kernel.org # 4.10.x Fixes: 19f8a84713edc ("ima: measure and appraise the IMA policy itself") Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/ima_fs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 8b030a1c5e0d..e3fcad871861 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -338,8 +338,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, "policy_update", "signed policy required", 1, 0); - if (ima_appraise & IMA_APPRAISE_ENFORCE) - result = -EACCES; + result = -EACCES; } else { result = ima_parse_add_rule(data); }
Hi Roberto,
On Mon, 2020-04-27 at 12:31 +0200, Roberto Sassu wrote:
This patch fixes the return value of ima_write_policy() when a new policy is directly passed to IMA and the current policy requires appraisal of the file containing the policy. Currently, if appraisal is not in ENFORCE mode, ima_write_policy() returns 0 and leads user space applications to an endless loop. Fix this issue by denying the operation regardless of the appraisal mode.
Changelog
v1:
- deny the operation in all cases (suggested by Mimi, Krzysztof)
Relatively recently, people have moved away from including the "Changelog" in the upstream commit. (I'm removing them now.)
Cc: stable@vger.kernel.org # 4.10.x Fixes: 19f8a84713edc ("ima: measure and appraise the IMA policy itself") Signed-off-by: Roberto Sassu roberto.sassu@huawei.com
Without the Changelog, the only way of acknowledging people's contributions is by including their tags. Krzysztof, did you want to add your "Reviewed-by" tag?
People have started putting the Changelog or any comments immediately below the separator "---" here.
thanks,
Mimi
security/integrity/ima/ima_fs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 8b030a1c5e0d..e3fcad871861 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -338,8 +338,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, "policy_update", "signed policy required", 1, 0);
if (ima_appraise & IMA_APPRAISE_ENFORCE)result = -EACCES;
result = -EACCES;
Hi Mimi,
-----Original Message----- From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Tuesday, April 28, 2020 7:47 PM To: Roberto Sassu roberto.sassu@huawei.com; Krzysztof Struczynski krzysztof.struczynski@huawei.com Cc: linux-integrity@vger.kernel.org; linux-security-module@vger.kernel.org; linux-kernel@vger.kernel.org; Silviu Vlasceanu Silviu.Vlasceanu@huawei.com; Krzysztof Struczynski krzysztof.struczynski@huawei.com; stable@vger.kernel.org Subject: Re: [PATCH v2 6/6] ima: Fix return value of ima_write_policy()
Hi Roberto,
On Mon, 2020-04-27 at 12:31 +0200, Roberto Sassu wrote:
This patch fixes the return value of ima_write_policy() when a new policy is directly passed to IMA and the current policy requires appraisal of the file containing the policy. Currently, if appraisal is not in ENFORCE mode, ima_write_policy() returns 0 and leads user space applications to an endless loop. Fix this issue by denying the operation regardless of the appraisal mode.
Changelog
v1:
- deny the operation in all cases (suggested by Mimi, Krzysztof)
Relatively recently, people have moved away from including the "Changelog" in the upstream commit. (I'm removing them now.)
Cc: stable@vger.kernel.org # 4.10.x Fixes: 19f8a84713edc ("ima: measure and appraise the IMA policy itself") Signed-off-by: Roberto Sassu roberto.sassu@huawei.com
Without the Changelog, the only way of acknowledging people's contributions is by including their tags. Krzysztof, did you want to add your "Reviewed-by" tag?
Please add: Reviewed-by: Krzysztof Struczynski krzysztof.struczynski@huawei.com
Thanks, Krzysztof
People have started putting the Changelog or any comments immediately below the separator "---" here.
thanks,
Mimi
security/integrity/ima/ima_fs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 8b030a1c5e0d..e3fcad871861 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -338,8 +338,7 @@ static ssize_t ima_write_policy(struct file *file, const
char __user *buf,
integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, "policy_update", "signed policy required", 1, 0);
if (ima_appraise & IMA_APPRAISE_ENFORCE)result = -EACCES;
result = -EACCES;
On 12:28 27/04, Roberto Sassu wrote:
Commit a408e4a86b36 ("ima: open a new file instance if no read permissions") tries to create a new file descriptor to calculate a file digest if the file has not been opened with O_RDONLY flag. However, if a new file descriptor cannot be obtained, it sets the FMODE_READ flag to file->f_flags instead of file->f_mode.
This patch fixes this issue by replacing f_flags with f_mode as it was before that commit.
Thanks for fixing this.
Reviewed-by: Goldwyn Rodrigues rgoldwyn@suse.com
Changelog
v1:
- fix comment for f_mode change (suggested by Mimi)
- rename modified_flags variable to modified_mode (suggested by Mimi)
Cc: stable@vger.kernel.org # 4.20.x Fixes: a408e4a86b36 ("ima: open a new file instance if no read permissions") Signed-off-by: Roberto Sassu roberto.sassu@huawei.com
security/integrity/ima/ima_crypto.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 5201f5ec2ce4..f3a7f4eb1fc1 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -537,7 +537,7 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash) loff_t i_size; int rc; struct file *f = file;
- bool new_file_instance = false, modified_flags = false;
- bool new_file_instance = false, modified_mode = false;
/* * For consistency, fail file's opened with the O_DIRECT flag on @@ -557,13 +557,13 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash) f = dentry_open(&file->f_path, flags, file->f_cred); if (IS_ERR(f)) { /*
* Cannot open the file again, lets modify f_flags
* Cannot open the file again, lets modify f_mode * of original and continue */ pr_info_ratelimited("Unable to reopen file for reading.\n"); f = file;
f->f_flags |= FMODE_READ;modified_flags = true;
f->f_mode |= FMODE_READ;modified_mode = true;@@ -581,8 +581,8 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash) out: if (new_file_instance) fput(f);
- else if (modified_flags)
f->f_flags &= ~FMODE_READ;
- else if (modified_mode)
f->f_mode &= ~FMODE_READ;} -- 2.17.1
linux-stable-mirror@lists.linaro.org
-
David Laight -
Goldwyn Rodrigues -
Krzysztof Struczynski -
Mimi Zohar -
Roberto Sassu