linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Shuah Khan <skhan@linuxfoundation.org>
To: James Bottomley <James.Bottomley@HansenPartnership.com>,
	Mimi Zohar <zohar@linux.ibm.com>,
	Tushar Sugandhi <tusharsu@linux.microsoft.com>,
	joe@perches.com, linux-integrity@vger.kernel.org
Cc: sashal@kernel.org, nramas@linux.microsoft.com,
	linux-kernel@vger.kernel.org,
	Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH v3 3/3] IMA: Add module name and base name prefix to log.
Date: Wed, 12 Feb 2020 15:52:40 -0700	[thread overview]
Message-ID: <d428f807-7e67-a173-183d-f2ab15bdef9e@linuxfoundation.org> (raw)
In-Reply-To: <1581521161.3494.7.camel@HansenPartnership.com>

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@linux.microsoft.com>
>>> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>>> Suggested-by: Joe Perches <joe@perches.com>
>>> Suggested-by: Shuah Khan <skhan@linuxfoundation.org>
>>
>> <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@us.ibm.com>
>>>    */
>>>   
>>> +#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


  parent reply	other threads:[~2020-02-12 22:52 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-11 23:14 [PATCH v3 0/3] IMA: improve log messages in IMA Tushar Sugandhi
2020-02-11 23:14 ` [PATCH v3 1/3] IMA: Update KBUILD_MODNAME for IMA files to ima Tushar Sugandhi
2020-02-12 14:49   ` Mimi Zohar
2020-02-12 22:25     ` Tushar Sugandhi
2020-02-11 23:14 ` [PATCH v3 2/3] IMA: Add log statements for failure conditions Tushar Sugandhi
2020-02-12 14:47   ` Mimi Zohar
2020-02-12 22:30     ` Tushar Sugandhi
2020-02-13  0:21       ` Mimi Zohar
2020-02-13 21:01         ` Tushar Sugandhi
2020-02-11 23:14 ` [PATCH v3 3/3] IMA: Add module name and base name prefix to log Tushar Sugandhi
2020-02-12 14:29   ` Mimi Zohar
2020-02-12 15:26     ` James Bottomley
2020-02-12 15:47       ` Joe Perches
2020-02-12 22:52       ` Shuah Khan [this message]
2020-02-13  0:38         ` Mimi Zohar
2020-02-13  0:56           ` Tushar Sugandhi
2020-02-12 15:23 ` [PATCH v3 0/3] IMA: improve log messages in IMA Mimi Zohar
2020-02-12 22:22   ` Tushar Sugandhi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d428f807-7e67-a173-183d-f2ab15bdef9e@linuxfoundation.org \
    --to=skhan@linuxfoundation.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=joe@perches.com \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nramas@linux.microsoft.com \
    --cc=sashal@kernel.org \
    --cc=tusharsu@linux.microsoft.com \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).