public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] ima: bug fixes for Linus
@ 2013-11-22 18:40 Mimi Zohar
  2013-11-24 15:21 ` Mimi Zohar
  0 siblings, 1 reply; 19+ messages in thread
From: Mimi Zohar @ 2013-11-22 18:40 UTC (permalink / raw)
  To: James Morris; +Cc: linux-security-module, linux-kernel

Hi James,

The following changes since commit 217091dd7a7a1bdac027ddb7c5a25f6ac0b8e241:

  ima: define '_ima' as a builtin 'trusted' keyring (2013-10-31 20:20:48 -0400)

are available in the git repository at:

  ssh://gitolite@ra.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git ima-fixes-new

for you to fetch changes up to 0fdb4f5d9a07a832b38ebbc616a19411700d1fac:

  ima: make a copy of template_fmt in template_desc_init_fields() (2013-11-22 13:08:31 -0500)

thanks,

Mimi

----------------------------------------------------------------
Mimi Zohar (4):
      ima: fix random config build error
      ima: limit trusted keyring name change
      KEYS: special dot prefixed keyring name bug fix
      ima: update IMA-templates.txt documentation

Roberto Sassu (7):
      ima: change the default hash algorithm to SHA1 in ima_eventdigest_ng_init()
      ima: pass HASH_ALGO__LAST as hash algo in ima_eventdigest_init()
      ima: remove unneeded size_limit argument from ima_eventdigest_init_common()
      ima: check result of crypto_shash_update() in ima_calc_field_array_hash_tfm
      ima: do not include field length in template digest calc for ima template
      ima: do not send field length to userspace for digest of ima template
      ima: make a copy of template_fmt in template_desc_init_fields()

 Documentation/security/IMA-templates.txt  |  6 ++++--
 security/integrity/digsig.c               | 12 ++++--------
 security/integrity/ima/ima.h              |  6 ++++--
 security/integrity/ima/ima_api.c          |  1 +
 security/integrity/ima/ima_crypto.c       | 17 ++++++++++++-----
 security/integrity/ima/ima_fs.c           | 14 +++++++++++---
 security/integrity/ima/ima_template.c     | 10 ++++++++--
 security/integrity/ima/ima_template_lib.c | 24 +++++++++++++-----------
 security/integrity/integrity.h            | 11 +++++------
 security/keys/keyctl.c                    | 11 ++++++++++-
 10 files changed, 72 insertions(+), 40 deletions(-)



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] ima: bug fixes for Linus
  2013-11-22 18:40 [GIT PULL] ima: bug fixes for Linus Mimi Zohar
@ 2013-11-24 15:21 ` Mimi Zohar
  2013-11-24 22:44   ` James Morris
  0 siblings, 1 reply; 19+ messages in thread
From: Mimi Zohar @ 2013-11-24 15:21 UTC (permalink / raw)
  To: James Morris; +Cc: linux-security-module, linux-kernel

Hi James,

Linus has already reverted the trusted keyring support for IMA patches.
These patches are re-based on -rc1.

The following changes since commit 4c1cc40a2d49500d84038ff751bc6cd183e729b5:

  Revert "KEYS: verify a certificate is signed by a 'trusted' key" (2013-11-23 16:38:17 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity for-linus

for you to fetch changes up to 3eeb2d63ab623be55bb2ff584e123c0df45691e3:

  ima: make a copy of template_fmt in template_desc_init_fields() (2013-11-24 00:29:23 -0500)

thanks,

Mimi

----------------------------------------------------------------
Mimi Zohar (1):
      ima: update IMA-templates.txt documentation

Roberto Sassu (7):
      ima: change the default hash algorithm to SHA1 in ima_eventdigest_ng_init()
      ima: pass HASH_ALGO__LAST as hash algo in ima_eventdigest_init()
      ima: remove unneeded size_limit argument from ima_eventdigest_init_common()
      ima: check result of crypto_shash_update() in ima_calc_field_array_hash_tfm
      ima: do not include field length in template digest calc for ima template
      ima: do not send field length to userspace for digest of ima template
      ima: make a copy of template_fmt in template_desc_init_fields()

 Documentation/security/IMA-templates.txt  |  6 ++++--
 security/integrity/ima/ima.h              |  6 ++++--
 security/integrity/ima/ima_api.c          |  1 +
 security/integrity/ima/ima_crypto.c       | 17 ++++++++++++-----
 security/integrity/ima/ima_fs.c           | 14 +++++++++++---
 security/integrity/ima/ima_template.c     | 10 ++++++++--
 security/integrity/ima/ima_template_lib.c | 24 +++++++++++++-----------
 7 files changed, 53 insertions(+), 25 deletions(-)



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] ima: bug fixes for Linus
  2013-11-24 15:21 ` Mimi Zohar
@ 2013-11-24 22:44   ` James Morris
  2013-11-25  0:14     ` Mimi Zohar
  0 siblings, 1 reply; 19+ messages in thread
From: James Morris @ 2013-11-24 22:44 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: linux-security-module, linux-kernel

On Sun, 24 Nov 2013, Mimi Zohar wrote:

> Hi James,
> 
> Linus has already reverted the trusted keyring support for IMA patches.
> These patches are re-based on -rc1.
> 
> The following changes since commit 4c1cc40a2d49500d84038ff751bc6cd183e729b5:
> 
>   Revert "KEYS: verify a certificate is signed by a 'trusted' key" (2013-11-23 16:38:17 -0800)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity for-linus
> 
> for you to fetch changes up to 3eeb2d63ab623be55bb2ff584e123c0df45691e3:
> 
>   ima: make a copy of template_fmt in template_desc_init_fields() (2013-11-24 00:29:23 -0500)
> 

I don't understand -- are these all fixes for regressions in the new 
kernel?


-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] ima: bug fixes for Linus
  2013-11-24 22:44   ` James Morris
@ 2013-11-25  0:14     ` Mimi Zohar
  2013-11-25  2:14       ` James Morris
  0 siblings, 1 reply; 19+ messages in thread
From: Mimi Zohar @ 2013-11-25  0:14 UTC (permalink / raw)
  To: James Morris; +Cc: linux-security-module, linux-kernel

On Mon, 2013-11-25 at 09:44 +1100, James Morris wrote:
> On Sun, 24 Nov 2013, Mimi Zohar wrote:
> 
> > Hi James,
> > 
> > Linus has already reverted the trusted keyring support for IMA patches.
> > These patches are re-based on -rc1.
> > 
> > The following changes since commit 4c1cc40a2d49500d84038ff751bc6cd183e729b5:
> > 
> >   Revert "KEYS: verify a certificate is signed by a 'trusted' key" (2013-11-23 16:38:17 -0800)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity for-linus
> > 
> > for you to fetch changes up to 3eeb2d63ab623be55bb2ff584e123c0df45691e3:
> > 
> >   ima: make a copy of template_fmt in template_desc_init_fields() (2013-11-24 00:29:23 -0500)
> > 
> 
> I don't understand -- are these all fixes for regressions in the new 
> kernel?

Yes, mostly.  There's one code cleanup, that could be deferred and a
documentation update.

Mimi




^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] ima: bug fixes for Linus
  2013-11-25  0:14     ` Mimi Zohar
