Re: [PATCH v3 3/3] IMA: Add module name and base name prefix to log.
From: Tushar Sugandhi
Date: Wed Feb 12 2020 - 19:56:30 EST
On 2020-02-12 4:38 p.m., Mimi Zohar wrote:
On Wed, 2020-02-12 at 15:52 -0700, Shuah Khan wrote:
On 2/12/20 8:26 AM, James Bottomley wrote:
On Wed, 2020-02-12 at 09:29 -0500, Mimi Zohar wrote:
On Tue, 2020-02-11 at 15:14 -0800, Tushar Sugandhi wrote:
The #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. Also, it adds KBUILD_MODNAME and
KBUILD_BASENAME prefix
to the log messages.
Signed-off-by: Tushar Sugandhi <tusharsu@xxxxxxxxxxxxxxxxxxx>
Reviewed-by: Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx>
Suggested-by: Joe Perches <joe@xxxxxxxxxxx>
Suggested-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>
<snip>
diff --git a/security/integrity/integrity.h
b/security/integrity/integrity.h
index 73fc286834d7..b1bb4d2263be 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -6,6 +6,12 @@
* Mimi Zohar <zohar@xxxxxxxxxx>
*/
+#ifdef pr_fmt
+#undef pr_fmt
+#endif
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " KBUILD_BASENAME ": " fmt
+
#include <linux/types.h>
#include <linux/integrity.h>
#include <crypto/sha.h>
Joe, Shuah, including the pr_fmt() in integrity/integrity.h not only
affects the integrity directory but everything below it. Adding
KBUILD_BASENAME to pr_fmt() modifies all of the existing IMA and EVM
kernel messages. Is that ok or should there be a separate pr_fmt()
for the subdirectories?
Log messages are often consumed by log monitors, which mostly use
pattern matching to find messages they're interested in, so you have to
take some care when changing the messages the kernel spits out and you
have to make sure any change gets well notified so the distributions
can warn about it.
For this one, can we see a "before" and "after" message so we know
what's happening?
Mimi and James,
My suggestion was based on thinking that simplifying this by removing
duplicate defines. Some messages are missing modules names, adding
module name to them does change the messages.
If using one pr_fmt for all modules changes the world and makes it
difficult for log monitors, I would say it isn't a good change.
I will leave this totally up to Mimi to decide. Feel free to throw
out my suggestion if it leads more trouble than help. :)
Thanks, Shuah. ÂTushar, I don't see any need for changing the existing
IMA/EVM messages. ÂEither remove the KBUILD_BASENAME from the format
or limit the new format to the integrity directory.
Ok. I will remove the KBUILD_BASENAME from the format.
And I will keep the definition in security/integrity/integrity.h, and
will keep the duplicates removed - as originally proposed in this patch
v3 3/3.
thanks,
Mimi