diff mbox series

[v10,1/6] IMA: Check IMA policy flag

Message ID 20191204224131.3384-2-nramas@linux.microsoft.com (mailing list archive)
State New, archived
Headers show
Series KEYS: Measure keys when they are created or updated | expand

Commit Message

Lakshmi Ramasubramanian Dec. 4, 2019, 10:41 p.m. UTC
Return immediately from process_buffer_measurement()
if the IMA policy flag is set to zero. Not doing this
can result in kernel panic when process_buffer_measurement()
is called before IMA is initialized (for instance, when
the IMA hook is called when a key is added to
the .builtin_trusted_keys keyring).

This change adds the check in process_buffer_measurement()
to return immediately if ima_policy_flag is set to zero.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 security/integrity/ima/ima_main.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Mimi Zohar Dec. 10, 2019, 10:42 p.m. UTC | #1
On Wed, 2019-12-04 at 14:41 -0800, Lakshmi Ramasubramanian wrote:
> Return immediately from process_buffer_measurement()
> if the IMA policy flag is set to zero. Not doing this
> can result in kernel panic when process_buffer_measurement()
> is called before IMA is initialized (for instance, when
> the IMA hook is called when a key is added to
> the .builtin_trusted_keys keyring).
> 
> This change adds the check in process_buffer_measurement()
> to return immediately if ima_policy_flag is set to zero.

Patch descriptions aren't suppose to be written as pseudo code.  Start
with the current status and problem description.

For example, "process_buffer_measurement() may be called prior to IMA being initialized, which would result in a kernel panic.  This patch ..."

Mimi

> 
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
>  security/integrity/ima/ima_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index d7e987baf127..9b35db2fc777 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -655,6 +655,9 @@ void process_buffer_measurement(const void *buf, int size,
>  	int action = 0;
>  	u32 secid;
>  
> +	if (!ima_policy_flag)
> +		return;
> +
>  	/*
>  	 * Both LSM hooks and auxilary based buffer measurements are
>  	 * based on policy.  To avoid code duplication, differentiate
Lakshmi Ramasubramanian Dec. 10, 2019, 11:29 p.m. UTC | #2
On 12/10/19 2:42 PM, Mimi Zohar wrote:

> Patch descriptions aren't suppose to be written as pseudo code.  Start
> with the current status and problem description.
> 
> For example, "process_buffer_measurement() may be called prior to IMA being initialized, which would result in a kernel panic.  This patch ..."
> 
> Mimi

I'll update the patch description in this one and in the other patches 
per your comments.

Are you done reviewing all the patches in this set?

Other than the one code change per your comment on "[PATCH v10 5/6]" 
there are no other code changes I need to make?
Just wanted to confirm.

	[PATCH v10 5/6] IMA: Add support to limit measuring keys
=> With the additional "uid" support this isn't necessarily true any
more.

thanks,
  -lakshmi
Mimi Zohar Dec. 11, 2019, 12:03 a.m. UTC | #3
On Tue, 2019-12-10 at 15:29 -0800, Lakshmi Ramasubramanian wrote:
> On 12/10/19 2:42 PM, Mimi Zohar wrote:
> 
> > Patch descriptions aren't suppose to be written as pseudo code.  Start
> > with the current status and problem description.
> > 
> > For example, "process_buffer_measurement() may be called prior to IMA being initialized, which would result in a kernel panic.  This patch ..."
> > 
> > Mimi
> 
> I'll update the patch description in this one and in the other patches 
> per your comments.
> 
> Are you done reviewing all the patches in this set?
> 
> Other than the one code change per your comment on "[PATCH v10 5/6]" 
> there are no other code changes I need to make?
> Just wanted to confirm.
> 
> 	[PATCH v10 5/6] IMA: Add support to limit measuring keys
> => With the additional "uid" support this isn't necessarily true any
> more.

Yes, other than the code change needed for this and the patch
descriptions, it looks good.  Am continuing with reviewing the other
patch set - queueing "key" measurements.

Mimi
diff mbox series

Patch

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index d7e987baf127..9b35db2fc777 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -655,6 +655,9 @@  void process_buffer_measurement(const void *buf, int size,
 	int action = 0;
 	u32 secid;
 
+	if (!ima_policy_flag)
+		return;
+
 	/*
 	 * Both LSM hooks and auxilary based buffer measurements are
 	 * based on policy.  To avoid code duplication, differentiate