@ 2013-11-25  2:14       ` James Morris
  2013-11-25 12:03         ` Mimi Zohar
  0 siblings, 1 reply; 19+ messages in thread
From: James Morris @ 2013-11-25  2:14 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: linux-security-module, linux-kernel

On Sun, 24 Nov 2013, Mimi Zohar wrote:

> On Mon, 2013-11-25 at 09:44 +1100, James Morris wrote:
> > On Sun, 24 Nov 2013, Mimi Zohar wrote:
> > 
> > > Hi James,
> > > 
> > > Linus has already reverted the trusted keyring support for IMA patches.
> > > These patches are re-based on -rc1.
> > > 
> > > The following changes since commit 4c1cc40a2d49500d84038ff751bc6cd183e729b5:
> > > 
> > >   Revert "KEYS: verify a certificate is signed by a 'trusted' key" (2013-11-23 16:38:17 -0800)
> > > 
> > > are available in the git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity for-linus
> > > 
> > > for you to fetch changes up to 3eeb2d63ab623be55bb2ff584e123c0df45691e3:
> > > 
> > >   ima: make a copy of template_fmt in template_desc_init_fields() (2013-11-24 00:29:23 -0500)
> > > 
> > 
> > I don't understand -- are these all fixes for regressions in the new 
> > kernel?
> 
> Yes, mostly.  There's one code cleanup, that could be deferred and a
> documentation update.

Can we leave documentation and code cleanups to the next cycle and only 
include essential fixes for regressions at this stage?

Also, please identify which upstream commits specifically are fixed by 
each patch.


- James

-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] ima: bug fixes for Linus
  2013-11-25  2:14       ` James Morris
@ 2013-11-25 12:03         ` Mimi Zohar
  2013-11-25 13:51           ` Mimi Zohar
  0 siblings, 1 reply; 19+ messages in thread
From: Mimi Zohar @ 2013-11-25 12:03 UTC (permalink / raw)
  To: James Morris; +Cc: linux-security-module, linux-kernel

On Mon, 2013-11-25 at 13:14 +1100, James Morris wrote:
> On Sun, 24 Nov 2013, Mimi Zohar wrote:
> 
> > On Mon, 2013-11-25 at 09:44 +1100, James Morris wrote:
> > > On Sun, 24 Nov 2013, Mimi Zohar wrote:
> > > 
> > > > Hi James,
> > > > 
> > > > Linus has already reverted the trusted keyring support for IMA patches.
> > > > These patches are re-based on -rc1.
> > > > 
> > > > The following changes since commit 4c1cc40a2d49500d84038ff751bc6cd183e729b5:
> > > > 
> > > >   Revert "KEYS: verify a certificate is signed by a 'trusted' key" (2013-11-23 16:38:17 -0800)
> > > > 
> > > > are available in the git repository at:
> > > > 
> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity for-linus
> > > > 
> > > > for you to fetch changes up to 3eeb2d63ab623be55bb2ff584e123c0df45691e3:
> > > > 
> > > >   ima: make a copy of template_fmt in template_desc_init_fields() (2013-11-24 00:29:23 -0500)
> > > > 
> > > 
> > > I don't understand -- are these all fixes for regressions in the new 
> > > kernel?
> > 
> > Yes, mostly.  There's one code cleanup, that could be deferred and a
> > documentation update.
> 
> Can we leave documentation and code cleanups to the next cycle and only 
> include essential fixes for regressions at this stage?

Ok, all of the patches are needed and need to be upstreamed.  I assume
all of the ones that fix backwards compatibility issues would be termed
"essential fixes for regressions".

47a20c2 ima: do not include field length in template digest calc for ima templat
4c8f4bb ima: do not send field length to userspace for digest of ima template
3eeb2d6 ima: make a copy of template_fmt in template_desc_init_fields()

Could the remaining patches be marked for -stable?

> Also, please identify which upstream commits specifically are fixed by 
> each patch.

Ok

thanks,

Mimi


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] ima: bug fixes for Linus
  2013-11-25 12:03         ` Mimi Zohar
@ 2013-11-25 13:51           ` Mimi Zohar
  2013-11-25 15:40             ` James Morris
  0 siblings, 1 reply; 19+ messages in thread
From: Mimi Zohar @ 2013-11-25 13:51 UTC (permalink / raw)
  To: James Morris; +Cc: linux-security-module, linux-kernel

Hi James,

These are the "essential fixes for regressions".

The following changes since commit 4c1cc40a2d49500d84038ff751bc6cd183e729b5:

  Revert "KEYS: verify a certificate is signed by a 'trusted' key" (2013-11-23 16:38:17 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity for-linus

for you to fetch changes up to 72ca1bd303a5126d0ce377cff699282b6b38bd86:

  ima: make a copy of template_fmt in template_desc_init_fields() (2013-11-25 07:32:46 -0500)

thanks,

Mimi

----------------------------------------------------------------
Roberto Sassu (3):
      ima: do not include field length in template digest calc for ima template
      ima: do not send field length to userspace for digest of ima template
      ima: make a copy of template_fmt in template_desc_init_fields()

 security/integrity/ima/ima.h              |  6 ++++--
 security/integrity/ima/ima_api.c          |  1 +
 security/integrity/ima/ima_crypto.c       | 17 ++++++++++++-----
 security/integrity/ima/ima_fs.c           | 14 +++++++++++---
 security/integrity/ima/ima_template.c     | 10 ++++++++--
 security/integrity/ima/ima_template_lib.c |  6 +++++-
 6 files changed, 41 insertions(+), 13 deletions(-)



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] ima: bug fixes for Linus
  2013-11-25 13:51           ` Mimi Zohar
@ 2013-11-25 15:40             ` James Morris
  2013-11-25 18:46               ` Roberto Sassu
  2013-11-25 19:18               ` [PATCH] ima: make a copy of template_fmt in template_desc_init_fields() Roberto Sassu
  0 siblings, 2 replies; 19+ messages in thread
From: James Morris @ 2013-11-25 15:40 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: linux-security-module, linux-kernel

On Mon, 25 Nov 2013, Mimi Zohar wrote:

