diff mbox series

[1/2] ima: Directly assign the ima_default_policy pointer to ima_rules

Message ID 20200603150821.8607-1-roberto.sassu@huawei.com (mailing list archive)
State New, archived
Headers show
Series [1/2] ima: Directly assign the ima_default_policy pointer to ima_rules | expand

Commit Message

Roberto Sassu June 3, 2020, 3:08 p.m. UTC
This patch prevents the following oops:

[   10.771813] BUG: kernel NULL pointer dereference, address: 0000000000000
[...]
[   10.779790] RIP: 0010:ima_match_policy+0xf7/0xb80
[...]
[   10.798576] Call Trace:
[   10.798993]  ? ima_lsm_policy_change+0x2b0/0x2b0
[   10.799753]  ? inode_init_owner+0x1a0/0x1a0
[   10.800484]  ? _raw_spin_lock+0x7a/0xd0
[   10.801592]  ima_must_appraise.part.0+0xb6/0xf0
[   10.802313]  ? ima_fix_xattr.isra.0+0xd0/0xd0
[   10.803167]  ima_must_appraise+0x4f/0x70
[   10.804004]  ima_post_path_mknod+0x2e/0x80
[   10.804800]  do_mknodat+0x396/0x3c0

It occurs when there is a failure during IMA initialization, and
ima_init_policy() is not called. IMA hooks still call ima_match_policy()
but ima_rules is NULL. This patch prevents the crash by directly assigning
the ima_default_policy pointer to ima_rules when ima_rules is defined. This
wouldn't alter the existing behavior, as ima_rules is always set at the end
of ima_init_policy().

Cc: stable@vger.kernel.org # 3.7.x
Fixes: 07f6a79415d7d ("ima: add appraise action keywords and default rules")
Reported-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_policy.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Mimi Zohar June 3, 2020, 9:30 p.m. UTC | #1
On Wed, 2020-06-03 at 17:08 +0200, Roberto Sassu wrote:
> This patch prevents the following oops:
> 
> [   10.771813] BUG: kernel NULL pointer dereference, address: 0000000000000
> [...]
> [   10.779790] RIP: 0010:ima_match_policy+0xf7/0xb80
> [...]
> [   10.798576] Call Trace:
> [   10.798993]  ? ima_lsm_policy_change+0x2b0/0x2b0
> [   10.799753]  ? inode_init_owner+0x1a0/0x1a0
> [   10.800484]  ? _raw_spin_lock+0x7a/0xd0
> [   10.801592]  ima_must_appraise.part.0+0xb6/0xf0
> [   10.802313]  ? ima_fix_xattr.isra.0+0xd0/0xd0
> [   10.803167]  ima_must_appraise+0x4f/0x70
> [   10.804004]  ima_post_path_mknod+0x2e/0x80
> [   10.804800]  do_mknodat+0x396/0x3c0
> 
> It occurs when there is a failure during IMA initialization, and
> ima_init_policy() is not called. IMA hooks still call ima_match_policy()
> but ima_rules is NULL. This patch prevents the crash by directly assigning
> the ima_default_policy pointer to ima_rules when ima_rules is defined. This
> wouldn't alter the existing behavior, as ima_rules is always set at the end
> of ima_init_policy().
> 
> Cc: stable@vger.kernel.org # 3.7.x
> Fixes: 07f6a79415d7d ("ima: add appraise action keywords and default rules")
> Reported-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Thanks, Roberto!
Sasha Levin June 5, 2020, 2:10 p.m. UTC | #2
Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: 07f6a79415d7 ("ima: add appraise action keywords and default rules").

The bot has tested the following trees: v5.6.15, v5.4.43, v4.19.125, v4.14.182, v4.9.225, v4.4.225.

v5.6.15: Build OK!
v5.4.43: Build OK!
v4.19.125: Build OK!
v4.14.182: Failed to apply! Possible dependencies:
    Unable to calculate

v4.9.225: Failed to apply! Possible dependencies:
    Unable to calculate

v4.4.225: Failed to apply! Possible dependencies:
    38d859f991f3 ("IMA: policy can now be updated multiple times")
    95ee08fa373b ("ima: require signed IMA policy")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?
diff mbox series

Patch

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index ef7f68cc935e..e493063a3c34 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -204,7 +204,7 @@  static struct ima_rule_entry *arch_policy_entry __ro_after_init;
 static LIST_HEAD(ima_default_rules);
 static LIST_HEAD(ima_policy_rules);
 static LIST_HEAD(ima_temp_rules);
-static struct list_head *ima_rules;
+static struct list_head *ima_rules = &ima_default_rules;
 
 /* Pre-allocated buffer used for matching keyrings. */
 static char *ima_keyrings;
@@ -768,7 +768,6 @@  void __init ima_init_policy(void)
 			  ARRAY_SIZE(default_appraise_rules),
 			  IMA_DEFAULT_POLICY);
 
-	ima_rules = &ima_default_rules;
 	ima_update_policy_flag();
 }