IMA: Updated KBUILD_MODNAME for IMA files to ima

PATCH Superseded Tushar Sugandhi 4 participants 9 messages Feb 7, 2020 – Feb 10, 2020 1 message not in the archive

Subsystems (4)

Security (LSM) Keys-trusted Integrity measurement architecture (IMA) Extended Verification Module (EVM)

lore

Share & tools

lore

Tushar Sugandhi

author

2020-02-07 19:53 UTC #0 lore raw

PATCH 1/1 IMA: Updated KBUILD_MODNAME for IMA files to ima

Log statements from ima_mok.c, ima_asymmetric_keys.c, and ima_queue_keys.c are prefixed with the respective file names and not with the string "ima".

This change fixes the log statement prefix to be consistent with the rest of the IMA files.

Signed-off-by Tushar Sugandhi <[email protected]>

Reviewed-by Lakshmi Ramasubramanian <[email protected]>

---
 security/integrity/ima/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

security/integrity/ima/Makefile +3 -3

index 064a256f8725..67dabca670e2 100644
@@ -11,6 +11,6 @@ ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
 ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
 ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o
 ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
-obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
-obj-$(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) += ima_asymmetric_keys.o
-obj-$(CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS) += ima_queue_keys.o
+ima-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
+ima-$(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) += ima_asymmetric_keys.o
+ima-$(CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS) += ima_queue_keys.o

Signature · 1 line

2.17.1

Tushar Sugandhi

author

2020-02-07 19:53 UTC #1 lore raw

PATCH 1/1 IMA: Add log statements for failure conditions.

process_buffer_measurement() and ima_alloc_key_entry() functions do not have log messages for failure conditions.

This change adds log statements in the above functions.

Signed-off-by Tushar Sugandhi <[email protected]>

Reviewed-by Lakshmi Ramasubramanian <[email protected]>

---
 security/integrity/ima/ima_main.c       | 4 ++++
 security/integrity/ima/ima_queue_keys.c | 2 ++
 2 files changed, 6 insertions(+)

security/integrity/ima/ima_main.c +4 -0

index 9fe949c6a530..afab796fb765 100644
@@ -757,6 +757,10 @@ void process_buffer_measurement(const void *buf, int size,
 		ima_free_template_entry(entry);
 
 out:
+	if (ret < 0)
+		pr_err("Process buffer measurement failed, result: %d\n",
+			ret);
+
 	return;
 }

security/integrity/ima/ima_queue_keys.c +2 -0

index c87c72299191..2cc52f17ea81 100644
@@ -90,6 +90,8 @@ static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring,
 
 out:
 	if (rc) {
+		pr_err("Key entry allocation failed, result: %d\n",
+			rc);
 		ima_free_key_entry(entry);
 		entry = NULL;
 	}

Signature · 1 line

2.17.1

Mimi Zohar

maintainer

2020-02-09 12:57 UTC #2 lore raw

Re: [PATCH] IMA: Add log statements for failure conditions.

Hi Tushar,

Quoted 7 lines · Tushar Sugandhi

process_buffer_measurement() and ima_alloc_key_entry() functions do not have log messages for failure conditions.
This change adds log statements in the above functions.
Signed-off-by Tushar Sugandhi <[email protected]>
Reviewed-by Lakshmi Ramasubramanian <[email protected]>

The two patches you posted are related. Please group them as a patch set, making this patch 2/2.

In addition, as Shuah Khan suggested for the security/integrity/ directory, "there is an opportunity here to add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt to integrity.h and get rid of duplicate defines." With Joe Perches patch (waiting for it to be re-posted), are all the pr_fmt definitions needed in each file in the integrity/ima directory?

Quoted 10 lines

---
 security/integrity/ima/ima_main.c       | 4 ++++
 security/integrity/ima/ima_queue_keys.c | 2 ++
 2 files changed, 6 insertions(+)

security/integrity/ima/ima_main.c +0 -0

index 9fe949c6a530..afab796fb765 100644

security/integrity/ima/ima_main.c void process_buffer_measurement(const void *buf, int size, · line 762

Quoted 6 lines

security/integrity/ima/ima_main.c +3 -0

@@ -757,6 +757,10 @@ void process_buffer_measurement(const void *buf, int size,
 		ima_free_template_entry(entry);
 
 out:
+	if (ret < 0)
+		pr_err("Process buffer measurement failed, result: %d\n",
+			ret);

There's no reason to split the statement like this. The joined line is less than 80 characters.

Quoted 9 lines

security/integrity/ima/ima_main.c +1 -0
+
 	return;
 }
 
security/integrity/ima/ima_queue_keys.c +0 -0
index c87c72299191..2cc52f17ea81 100644

security/integrity/ima/ima_queue_keys.c static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring, · line 94

Quoted 5 lines

security/integrity/ima/ima_queue_keys.c +2 -0

@@ -90,6 +90,8 @@ static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring,
 
 out:
 	if (rc) {
+		pr_err("Key entry allocation failed, result: %d\n",
+			rc);

ditto

security/integrity/ima/ima_queue_keys.c static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring, · line 97

Quoted 3 lines

security/integrity/ima/ima_queue_keys.c +0 -0
 		ima_free_key_entry(entry);
 		entry = NULL;
 	}

thanks,

Mimi

Joe Perches

2020-02-10 02:47 UTC #3 lore raw

Re: [PATCH] IMA: Add log statements for failure conditions.

Quoted 14 lines · Mimi Zohar

Hi Tushar,
Quoted 7 lines · Tushar Sugandhi
process_buffer_measurement() and ima_alloc_key_entry() functions do not have log messages for failure conditions.
This change adds log statements in the above functions.
Signed-off-by Tushar Sugandhi <[email protected]>
Reviewed-by Lakshmi Ramasubramanian <[email protected]>
The two patches you posted are related. Please group them as a patch set, making this patch 2/2.

Quoted 6 lines

In addition, as Shuah Khan suggested for the security/integrity/ directory, "there is an opportunity here to add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt to integrity.h and get rid of duplicate defines." With Joe Perches patch (waiting for it to be re-posted), are all the pr_fmt definitions needed in each file in the integrity/ima directory?

btw Tushar and Lakshmi:

I am not formally submitting a patch here.

I was just making suggestions and please do
with it as you think appropriate.

and welcome, cheers, Joe

Quoted 45 lines

Quoted 10 lines
---
 security/integrity/ima/ima_main.c       | 4 ++++
 security/integrity/ima/ima_queue_keys.c | 2 ++
 2 files changed, 6 insertions(+)
security/integrity/ima/ima_main.c +0 -0
index 9fe949c6a530..afab796fb765 100644
security/integrity/ima/ima_main.c void process_buffer_measurement(const void *buf, int size, · line 762
Quoted 6 lines
security/integrity/ima/ima_main.c +3 -0
@@ -757,6 +757,10 @@ void process_buffer_measurement(const void *buf, int size,
 		ima_free_template_entry(entry);
 
 out:
+	if (ret < 0)
+		pr_err("Process buffer measurement failed, result: %d\n",
+			ret);
There's no reason to split the statement like this. The joined line is less than 80 characters.
Quoted 9 lines
security/integrity/ima/ima_main.c +1 -0
+
 	return;
 }
 
security/integrity/ima/ima_queue_keys.c +0 -0
index c87c72299191..2cc52f17ea81 100644
security/integrity/ima/ima_queue_keys.c static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring, · line 94
Quoted 5 lines
security/integrity/ima/ima_queue_keys.c +2 -0
@@ -90,6 +90,8 @@ static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring,
 
 out:
 	if (rc) {
+		pr_err("Key entry allocation failed, result: %d\n",
+			rc);
ditto
security/integrity/ima/ima_queue_keys.c static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring, · line 97
Quoted 3 lines
security/integrity/ima/ima_queue_keys.c +0 -0
 		ima_free_key_entry(entry);
 		entry = NULL;
 	}
thanks,
Mimi

Lakshmi Ramasubramanian

2020-02-10 16:40 UTC #4 lore raw

Re: [PATCH] IMA: Add log statements for failure conditions.

Quoted 5 lines · Joe Perches

Quoted 5 lines
In addition, as Shuah Khan suggested for the security/integrity/ directory, "there is an opportunity here to add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt to integrity.h and get rid of duplicate defines."

Good point - we'll make that change.

With Joe Perches patch (waiting for it to be re-posted),

Quoted 9 lines

Quoted 2 lines
are all the pr_fmt definitions needed in each file in the integrity/ima directory?
btw Tushar and Lakshmi:
I am not formally submitting a patch here.
I was just making suggestions and please do
with it as you think appropriate.

Joe - it's not clear to me what you are suggesting. We'll move the #define for pr_fmt to integrity.h.

What's other changes are you proposing?

Quoted 8 lines

Quoted 8 lines
security/integrity/ima/ima_main.c void process_buffer_measurement(const void *buf, int size, · line 762
Quoted 5 lines
out:
security/integrity/ima/ima_main.c +3 -0
+	if (ret < 0)
+		pr_err("Process buffer measurement failed, result: %d\n",
+			ret);
There's no reason to split the statement like this. The joined line is less than 80 characters.

Agree.

thanks,
-lakshmi

Joe Perches

2020-02-10 16:51 UTC #5 lore raw

Re: [PATCH] IMA: Add log statements for failure conditions.

Quoted 17 lines · Lakshmi Ramasubramanian

Quoted 4 lines · Joe Perches
Quoted 4 lines
In addition, as Shuah Khan suggested for the security/integrity/ directory, "there is an opportunity here to add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt to integrity.h and get rid of duplicate defines."
Good point - we'll make that change.
With Joe Perches patch (waiting for it to be re-posted),
Quoted 7 lines
Quoted 2 lines
are all the pr_fmt definitions needed in each file in the integrity/ima directory?
btw Tushar and Lakshmi:
I am not formally submitting a patch here.

Quoted 7 lines

Quoted 2 lines
I was just making suggestions and please do
with it as you think appropriate.
Joe - it's not clear to me what you are suggesting. We'll move the #define for pr_fmt to integrity.h.
What's other changes are you proposing?

https://lore.kernel.org/lkml/[email protected]/

Tushar Sugandhi

author

2020-02-10 21:42 UTC #6 lore raw

Re: [PATCH] IMA: Add log statements for failure conditions.

Quoted 21 lines · Joe Perches

Quoted 20 lines · Lakshmi Ramasubramanian
Quoted 4 lines · Joe Perches
Quoted 4 lines
In addition, as Shuah Khan suggested for the security/integrity/ directory, "there is an opportunity here to add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt to integrity.h and get rid of duplicate defines."
Good point - we'll make that change.
With Joe Perches patch (waiting for it to be re-posted),
Quoted 9 lines
Quoted 2 lines
are all the pr_fmt definitions needed in each file in the integrity/ima directory?
btw Tushar and Lakshmi:
I am not formally submitting a patch here.
I was just making suggestions and please do
with it as you think appropriate.

Quoted 7 lines

Quoted 4 lines
Joe - it's not clear to me what you are suggesting. We'll move the #define for pr_fmt to integrity.h.
What's other changes are you proposing?
https://lore.kernel.org/lkml/[email protected]/

Thanks Joe.

Joe, Shuah:

Could one of you please clarify if the changes proposed in the above URL will be part of Shuah's future patchset?

Or should I include those in my patchset? I am referring to the following snippet in security/integrity/integrity.h.

+#ifdef pr_fmt
+#undef pr_fmt
+#endif
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " KBUILD_BASENAME ": " fmt +

If I add the above in my patchset, I believe I should remove #defines for pr_fmt in the .c files under /security/integrity? (except the below one) latform_certs/efi_parser.c:#define pr_fmt(fmt) "EFI: "fmt

Please let me know.

Thanks,
Tushar

A message that is not in the archive (sent off-list or not yet received): [email protected]

Mimi Zohar

maintainer

2020-02-10 21:46 UTC #8 lore raw

Re: [PATCH] IMA: Add log statements for failure conditions.

Hi Tushar,

Quoted 27 lines · Tushar Sugandhi

Quoted 16 lines · Joe Perches
Quoted 15 lines · Lakshmi Ramasubramanian
Quoted 4 lines · Joe Perches
Quoted 4 lines
In addition, as Shuah Khan suggested for the security/integrity/ directory, "there is an opportunity here to add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt to integrity.h and get rid of duplicate defines."
Good point - we'll make that change.
With Joe Perches patch (waiting for it to be re-posted),
Quoted 6 lines
Quoted 2 lines
are all the pr_fmt definitions needed in each file in the integrity/ima directory?
btw Tushar and Lakshmi:
I am not formally submitting a patch here.
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
I was just making suggestions and please do
with it as you think appropriate.
Joe - it's not clear to me what you are suggesting. We'll move the #define for pr_fmt to integrity.h.
What's other changes are you proposing?
https://lore.kernel.org/lkml/[email protected]/
Thanks Joe.

Quoted 7 lines

Joe, Shuah:
Could one of you please clarify if the changes proposed in the above URL will be part of Shuah's future patchset?
Or should I include those in my patchset? I am referring to the following snippet in security/integrity/integrity.h.

Joe is saying that he made some suggestions, which Shuah commented on, but has no intention of posting a formal patch. The end result of that discussion is to define pr_fmt once in integrity/integrity.h and remove any duplication in the integrity/ files.

I'd appreciate your including that change in this patch set, and if needed a similar one in ima/ima.h.

thanks,

Mimi

Tushar Sugandhi

author

2020-02-10 22:19 UTC #9 lore raw

Re: [PATCH] IMA: Add log statements for failure conditions.

Thanks Mimi.

Quoted 38 lines · Mimi Zohar

Hi Tushar,
Quoted 27 lines · Tushar Sugandhi
Quoted 16 lines · Joe Perches
Quoted 15 lines · Lakshmi Ramasubramanian
Quoted 4 lines · Joe Perches
Quoted 4 lines
In addition, as Shuah Khan suggested for the security/integrity/ directory, "there is an opportunity here to add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt to integrity.h and get rid of duplicate defines."
Good point - we'll make that change.
With Joe Perches patch (waiting for it to be re-posted),
Quoted 6 lines
Quoted 2 lines
are all the pr_fmt definitions needed in each file in the integrity/ima directory?
btw Tushar and Lakshmi:
I am not formally submitting a patch here.
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
I was just making suggestions and please do
with it as you think appropriate.
Joe - it's not clear to me what you are suggesting. We'll move the #define for pr_fmt to integrity.h.
What's other changes are you proposing?
https://lore.kernel.org/lkml/[email protected]/
Thanks Joe.
Quoted 7 lines
Joe, Shuah:
Could one of you please clarify if the changes proposed in the above URL will be part of Shuah's future patchset?
Or should I include those in my patchset? I am referring to the following snippet in security/integrity/integrity.h.

Quoted 7 lines

Joe is saying that he made some suggestions, which Shuah commented on, but has no intention of posting a formal patch. The end result of that discussion is to define pr_fmt once in integrity/integrity.h and remove any duplication in the integrity/ files.
I'd appreciate your including that change in this patch set, and if needed a similar one in ima/ima.h.

I will add the proposed change to pr_fmt to this patchset. I will also check if a similar change is needed in ima/ima.h.

Quoted 5 lines

thanks,
Mimi