> Hi James,
> 
> These are the "essential fixes for regressions".
> 
> The following changes since commit 4c1cc40a2d49500d84038ff751bc6cd183e729b5:
> 
>   Revert "KEYS: verify a certificate is signed by a 'trusted' key" (2013-11-23 16:38:17 -0800)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity for-linus
> 
> for you to fetch changes up to 72ca1bd303a5126d0ce377cff699282b6b38bd86:
> 
>   ima: make a copy of template_fmt in template_desc_init_fields() (2013-11-25 07:32:46 -0500)
> 
> thanks,
> 
> Mimi
> 
> ----------------------------------------------------------------
> Roberto Sassu (3):

>       ima: make a copy of template_fmt in template_desc_init_fields()


> template_desc_init_fields(char *template_fmt,

That should probably be const char.

Also, the call to kstrdup() results in a memory leak.





-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] ima: bug fixes for Linus
  2013-11-25 15:40             ` James Morris
@ 2013-11-25 18:46               ` Roberto Sassu
  2013-11-25 18:55                 ` Roberto Sassu
  2013-11-25 20:33                 ` [GIT PULL v3] ima: bug fixes for Linus Mimi Zohar
  2013-11-25 19:18               ` [PATCH] ima: make a copy of template_fmt in template_desc_init_fields() Roberto Sassu
  1 sibling, 2 replies; 19+ messages in thread
From: Roberto Sassu @ 2013-11-25 18:46 UTC (permalink / raw)
  To: James Morris, Mimi Zohar; +Cc: linux-security-module, linux-kernel

On 11/25/2013 04:40 PM, James Morris wrote:
> On Mon, 25 Nov 2013, Mimi Zohar wrote:
>
>> Hi James,
>>
>> These are the "essential fixes for regressions".
>>
>> The following changes since commit 4c1cc40a2d49500d84038ff751bc6cd183e729b5:
>>
>>    Revert "KEYS: verify a certificate is signed by a 'trusted' key" (2013-11-23 16:38:17 -0800)
>>
>> are available in the git repository at:
>>
>>    git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity for-linus
>>
>> for you to fetch changes up to 72ca1bd303a5126d0ce377cff699282b6b38bd86:
>>
>>    ima: make a copy of template_fmt in template_desc_init_fields() (2013-11-25 07:32:46 -0500)
>>
>> thanks,
>>
>> Mimi
>>
>> ----------------------------------------------------------------
>> Roberto Sassu (3):
>
>>        ima: make a copy of template_fmt in template_desc_init_fields()
>
>
>> template_desc_init_fields(char *template_fmt,
>
> That should probably be const char.
>
> Also, the call to kstrdup() results in a memory leak.
>

Hi James

thanks for the comments. I'm implementing them and I will post
a new version of the patch 'ima: make a copy of template_fmt in
template_desc_init_fields()' shortly.

Roberto Sassu


>
>
>
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] ima: bug fixes for Linus
  2013-11-25 18:46               ` Roberto Sassu
@ 2013-11-25 18:55                 ` Roberto Sassu
  2013-11-27 12:11                   ` Sebastian Ott
  2013-11-25 20:33                 ` [GIT PULL v3] ima: bug fixes for Linus Mimi Zohar
  1 sibling, 1 reply; 19+ messages in thread
From: Roberto Sassu @ 2013-11-25 18:55 UTC (permalink / raw)
  To: James Morris, Mimi Zohar; +Cc: linux-security-module, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1497 bytes --]

On 11/25/2013 07:46 PM, Roberto Sassu wrote:
> On 11/25/2013 04:40 PM, James Morris wrote:
>> On Mon, 25 Nov 2013, Mimi Zohar wrote:
>>
>>> Hi James,
>>>
>>> These are the "essential fixes for regressions".
>>>
>>> The following changes since commit
>>> 4c1cc40a2d49500d84038ff751bc6cd183e729b5:
>>>
>>>    Revert "KEYS: verify a certificate is signed by a 'trusted' key"
>>> (2013-11-23 16:38:17 -0800)
>>>
>>> are available in the git repository at:
>>>
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity
>>> for-linus
>>>
>>> for you to fetch changes up to 72ca1bd303a5126d0ce377cff699282b6b38bd86:
>>>
>>>    ima: make a copy of template_fmt in template_desc_init_fields()
>>> (2013-11-25 07:32:46 -0500)
>>>
>>> thanks,
>>>
>>> Mimi
>>>
>>> ----------------------------------------------------------------
>>> Roberto Sassu (3):
>>
>>>        ima: make a copy of template_fmt in template_desc_init_fields()
>>
>>
>>> template_desc_init_fields(char *template_fmt,
>>
>> That should probably be const char.
>>
>> Also, the call to kstrdup() results in a memory leak.
>>
>
> Hi James
>
> thanks for the comments. I'm implementing them and I will post
> a new version of the patch 'ima: make a copy of template_fmt in
> template_desc_init_fields()' shortly.
>

Hi everyone

attached to this email, there is the new version of the above patch.

Regards

Roberto Sassu


> Roberto Sassu
>
>
>>
>>
>>
>>
>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-ima-make-a-copy-of-template_fmt-in-template_desc_ini.patch --]
[-- Type: text/x-diff; name="0001-ima-make-a-copy-of-template_fmt-in-template_desc_ini.patch", Size: 4827 bytes --]

From fde6cb013e4613e87bfb2d8436782cc9a98ef906 Mon Sep 17 00:00:00 2001
From: Roberto Sassu <roberto.sassu@polito.it>
Date: Thu, 7 Nov 2013 15:00:43 +0100
Subject: [PATCH] ima: make a copy of template_fmt in
 template_desc_init_fields()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch makes a copy of the 'template_fmt' function argument so that
the latter will not be modified by strsep(), which does the splitting by
replacing the given separator with '\0'.

 IMA: No TPM chip found, activating TPM-bypass!
 Unable to handle kernel pointer dereference at virtual kernel address 0000000000842000
 Oops: 0004 [#1] SMP
 Modules linked in:
 CPU: 3 PID: 1 Comm: swapper/0 Not tainted 3.12.0-rc2-00098-g3ce1217d6cd5 #17
 task: 000000003ffa0000 ti: 000000003ff84000 task.ti: 000000003ff84000
 Krnl PSW : 0704e00180000000 000000000044bf88 (strsep+0x7c/0xa0)
            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 EA:3
 Krnl GPRS: 000000000000007c 000000000000007c 000000003ff87d90 0000000000821fd8
            0000000000000000 000000000000007c 0000000000aa37e0 0000000000aa9008
            0000000000000051 0000000000a114d8 0000000100000002 0000000000842bde
            0000000000842bdf 00000000006f97f0 000000000040062c 000000003ff87cf0
 Krnl Code: 000000000044bf7c: a7f4000a           brc     15,44bf90
            000000000044bf80: b90200cc           ltgr    %r12,%r12
           #000000000044bf84: a7840006           brc     8,44bf90
           >000000000044bf88: 9200c000           mvi     0(%r12),0
            000000000044bf8c: 41c0c001           la      %r12,1(%r12)
            000000000044bf90: e3c020000024       stg     %r12,0(%r2)
            000000000044bf96: b904002b           lgr     %r2,%r11
            000000000044bf9a: ebbcf0700004       lmg     %r11,%r12,112(%r15)
 Call Trace:
 ([<00000000004005fe>] ima_init_template+0xa2/0x1bc)
  [<0000000000a7c896>] ima_init+0x7a/0xa8
  [<0000000000a7c938>] init_ima+0x24/0x40
  [<00000000001000e8>] do_one_initcall+0x68/0x128
  [<0000000000a4eb56>] kernel_init_freeable+0x20a/0x2b4
  [<00000000006a1ff4>] kernel_init+0x30/0x178
  [<00000000006b69fe>] kernel_thread_starter+0x6/0xc
  [<00000000006b69f8>] kernel_thread_starter+0x0/0xc
 Last Breaking-Event-Address:
  [<000000000044bf42>] strsep+0x36/0xa0

Fixes commit: adf53a7 ima: new templates management mechanism

Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Tested-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 security/integrity/ima/ima_template.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 4e5da99..913e192 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -90,7 +90,7 @@ static struct ima_template_field *lookup_template_field(const char *field_id)
 	return NULL;
 }
 
-static int template_fmt_size(char *template_fmt)
+static int template_fmt_size(const char *template_fmt)
 {
 	char c;
 	int template_fmt_len = strlen(template_fmt);
@@ -106,23 +106,28 @@ static int template_fmt_size(char *template_fmt)
 	return j + 1;
 }
 
-static int template_desc_init_fields(char *template_fmt,
+static int template_desc_init_fields(const char *template_fmt,
 				     struct ima_template_field ***fields,
 				     int *num_fields)
 {
-	char *c, *template_fmt_ptr = template_fmt;
+	char *c, *template_fmt_copy;
 	int template_num_fields = template_fmt_size(template_fmt);
 	int i, result = 0;
 
 	if (template_num_fields > IMA_TEMPLATE_NUM_FIELDS_MAX)
 		return -EINVAL;
 
+	/* copying is needed as strsep() modifies the original buffer */
+	template_fmt_copy = kstrdup(template_fmt, GFP_KERNEL);
+	if (template_fmt_copy == NULL)
+		return -ENOMEM;
+
 	*fields = kzalloc(template_num_fields * sizeof(*fields), GFP_KERNEL);
 	if (*fields == NULL) {
 		result = -ENOMEM;
 		goto out;
 	}
-	for (i = 0; (c = strsep(&template_fmt_ptr, "|")) != NULL &&
+	for (i = 0; (c = strsep(&template_fmt_copy, "|")) != NULL &&
 	     i < template_num_fields; i++) {
 		struct ima_template_field *f = lookup_template_field(c);
 
@@ -133,10 +138,12 @@ static int template_desc_init_fields(char *template_fmt,
 		(*fields)[i] = f;
 	}
 	*num_fields = i;
-	return 0;
 out:
-	kfree(*fields);
-	*fields = NULL;
+	if (result < 0) {
+		kfree(*fields);
+		*fields = NULL;
+	}
+	kfree(template_fmt_copy);
 	return result;
 }
 
-- 
1.8.1.4


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4720 bytes --]

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH] ima: make a copy of template_fmt in template_desc_init_fields()
  2013-11-25 15:40             ` James Morris
  2013-11-25 18:46               ` Roberto Sassu
@ 2013-11-25 19:18               ` Roberto Sassu
  1 sibling, 0 replies; 19+ messages in thread
