IMA: improve log messages
Subsystems (4)
v4 cover IMA: improve log messages
The log messages from IMA subsystem should be consistent for better diagnosability and discoverability.
This patch set improves the logging by removing duplicate log formatting macros, adding a consistent prefix to the log messages, and adding new log messages where necessary.
Tushar Sugandhi (3):
add log prefix
add log message to process_buffer_measurement failure conditions add module name prefix to log statements
security/integrity/digsig.c | 2 -- security/integrity/digsig_asymmetric.c | 2 -- security/integrity/evm/evm_crypto.c | 2 -- security/integrity/evm/evm_main.c | 2 -- security/integrity/evm/evm_secfs.c | 2 -- security/integrity/ima/Makefile | 6 +++--- security/integrity/ima/ima_asymmetric_keys.c | 2 -- security/integrity/ima/ima_crypto.c | 2 -- security/integrity/ima/ima_fs.c | 2 -- security/integrity/ima/ima_init.c | 2 -- security/integrity/ima/ima_kexec.c | 1 - security/integrity/ima/ima_main.c | 5 +++-- security/integrity/ima/ima_policy.c | 2 -- security/integrity/ima/ima_queue.c | 2 -- security/integrity/ima/ima_queue_keys.c | 2 -- security/integrity/ima/ima_template.c | 2 -- security/integrity/ima/ima_template_lib.c | 2 -- security/integrity/integrity.h | 6 ++++++ 18 files changed, 12 insertions(+), 34 deletions(-)
Signature · 1 line
2.17.1
PATCH v4 1/3 IMA: Update KBUILD_MODNAME for IMA files to ima
The kbuild Makefile specifies object files for vmlinux in the $(obj-y) lists. These lists depend on the kernel configuration[1].
The kbuild Makefile for IMA combines the object files for IMA into a single object file namely ima.o. All the object files for IMA should be combined into ima.o. But certain object files are being added to their own $(obj-y). This results in the log messages from those modules getting prefixed with their respective base file name, instead of "ima". This is inconsistent with the log messages from the IMA modules that are combined into ima.o.
This change fixes the above issue.
[1] Documentation\kbuild\makefiles.rst
--- security/integrity/ima/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
security/integrity/ima/Makefile +3 -3
@@ -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
Re: [PATCH v4 1/3] IMA: Update KBUILD_MODNAME for IMA files to ima
Hi Mimi,
Quoted 11 lines · Tushar Sugandhi
The kbuild
Makefilespecifies object files for vmlinux in the $(obj-y) lists. These lists depend on the kernel configuration[1].The kbuild
Makefilefor IMA combines the object files for IMA into a single object file namely ima.o. All the object files for IMA should be combined into ima.o. But certain object files are being added to their own $(obj-y). This results in the log messages from those modules getting prefixed with their respective base file name, instead of "ima". This is inconsistent with the log messages from the IMA modules that are combined into ima.o.
Quoted 4 lines
This change fixes the above issue.
[1] Documentation\kbuild\makefiles.rst
Is there any feedback on this patch description?
I can address it in the next iteration.
Thanks,
Tushar
Mimi Zohar
maintainerRe: [PATCH v4 1/3] IMA: Update KBUILD_MODNAME for IMA files to ima
Quoted 14 lines · Tushar Sugandhi
Hi Mimi,
Quoted 11 lines · Tushar Sugandhi
The kbuild
Makefilespecifies object files for vmlinux in the $(obj-y) lists. These lists depend on the kernel configuration[1].The kbuild
Makefilefor IMA combines the object files for IMA into a single object file namely ima.o. All the object files for IMA should be combined into ima.o. But certain object files are being added to their own $(obj-y). This results in the log messages from those modules getting prefixed with their respective base file name, instead of "ima". This is inconsistent with the log messages from the IMA modules that are combined into ima.o.
Quoted 6 lines
Quoted 4 lines
This change fixes the above issue.
[1] Documentation\kbuild\makefiles.rst
Is there any feedback on this patch description?
I can address it in the next iteration.
No, it looks good to me.
Mimi
PATCH v4 2/3 IMA: Add log statements for failure conditions
In reply to Tushar Sugandhiprocess_buffer_measurement() does not have log messages for failure conditions.
This change adds a log statement in the above function.
--- security/integrity/ima/ima_main.c | 3 +++ 1 file changed, 3 insertions(+)
security/integrity/ima/ima_main.c +3 -0
@@ -757,6 +757,9 @@ void process_buffer_measurement(const void *buf, int size,ima_free_template_entry (entry); out: +if (ret <0 ) +pr_devel ("%s: failed, result: %d\n" , __func__, ret); +return ; }
Signature · 1 line
2.17.1
Re: [PATCH v4 2/3] IMA: Add log statements for failure conditions
Quoted 8 lines · Tushar Sugandhi
process_buffer_measurement()does not have log messages for failure conditions.This change adds a log statement in the above function.
Signed-off-by Tushar Sugandhi <[email protected]>Reviewed-by Lakshmi Ramasubramanian <[email protected]>Suggested-by Joe Perches <[email protected]>
Quoted 18 lines
--- security/integrity/ima/ima_main.c | 3 +++ 1 file changed, 3 insertions(+)security/integrity/ima/ima_main.c +3
-0@@ -757,6 +757,9 @@ void process_buffer_measurement(const void *buf, int size,ima_free_template_entry (entry); out: +if (ret <0 ) +pr_devel ("%s: failed, result: %d\n" , __func__, ret); +return ; }
PATCH v4 3/3 IMA: Remove duplicate pr_fmt definitions
In reply to Tushar SugandhiThe #define for formatting log messages, pr_fmt, is duplicated in the files under security/integrity.
This change moves the definition to security/integrity/integrity.h and removes the duplicate definitions in the other files under security/integrity.
--- security/integrity/digsig.c | 2 -- security/integrity/digsig_asymmetric.c | 2 -- security/integrity/evm/evm_crypto.c | 2 -- security/integrity/evm/evm_main.c | 2 -- security/integrity/evm/evm_secfs.c | 2 -- security/integrity/ima/ima_asymmetric_keys.c | 2 -- security/integrity/ima/ima_crypto.c | 2 -- security/integrity/ima/ima_fs.c | 2 -- security/integrity/ima/ima_init.c | 2 -- security/integrity/ima/ima_kexec.c | 1 - security/integrity/ima/ima_main.c | 2 -- security/integrity/ima/ima_policy.c | 2 -- security/integrity/ima/ima_queue.c | 2 -- security/integrity/ima/ima_queue_keys.c | 2 -- security/integrity/ima/ima_template.c | 2 -- security/integrity/ima/ima_template_lib.c | 2 -- security/integrity/integrity.h | 6 ++++++ 17 files changed, 6 insertions(+), 31 deletions(-)
security/integrity/digsig.c +0 -2
@@ -6,8 +6,6 @@* Dmitry Kasatkin <[email protected]> */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/err.h> #include <linux/sched.h> #include <linux/slab.h>
security/integrity/digsig_asymmetric.c +0 -2
@@ -6,8 +6,6 @@* Dmitry Kasatkin <[email protected]> */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/err.h> #include <linux/ratelimit.h> #include <linux/key-type.h>
security/integrity/evm/evm_crypto.c +0 -2
@@ -10,8 +10,6 @@* Using root's kernel master key (kmk), calculate the HMAC */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/export.h> #include <linux/crypto.h> #include <linux/xattr.h>
security/integrity/evm/evm_main.c +0 -2
@@ -11,8 +11,6 @@* evm_inode_removexattr, and evm_verifyxattr */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/init.h> #include <linux/crypto.h> #include <linux/audit.h>
security/integrity/evm/evm_secfs.c +0 -2
@@ -10,8 +10,6 @@* - Get the key and enable EVM */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/audit.h> #include <linux/uaccess.h> #include <linux/init.h>
security/integrity/ima/ima_asymmetric_keys.c +0 -2
@@ -9,8 +9,6 @@* create or update. */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <keys/asymmetric-type.h> #include "ima.h"
security/integrity/ima/ima_crypto.c +0 -2
@@ -10,8 +10,6 @@* Calculates md5/sha1 file hash, template hash, boot-aggreate hash */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/kernel.h> #include <linux/moduleparam.h> #include <linux/ratelimit.h>
security/integrity/ima/ima_fs.c +0 -2
@@ -12,8 +12,6 @@* current measurement list and IMA statistics */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/fcntl.h> #include <linux/slab.h> #include <linux/init.h>
security/integrity/ima/ima_init.c +0 -2
@@ -11,8 +11,6 @@* initialization and cleanup functions */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/init.h> #include <linux/scatterlist.h> #include <linux/slab.h>
security/integrity/ima/ima_kexec.c +0 -1
@@ -6,7 +6,6 @@* Thiago Jung Bauermann <[email protected]> * Mimi Zohar <[email protected]> */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt#include <linux/seq_file.h> #include <linux/vmalloc.h>
security/integrity/ima/ima_main.c +0 -2
@@ -15,8 +15,6 @@* and ima_file_check. */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/module.h> #include <linux/file.h> #include <linux/binfmts.h>
security/integrity/ima/ima_policy.c +0 -2
@@ -7,8 +7,6 @@* - initialize default measure policy rules */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/init.h> #include <linux/list.h> #include <linux/fs.h>
security/integrity/ima/ima_queue.c +0 -2
@@ -15,8 +15,6 @@* ever removed or changed during the boot-cycle. */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/rculist.h> #include <linux/slab.h> #include "ima.h"
security/integrity/ima/ima_queue_keys.c +0 -2
@@ -8,8 +8,6 @@* Enables deferred processing of keys */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/workqueue.h> #include <keys/asymmetric-type.h> #include "ima.h"
security/integrity/ima/ima_template.c +0 -2
@@ -9,8 +9,6 @@* Helpers to manage template descriptors. */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/rculist.h> #include "ima.h" #include "ima_template_lib.h"
security/integrity/ima/ima_template_lib.c +0 -2
@@ -9,8 +9,6 @@* Library of supported template fields. */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include "ima_template_lib.h" static bool ima_template_hash_algo_allowed (u8 algo)
security/integrity/integrity.h +6 -0
@@ -6,6 +6,12 @@* Mimi Zohar <[email protected]> */ +#ifdef pr_fmt +#undef pr_fmt +#endif + +#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt +#include <linux/types.h> #include <linux/integrity.h> #include <crypto/sha.h>
Signature · 1 line
2.17.1
Mimi Zohar
maintainerRe: [PATCH v4 3/3] IMA: Remove duplicate pr_fmt definitions
Quoted 6 lines · Tushar Sugandhi
The #define for formatting log messages,
pr_fmt, is duplicated in the files undersecurity/integrity.This change moves the definition to
security/integrity/integrity.hand removes the duplicate definitions in the other files undersecurity/integrity.
A number of files under security/integrity, "pr_fmt" was not defined. As a result of this patch, messages in those files did change. Please include in this patch description a list of the updated messages. This includes messages in iint.c and under integrity/evm.
thanks,
Mimi
Quoted 249 lines
Signed-off-by Tushar Sugandhi <[email protected]>Reviewed-by Lakshmi Ramasubramanian <[email protected]>Suggested-by Joe Perches <[email protected]>Suggested-by Shuah Khan <[email protected]>--- security/integrity/digsig.c | 2 -- security/integrity/digsig_asymmetric.c | 2 -- security/integrity/evm/evm_crypto.c | 2 -- security/integrity/evm/evm_main.c | 2 -- security/integrity/evm/evm_secfs.c | 2 -- security/integrity/ima/ima_asymmetric_keys.c | 2 -- security/integrity/ima/ima_crypto.c | 2 -- security/integrity/ima/ima_fs.c | 2 -- security/integrity/ima/ima_init.c | 2 -- security/integrity/ima/ima_kexec.c | 1 - security/integrity/ima/ima_main.c | 2 -- security/integrity/ima/ima_policy.c | 2 -- security/integrity/ima/ima_queue.c | 2 -- security/integrity/ima/ima_queue_keys.c | 2 -- security/integrity/ima/ima_template.c | 2 -- security/integrity/ima/ima_template_lib.c | 2 -- security/integrity/integrity.h | 6 ++++++ 17 files changed, 6 insertions(+), 31 deletions(-)security/integrity/digsig.c +0
-2@@ -6,8 +6,6 @@* Dmitry Kasatkin <[email protected]> */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/err.h> #include <linux/sched.h> #include <linux/slab.h> security/integrity/digsig_asymmetric.c +0
-2@@ -6,8 +6,6 @@* Dmitry Kasatkin <[email protected]> */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/err.h> #include <linux/ratelimit.h> #include <linux/key-type.h> security/integrity/evm/evm_crypto.c +0
-2@@ -10,8 +10,6 @@* Using root's kernel master key (kmk), calculate the HMAC */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/export.h> #include <linux/crypto.h> #include <linux/xattr.h> security/integrity/evm/evm_main.c +0
-2@@ -11,8 +11,6 @@* evm_inode_removexattr, and evm_verifyxattr */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/init.h> #include <linux/crypto.h> #include <linux/audit.h> security/integrity/evm/evm_secfs.c +0
-2@@ -10,8 +10,6 @@* - Get the key and enable EVM */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/audit.h> #include <linux/uaccess.h> #include <linux/init.h> security/integrity/ima/ima_asymmetric_keys.c +0
-2@@ -9,8 +9,6 @@* create or update. */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <keys/asymmetric-type.h> #include "ima.h" security/integrity/ima/ima_crypto.c +0
-2@@ -10,8 +10,6 @@* Calculates md5/sha1 file hash, template hash, boot-aggreate hash */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/kernel.h> #include <linux/moduleparam.h> #include <linux/ratelimit.h> security/integrity/ima/ima_fs.c +0
-2@@ -12,8 +12,6 @@* current measurement list and IMA statistics */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/fcntl.h> #include <linux/slab.h> #include <linux/init.h> security/integrity/ima/ima_init.c +0
-2@@ -11,8 +11,6 @@* initialization and cleanup functions */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/init.h> #include <linux/scatterlist.h> #include <linux/slab.h> security/integrity/ima/ima_kexec.c +0
-1@@ -6,7 +6,6 @@* Thiago Jung Bauermann <[email protected]> * Mimi Zohar <[email protected]> */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt#include <linux/seq_file.h> #include <linux/vmalloc.h> security/integrity/ima/ima_main.c +0
-2@@ -15,8 +15,6 @@* and ima_file_check. */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/module.h> #include <linux/file.h> #include <linux/binfmts.h> security/integrity/ima/ima_policy.c +0
-2@@ -7,8 +7,6 @@* - initialize default measure policy rules */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/init.h> #include <linux/list.h> #include <linux/fs.h> security/integrity/ima/ima_queue.c +0
-2@@ -15,8 +15,6 @@* ever removed or changed during the boot-cycle. */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/rculist.h> #include <linux/slab.h> #include "ima.h" security/integrity/ima/ima_queue_keys.c +0
-2@@ -8,8 +8,6 @@* Enables deferred processing of keys */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/workqueue.h> #include <keys/asymmetric-type.h> #include "ima.h" security/integrity/ima/ima_template.c +0
-2@@ -9,8 +9,6 @@* Helpers to manage template descriptors. */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/rculist.h> #include "ima.h" #include "ima_template_lib.h" security/integrity/ima/ima_template_lib.c +0
-2@@ -9,8 +9,6 @@* Library of supported template fields. */ -#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include "ima_template_lib.h" static bool ima_template_hash_algo_allowed (u8 algo)security/integrity/integrity.h +6
-0@@ -6,6 +6,12 @@* Mimi Zohar <[email protected]> */ +#ifdef pr_fmt +#undef pr_fmt +#endif + +#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt +#include <linux/types.h> #include <linux/integrity.h> #include <crypto/sha.h>
Mimi Zohar
maintainerQuoted 2 lines · Tushar Sugandhi
The log messages from IMA subsystem should be consistent for better diagnosability and discoverability.
The change isn't limited to IMA. I would change "IMA" to "integrity" in the Subject line and in this patch description.
Quoted 3 lines
This patch set improves the logging by removing duplicate log formatting macros, adding a consistent prefix to the log messages, and adding new log messages where necessary.
Much better!
thanks,
Mimi
Quoted 7 lines · Mimi Zohar
Quoted 2 lines · Tushar Sugandhi
The log messages from IMA subsystem should be consistent for better diagnosability and discoverability.
The change isn't limited to IMA. I would change "IMA" to "integrity" in the Subject line and in this patch description.
Will do.
Quoted 10 lines
Quoted 3 lines
This patch set improves the logging by removing duplicate log formatting macros, adding a consistent prefix to the log messages, and adding new log messages where necessary.
Much better!
thanks,
Mimi