From: Roberto Sassu @ 2013-11-25 19:18 UTC (permalink / raw)
  To: jmorris, zohar; +Cc: linux-security-module, linux-kernel, Roberto Sassu

[-- Attachment #1: Type: text/plain, Size: 4500 bytes --]

This patch makes a copy of the 'template_fmt' function argument so that
the latter will not be modified by strsep(), which does the splitting by
replacing the given separator with '\0'.

 IMA: No TPM chip found, activating TPM-bypass!
 Unable to handle kernel pointer dereference at virtual kernel address 0000000000842000
 Oops: 0004 [#1] SMP
 Modules linked in:
 CPU: 3 PID: 1 Comm: swapper/0 Not tainted 3.12.0-rc2-00098-g3ce1217d6cd5 #17
 task: 000000003ffa0000 ti: 000000003ff84000 task.ti: 000000003ff84000
 Krnl PSW : 0704e00180000000 000000000044bf88 (strsep+0x7c/0xa0)
            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 EA:3
 Krnl GPRS: 000000000000007c 000000000000007c 000000003ff87d90 0000000000821fd8
            0000000000000000 000000000000007c 0000000000aa37e0 0000000000aa9008
            0000000000000051 0000000000a114d8 0000000100000002 0000000000842bde
            0000000000842bdf 00000000006f97f0 000000000040062c 000000003ff87cf0
 Krnl Code: 000000000044bf7c: a7f4000a           brc     15,44bf90
            000000000044bf80: b90200cc           ltgr    %r12,%r12
           #000000000044bf84: a7840006           brc     8,44bf90
           >000000000044bf88: 9200c000           mvi     0(%r12),0
            000000000044bf8c: 41c0c001           la      %r12,1(%r12)
            000000000044bf90: e3c020000024       stg     %r12,0(%r2)
            000000000044bf96: b904002b           lgr     %r2,%r11
            000000000044bf9a: ebbcf0700004       lmg     %r11,%r12,112(%r15)
 Call Trace:
 ([<00000000004005fe>] ima_init_template+0xa2/0x1bc)
  [<0000000000a7c896>] ima_init+0x7a/0xa8
  [<0000000000a7c938>] init_ima+0x24/0x40
  [<00000000001000e8>] do_one_initcall+0x68/0x128
  [<0000000000a4eb56>] kernel_init_freeable+0x20a/0x2b4
  [<00000000006a1ff4>] kernel_init+0x30/0x178
  [<00000000006b69fe>] kernel_thread_starter+0x6/0xc
  [<00000000006b69f8>] kernel_thread_starter+0x0/0xc
 Last Breaking-Event-Address:
  [<000000000044bf42>] strsep+0x36/0xa0

Fixes commit: adf53a7 ima: new templates management mechanism

Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Tested-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 security/integrity/ima/ima_template.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 4e5da99..913e192 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -90,7 +90,7 @@ static struct ima_template_field *lookup_template_field(const char *field_id)
 	return NULL;
 }
 
-static int template_fmt_size(char *template_fmt)
+static int template_fmt_size(const char *template_fmt)
 {
 	char c;
 	int template_fmt_len = strlen(template_fmt);
@@ -106,23 +106,28 @@ static int template_fmt_size(char *template_fmt)
 	return j + 1;
 }
 
-static int template_desc_init_fields(char *template_fmt,
+static int template_desc_init_fields(const char *template_fmt,
 				     struct ima_template_field ***fields,
 				     int *num_fields)
 {
-	char *c, *template_fmt_ptr = template_fmt;
+	char *c, *template_fmt_copy;
 	int template_num_fields = template_fmt_size(template_fmt);
 	int i, result = 0;
 
 	if (template_num_fields > IMA_TEMPLATE_NUM_FIELDS_MAX)
 		return -EINVAL;
 
+	/* copying is needed as strsep() modifies the original buffer */
+	template_fmt_copy = kstrdup(template_fmt, GFP_KERNEL);
+	if (template_fmt_copy == NULL)
+		return -ENOMEM;
+
 	*fields = kzalloc(template_num_fields * sizeof(*fields), GFP_KERNEL);
 	if (*fields == NULL) {
 		result = -ENOMEM;
 		goto out;
 	}
-	for (i = 0; (c = strsep(&template_fmt_ptr, "|")) != NULL &&
+	for (i = 0; (c = strsep(&template_fmt_copy, "|")) != NULL &&
 	     i < template_num_fields; i++) {
 		struct ima_template_field *f = lookup_template_field(c);
 
@@ -133,10 +138,12 @@ static int template_desc_init_fields(char *template_fmt,
 		(*fields)[i] = f;
 	}
 	*num_fields = i;
-	return 0;
 out:
-	kfree(*fields);
-	*fields = NULL;
+	if (result < 0) {
+		kfree(*fields);
+		*fields = NULL;
+	}
+	kfree(template_fmt_copy);
 	return result;
 }
 
-- 
1.8.1.4


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2061 bytes --]

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [GIT PULL v3] ima: bug fixes for Linus
  2013-11-25 18:46               ` Roberto Sassu
  2013-11-25 18:55                 ` Roberto Sassu
@ 2013-11-25 20:33                 ` Mimi Zohar
  2013-11-25 20:54                   ` Shuah Khan
  1 sibling, 1 reply; 19+ messages in thread
From: Mimi Zohar @ 2013-11-25 20:33 UTC (permalink / raw)
  To: Roberto Sassu; +Cc: James Morris, linux-security-module, linux-kernel

Hi James,

Included in this pull request, is Roberto's updated patch.  I've
included a Changelog.

The following changes since commit 4c1cc40a2d49500d84038ff751bc6cd183e729b5:

  Revert "KEYS: verify a certificate is signed by a 'trusted' key" (2013-11-23 16:38:17 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity for-linus

for you to fetch changes up to dbc335d2dc3c437649eb6b39f4e9aee2a13eb0af:

  ima: make a copy of template_fmt in template_desc_init_fields() (2013-11-25 15:05:33 -0500)

thanks,

Mimi

----------------------------------------------------------------
Roberto Sassu (3):
      ima: do not include field length in template digest calc for ima template
      ima: do not send field length to userspace for digest of ima template
      ima: make a copy of template_fmt in template_desc_init_fields()

 security/integrity/ima/ima.h              |  6 ++++--
 security/integrity/ima/ima_api.c          |  1 +
 security/integrity/ima/ima_crypto.c       | 17 ++++++++++++-----
 security/integrity/ima/ima_fs.c           | 14 +++++++++++---
 security/integrity/ima/ima_template.c     | 21 ++++++++++++++-------
 security/integrity/ima/ima_template_lib.c |  6 +++++-
 6 files changed, 47 insertions(+), 18 deletions(-)

----------------------------------------------------------------
Roberto Sassu (3):
      ima: do not include field length in template digest calc for ima
template
      ima: do not send field length to userspace for digest of ima
template
      ima: make a copy of template_fmt in template_desc_init_fields()

 security/integrity/ima/ima.h              |  6 ++++--
 security/integrity/ima/ima_api.c          |  1 +
 security/integrity/ima/ima_crypto.c       | 17 ++++++++++++-----
 security/integrity/ima/ima_fs.c           | 14 +++++++++++---
 security/integrity/ima/ima_template.c     | 21 ++++++++++++++-------
 security/integrity/ima/ima_template_lib.c |  6 +++++-
 6 files changed, 47 insertions(+), 18 deletions(-)



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL v3] ima: bug fixes for Linus
  2013-11-25 20:33                 ` [GIT PULL v3] ima: bug fixes for Linus Mimi Zohar
@ 2013-11-25 20:54                   ` Shuah Khan
  2013-11-25 21:32                     ` Mimi Zohar
  0 siblings, 1 reply; 19+ messages in thread
From: Shuah Khan @ 2013-11-25 20:54 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Roberto Sassu, James Morris, linux-security-module, linux-kernel

Mimi,

Do you have fix for the following build error as well.

  CC      security/integrity/digsig.o
security/integrity/digsig.c:70:5: error: redefinition of
‘integrity_init_keyring’
 int integrity_init_keyring(const unsigned int id)
     ^
In file included from security/integrity/digsig.c:22:0:
security/integrity/integrity.h:149:12: note: previous definition of
‘integrity_init_keyring’ was here
 static int integrity_init_keyring(const unsigned int id)
            ^
security/integrity/integrity.h:149:12: warning:
‘integrity_init_keyring’ defined but not used [-Wunused-function]
make[2]: *** [security/integrity/digsig.o] Error 1
make[1]: *** [security/integrity] Error 2
make: *** [security] Error 2

I noticed the problem in linux-next and sent patch to fix it:

https://lkml.org/lkml/2013/11/8/441

I think It has been noticed in 3.13-rc1 as well.

-- Shuah

On Mon, Nov 25, 2013 at 1:33 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Hi James,
>
> Included in this pull request, is Roberto's updated patch.  I've
> included a Changelog.
>
> The following changes since commit 4c1cc40a2d49500d84038ff751bc6cd183e729b5:
>
>   Revert "KEYS: verify a certificate is signed by a 'trusted' key" (2013-11-23 16:38:17 -0800)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity for-linus
>
> for you to fetch changes up to dbc335d2dc3c437649eb6b39f4e9aee2a13eb0af:
>
>   ima: make a copy of template_fmt in template_desc_init_fields() (2013-11-25 15:05:33 -0500)
>
> thanks,
>
> Mimi
>
> ----------------------------------------------------------------
> Roberto Sassu (3):
>       ima: do not include field length in template digest calc for ima template
>       ima: do not send field length to userspace for digest of ima template
>       ima: make a copy of template_fmt in template_desc_init_fields()
>
>  security/integrity/ima/ima.h              |  6 ++++--
>  security/integrity/ima/ima_api.c          |  1 +
>  security/integrity/ima/ima_crypto.c       | 17 ++++++++++++-----
>  security/integrity/ima/ima_fs.c           | 14 +++++++++++---
>  security/integrity/ima/ima_template.c     | 21 ++++++++++++++-------
>  security/integrity/ima/ima_template_lib.c |  6 +++++-
>  6 files changed, 47 insertions(+), 18 deletions(-)
>
> ----------------------------------------------------------------
> Roberto Sassu (3):
>       ima: do not include field length in template digest calc for ima
> template
>       ima: do not send field length to userspace for digest of ima
> template
>       ima: make a copy of template_fmt in template_desc_init_fields()
>
>  security/integrity/ima/ima.h              |  6 ++++--
>  security/integrity/ima/ima_api.c          |  1 +
>  security/integrity/ima/ima_crypto.c       | 17 ++++++++++++-----
>  security/integrity/ima/ima_fs.c           | 14 +++++++++++---
>  security/integrity/ima/ima_template.c     | 21 ++++++++++++++-------
>  security/integrity/ima/ima_template_lib.c |  6 +++++-
>  6 files changed, 47 insertions(+), 18 deletions(-)
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL v3] ima: bug fixes for Linus
  2013-11-25 20:54                   ` Shuah Khan
@ 2013-11-25 21:32                     ` Mimi Zohar
  0 siblings, 0 replies; 19+ messages in thread
From: Mimi Zohar @ 2013-11-25 21:32 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Roberto Sassu, James Morris, linux-security-module, linux-kernel

On Mon, 2013-11-25 at 13:54 -0700, Shuah Khan wrote:
> Mimi,
> 
> Do you have fix for the following build error as well.
> 
>   CC      security/integrity/digsig.o
> security/integrity/digsig.c:70:5: error: redefinition of
> ‘integrity_init_keyring’
>  int integrity_init_keyring(const unsigned int id)
>      ^
> In file included from security/integrity/digsig.c:22:0:
> security/integrity/integrity.h:149:12: note: previous definition of
> ‘integrity_init_keyring’ was here
>  static int integrity_init_keyring(const unsigned int id)
>             ^
> security/integrity/integrity.h:149:12: warning:
> ‘integrity_init_keyring’ defined but not used [-Wunused-function]
> make[2]: *** [security/integrity/digsig.o] Error 1
> make[1]: *** [security/integrity] Error 2
> make: *** [security] Error 2
> 
> I noticed the problem in linux-next and sent patch to fix it:
> 
> https://lkml.org/lkml/2013/11/8/441
> 
> I think It has been noticed in 3.13-rc1 as well.

Thanks, Shuah.  As much as I wanted to extend the UEFI secure boot
signature chain of trust to IMA-appraisal, there are still a couple of
trusted keyring issues that need to be addressed.  For now, Linus has
reverted this patch.

thanks,

Mimi


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] ima: bug fixes for Linus
  2013-11-25 18:55                 ` Roberto Sassu
@ 2013-11-27 12:11                   ` Sebastian Ott
  2013-11-27 12:46                     ` Roberto Sassu
  0 siblings, 1 reply; 19+ messages in thread
From: Sebastian Ott @ 2013-11-27 12:11 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: James Morris, Mimi Zohar, linux-security-module, linux-kernel

Hello,

On Mon, 25 Nov 2013, Roberto Sassu wrote:

> On 11/25/2013 07:46 PM, Roberto Sassu wrote:
> > On 11/25/2013 04:40 PM, James Morris wrote:
> > > On Mon, 25 Nov 2013, Mimi Zohar wrote:
> > > 
> > > > Hi James,
> > > > 
> > > > These are the "essential fixes for regressions".
> > > > 
> > > > The following changes since commit
> > > > 4c1cc40a2d49500d84038ff751bc6cd183e729b5:
> > > > 
> > > >    Revert "KEYS: verify a certificate is signed by a 'trusted' key"
> > > > (2013-11-23 16:38:17 -0800)
> > > > 
> > > > are available in the git repository at:
> > > > 
> > > > 
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity
> > > > for-linus
> > > > 
> > > > for you to fetch changes up to 72ca1bd303a5126d0ce377cff699282b6b38bd86:
> > > > 
> > > >    ima: make a copy of template_fmt in template_desc_init_fields()
> > > > (2013-11-25 07:32:46 -0500)
> > > > 
> > > > thanks,
> > > > 
> > > > Mimi
> > > > 
> > > > ----------------------------------------------------------------
> > > > Roberto Sassu (3):
> > > 
> > > >        ima: make a copy of template_fmt in template_desc_init_fields()

commit dbc335d2dc3c437649eb6b39f4e9aee2a13eb0af
"ima: make a copy of template_fmt in template_desc_init_fields()"

claimed to fix a kstrdup memleak..looks like it didn't:

unreferenced object 0x56c82370 (size 8):
  comm "swapper/0", pid 1, jiffies 4294937476 (age 916.520s)
  hex dump (first 8 bytes):
    64 00 6e 00 6b 6b 6b a5                          d.n.kkk.
  backtrace:
    [<000000000027c7ac>] __kmalloc_track_caller+0x2e0/0x450
    [<0000000000240738>] kstrdup+0x4c/0xd0
    [<00000000003c93c4>] ima_init_template+0x9c/0x1f4
    [<0000000000a11a54>] ima_init+0x74/0x98
    [<0000000000a11ba8>] init_ima+0x30/0x4c
    [<00000000001001e2>] do_one_initcall+0xce/0x160
    [<00000000009ebb70>] kernel_init_freeable+0x22c/0x2dc
    [<000000000061b704>] kernel_init+0x24/0x134
    [<0000000000633322>] kernel_thread_starter+0x6/0xc
    [<000000000063331c>] kernel_thread_starter+0x0/0xc
unreferenced object 0x56ccc158 (size 16):
  comm "swapper/0", pid 1, jiffies 4294937476 (age 916.520s)
  hex dump (first 16 bytes):
    64 2d 6e 67 00 6e 2d 6e 67 00 73 69 67 00 6b a5  d-ng.n-ng.sig.k.
  backtrace:
    [<000000000027c7ac>] __kmalloc_track_caller+0x2e0/0x450
    [<0000000000240738>] kstrdup+0x4c/0xd0
    [<00000000003c93c4>] ima_init_template+0x9c/0x1f4
    [<0000000000a11a54>] ima_init+0x74/0x98
    [<0000000000a11ba8>] init_ima+0x30/0x4c
    [<00000000001001e2>] do_one_initcall+0xce/0x160
    [<00000000009ebb70>] kernel_init_freeable+0x22c/0x2dc
    [<000000000061b704>] kernel_init+0x24/0x134
    [<0000000000633322>] kernel_thread_starter+0x6/0xc
    [<000000000063331c>] kernel_thread_starter+0x0/0xc


strsep will modify your template_fmt_copy pointer.

Regards,
Sebastian

> > > 
> > > 
> > > > template_desc_init_fields(char *template_fmt,
> > > 
> > > That should probably be const char.
> > > 
> > > Also, the call to kstrdup() results in a memory leak.
> > > 
> > 
> > Hi James
> > 
> > thanks for the comments. I'm implementing them and I will post
> > a new version of the patch 'ima: make a copy of template_fmt in
> > template_desc_init_fields()' shortly.
> > 
> 
> Hi everyone
> 
> attached to this email, there is the new version of the above patch.
> 
> Regards
> 
> Roberto Sassu
> 
> 
> > Roberto Sassu
> > 
> > 
> > > 
> > > 
> > > 
> > > 
> > 
> 
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] ima: bug fixes for Linus
  2013-11-27 12:11                   ` Sebastian Ott
@ 2013-11-27 12:46                     ` Roberto Sassu
  2013-11-27 13:40                       ` [PATCH] ima: store address of template_fmt_copy in a pointer before calling strsep Roberto Sassu
  0 siblings, 1 reply; 19+ messages in thread
From: Roberto Sassu @ 2013-11-27 12:46 UTC (permalink / raw)
  To: Sebastian Ott
  Cc: James Morris, Mimi Zohar, linux-security-module, linux-kernel

On 11/27/2013 01:11 PM, Sebastian Ott wrote:
> Hello,
>
> On Mon, 25 Nov 2013, Roberto Sassu wrote:
>
>> On 11/25/2013 07:46 PM, Roberto Sassu wrote:
>>> On 11/25/2013 04:40 PM, James Morris wrote:
>>>> On Mon, 25 Nov 2013, Mimi Zohar wrote:
>>>>
>>>>> Hi James,
>>>>>
>>>>> These are the "essential fixes for regressions".
>>>>>
>>>>> The following changes since commit
>>>>> 4c1cc40a2d49500d84038ff751bc6cd183e729b5:
>>>>>
>>>>>     Revert "KEYS: verify a certificate is signed by a 'trusted' key"
>>>>> (2013-11-23 16:38:17 -0800)
>>>>>
>>>>> are available in the git repository at:
>>>>>
>>>>>
>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity
>>>>> for-linus
>>>>>
>>>>> for you to fetch changes up to 72ca1bd303a5126d0ce377cff699282b6b38bd86:
>>>>>
>>>>>     ima: make a copy of template_fmt in template_desc_init_fields()
>>>>> (2013-11-25 07:32:46 -0500)
>>>>>
>>>>> thanks,
>>>>>
>>>>> Mimi
>>>>>
>>>>> ----------------------------------------------------------------
>>>>> Roberto Sassu (3):
>>>>
>>>>>         ima: make a copy of template_fmt in template_desc_init_fields()
>
> commit dbc335d2dc3c437649eb6b39f4e9aee2a13eb0af
> "ima: make a copy of template_fmt in template_desc_init_fields()"
>
> claimed to fix a kstrdup memleak..looks like it didn't:
>
> unreferenced object 0x56c82370 (size 8):
>    comm "swapper/0", pid 1, jiffies 4294937476 (age 916.520s)
>    hex dump (first 8 bytes):
>      64 00 6e 00 6b 6b 6b a5                          d.n.kkk.
>    backtrace:
>      [<000000000027c7ac>] __kmalloc_track_caller+0x2e0/0x450
>      [<0000000000240738>] kstrdup+0x4c/0xd0
>      [<00000000003c93c4>] ima_init_template+0x9c/0x1f4
>      [<0000000000a11a54>] ima_init+0x74/0x98
>      [<0000000000a11ba8>] init_ima+0x30/0x4c
>      [<00000000001001e2>] do_one_initcall+0xce/0x160
>      [<00000000009ebb70>] kernel_init_freeable+0x22c/0x2dc
>      [<000000000061b704>] kernel_init+0x24/0x134
>      [<0000000000633322>] kernel_thread_starter+0x6/0xc
>      [<000000000063331c>] kernel_thread_starter+0x0/0xc
> unreferenced object 0x56ccc158 (size 16):
>    comm "swapper/0", pid 1, jiffies 4294937476 (age 916.520s)
>    hex dump (first 16 bytes):
>      64 2d 6e 67 00 6e 2d 6e 67 00 73 69 67 00 6b a5  d-ng.n-ng.sig.k.
>    backtrace:
>      [<000000000027c7ac>] __kmalloc_track_caller+0x2e0/0x450
>      [<0000000000240738>] kstrdup+0x4c/0xd0
>      [<00000000003c93c4>] ima_init_template+0x9c/0x1f4
>      [<0000000000a11a54>] ima_init+0x74/0x98
>      [<0000000000a11ba8>] init_ima+0x30/0x4c
>      [<00000000001001e2>] do_one_initcall+0xce/0x160
>      [<00000000009ebb70>] kernel_init_freeable+0x22c/0x2dc
>      [<000000000061b704>] kernel_init+0x24/0x134
>      [<0000000000633322>] kernel_thread_starter+0x6/0xc
>      [<000000000063331c>] kernel_thread_starter+0x0/0xc
>
>
> strsep will modify your template_fmt_copy pointer.
>

Hi Sebastian

thanks for the report. I'm very sorry.
I should have prepared the patch more carefully.
I'll provide a fix shortly.

Thanks

Roberto Sassu


> Regards,
> Sebastian
>
>>>>
>>>>
>>>>> template_desc_init_fields(char *template_fmt,
>>>>
>>>> That should probably be const char.
>>>>
>>>> Also, the call to kstrdup() results in a memory leak.
>>>>
>>>
>>> Hi James
>>>
>>> thanks for the comments. I'm implementing them and I will post
>>> a new version of the patch 'ima: make a copy of template_fmt in
>>> template_desc_init_fields()' shortly.
>>>
>>
>> Hi everyone
>>
>> attached to this email, there is the new version of the above patch.
>>
>> Regards
>>
>> Roberto Sassu
>>
>>
>>> Roberto Sassu
>>>
>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] ima: store address of template_fmt_copy in a pointer before calling strsep
  2013-11-27 12:46                     ` Roberto Sassu
@ 2013-11-27 13:40                       ` Roberto Sassu
  2013-11-27 14:55                         ` Mimi Zohar
  2013-11-27 15:01                         ` Sebastian Ott
  0 siblings, 2 replies; 19+ messages in thread
From: Roberto Sassu @ 2013-11-27 13:40 UTC (permalink / raw)
  To: jmorris, zohar; +Cc: sebott, linux-security-module, linux-kernel, Roberto Sassu

[-- Attachment #1: Type: text/plain, Size: 1446 bytes --]

This patch stores the address of the 'template_fmt_copy' variable in a new
variable, called 'template_fmt_ptr', so that the latter is passed as an
argument of strsep() instead of the former. This modification is needed
in order to correctly free the memory area referenced by
'template_fmt_copy' (strsep() modifies the pointer of the passed string).

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
---
 security/integrity/ima/ima_template.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 913e192..635695f 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -110,7 +110,7 @@ static int template_desc_init_fields(const char *template_fmt,
 				     struct ima_template_field ***fields,
 				     int *num_fields)
 {
-	char *c, *template_fmt_copy;
+	char *c, *template_fmt_copy, *template_fmt_ptr;
 	int template_num_fields = template_fmt_size(template_fmt);
 	int i, result = 0;
 
@@ -127,7 +127,9 @@ static int template_desc_init_fields(const char *template_fmt,
 		result = -ENOMEM;
 		goto out;
 	}
-	for (i = 0; (c = strsep(&template_fmt_copy, "|")) != NULL &&
+
+	template_fmt_ptr = template_fmt_copy;
+	for (i = 0; (c = strsep(&template_fmt_ptr, "|")) != NULL &&
 	     i < template_num_fields; i++) {
 		struct ima_template_field *f = lookup_template_field(c);
 
-- 
1.8.1.4


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2061 bytes --]

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] ima: store address of template_fmt_copy in a pointer before calling strsep
  2013-11-27 13:40                       ` [PATCH] ima: store address of template_fmt_copy in a pointer before calling strsep Roberto Sassu
@ 2013-11-27 14:55                         ` Mimi Zohar
  2013-11-27 15:01                         ` Sebastian Ott
  1 sibling, 0 replies; 19+ messages in thread
From: Mimi Zohar @ 2013-11-27 14:55 UTC (permalink / raw)
  To: Roberto Sassu; +Cc: jmorris, sebott, linux-security-module, linux-kernel

On Wed, 2013-11-27 at 14:40 +0100, Roberto Sassu wrote:
> This patch stores the address of the 'template_fmt_copy' variable in a new
> variable, called 'template_fmt_ptr', so that the latter is passed as an
> argument of strsep() instead of the former. This modification is needed
> in order to correctly free the memory area referenced by
> 'template_fmt_copy' (strsep() modifies the pointer of the passed string).
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>

> ---
>  security/integrity/ima/ima_template.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 913e192..635695f 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -110,7 +110,7 @@ static int template_desc_init_fields(const char *template_fmt,
>  				     struct ima_template_field ***fields,
>  				     int *num_fields)
>  {
> -	char *c, *template_fmt_copy;
> +	char *c, *template_fmt_copy, *template_fmt_ptr;
>  	int template_num_fields = template_fmt_size(template_fmt);
>  	int i, result = 0;
> 
> @@ -127,7 +127,9 @@ static int template_desc_init_fields(const char *template_fmt,
>  		result = -ENOMEM;
>  		goto out;
>  	}
> -	for (i = 0; (c = strsep(&template_fmt_copy, "|")) != NULL &&
> +
> +	template_fmt_ptr = template_fmt_copy;
> +	for (i = 0; (c = strsep(&template_fmt_ptr, "|")) != NULL &&
>  	     i < template_num_fields; i++) {
>  		struct ima_template_field *f = lookup_template_field(c);
> 



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ima: store address of template_fmt_copy in a pointer before calling strsep
  2013-11-27 13:40                       ` [PATCH] ima: store address of template_fmt_copy in a pointer before calling strsep Roberto Sassu
  2013-11-27 14:55                         ` Mimi Zohar
@ 2013-11-27 15:01                         ` Sebastian Ott
  1 sibling, 0 replies; 19+ messages in thread
From: Sebastian Ott @ 2013-11-27 15:01 UTC (permalink / raw)
  To: Roberto Sassu; +Cc: jmorris, zohar, linux-security-module, linux-kernel



On Wed, 27 Nov 2013, Roberto Sassu wrote:
> This patch stores the address of the 'template_fmt_copy' variable in a new
> variable, called 'template_fmt_ptr', so that the latter is passed as an
> argument of strsep() instead of the former. This modification is needed
> in order to correctly free the memory area referenced by
> 'template_fmt_copy' (strsep() modifies the pointer of the passed string).
> 

This one helped. Thanks!

Sebastian

> Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
> ---
>  security/integrity/ima/ima_template.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 913e192..635695f 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -110,7 +110,7 @@ static int template_desc_init_fields(const char *template_fmt,
>  				     struct ima_template_field ***fields,
>  				     int *num_fields)
>  {
> -	char *c, *template_fmt_copy;
> +	char *c, *template_fmt_copy, *template_fmt_ptr;
>  	int template_num_fields = template_fmt_size(template_fmt);
>  	int i, result = 0;
> 
> @@ -127,7 +127,9 @@ static int template_desc_init_fields(const char *template_fmt,
>  		result = -ENOMEM;
>  		goto out;
>  	}
> -	for (i = 0; (c = strsep(&template_fmt_copy, "|")) != NULL &&
> +
> +	template_fmt_ptr = template_fmt_copy;
> +	for (i = 0; (c = strsep(&template_fmt_ptr, "|")) != NULL &&
>  	     i < template_num_fields; i++) {
>  		struct ima_template_field *f = lookup_template_field(c);
> 
> -- 
> 1.8.1.4
> 
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2013-11-27 15:02 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-22 18:40 [GIT PULL] ima: bug fixes for Linus Mimi Zohar
2013-11-24 15:21 ` Mimi Zohar
2013-11-24 22:44   ` James Morris
2013-11-25  0:14     ` Mimi Zohar
2013-11-25  2:14       ` James Morris
2013-11-25 12:03         ` Mimi Zohar
2013-11-25 13:51           ` Mimi Zohar
2013-11-25 15:40             ` James Morris
2013-11-25 18:46               ` Roberto Sassu
2013-11-25 18:55                 ` Roberto Sassu
2013-11-27 12:11                   ` Sebastian Ott
2013-11-27 12:46                     ` Roberto Sassu
2013-11-27 13:40                       ` [PATCH] ima: store address of template_fmt_copy in a pointer before calling strsep Roberto Sassu
2013-11-27 14:55                         ` Mimi Zohar
2013-11-27 15:01                         ` Sebastian Ott
2013-11-25 20:33                 ` [GIT PULL v3] ima: bug fixes for Linus Mimi Zohar
2013-11-25 20:54                   ` Shuah Khan
2013-11-25 21:32                     ` Mimi Zohar
2013-11-25 19:18               ` [PATCH] ima: make a copy of template_fmt in template_desc_init_fields() Roberto Sassu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox