2014-10-28 11:32:16

by Dmitry Kasatkin

[permalink] [raw]
Subject: [PATCH v3 0/3] integrity: NULL pointer dereference fixes

Hi,

Recently Jan Kara reported about possible NULL pointer dereference
in IMA and EVM setxattr hooks. This patchset fixes the problem.

http://sourceforge.net/p/linux-ima/mailman/message/32957578

In this version of the patchset we agreed with Mimi to split IMA
patch into two, where the first patch fixes the oops and must go to
the stable and the second patch limits setting of file hash to fix
and log modes and may go to the next kernel version via normal process.

- Dmitry

Dmitry Kasatkin (3):
ima: check xattr value length and type in the ima_inode_setxattr()
ima: limit file hash setting by user to fix and log modes
evm: check xattr value length and type in evm_inode_setxattr()

security/integrity/evm/evm_main.c | 11 ++++++++---
security/integrity/ima/ima_appraise.c | 10 ++++++++--
security/integrity/integrity.h | 1 +
3 files changed, 17 insertions(+), 5 deletions(-)

--
1.9.1


2014-10-28 11:32:21

by Dmitry Kasatkin

[permalink] [raw]
Subject: [PATCH v3 3/3] evm: check xattr value length and type in evm_inode_setxattr()

evm_inode_setxattr() can be called with no value. The function does not
check the length so that following command can be used to produce the
kernel oops: setfattr -n security.evm FOO. This patch fixes it.

Changes in v2:
* testing for validity of xattr type

[ 1106.396921] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 1106.398192] IP: [<ffffffff812af7b8>] evm_inode_setxattr+0x2a/0x48
[ 1106.399244] PGD 29048067 PUD 290d7067 PMD 0
[ 1106.399953] Oops: 0000 [#1] SMP
[ 1106.400020] Modules linked in: bridge stp llc evdev serio_raw i2c_piix4 button fuse
[ 1106.400020] CPU: 0 PID: 3635 Comm: setxattr Not tainted 3.16.0-kds+ #2936
[ 1106.400020] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 1106.400020] task: ffff8800291a0000 ti: ffff88002917c000 task.ti: ffff88002917c000
[ 1106.400020] RIP: 0010:[<ffffffff812af7b8>] [<ffffffff812af7b8>] evm_inode_setxattr+0x2a/0x48
[ 1106.400020] RSP: 0018:ffff88002917fd50 EFLAGS: 00010246
[ 1106.400020] RAX: 0000000000000000 RBX: ffff88002917fdf8 RCX: 0000000000000000
[ 1106.400020] RDX: 0000000000000000 RSI: ffffffff818136d3 RDI: ffff88002917fdf8
[ 1106.400020] RBP: ffff88002917fd68 R08: 0000000000000000 R09: 00000000003ec1df
[ 1106.400020] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8800438a0a00
[ 1106.400020] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 1106.400020] FS: 00007f7dfa7d7740(0000) GS:ffff88005da00000(0000) knlGS:0000000000000000
[ 1106.400020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1106.400020] CR2: 0000000000000000 CR3: 000000003763e000 CR4: 00000000000006f0
[ 1106.400020] Stack:
[ 1106.400020] ffff8800438a0a00 ffff88002917fdf8 0000000000000000 ffff88002917fd98
[ 1106.400020] ffffffff812a1030 ffff8800438a0a00 ffff88002917fdf8 0000000000000000
[ 1106.400020] 0000000000000000 ffff88002917fde0 ffffffff8116d08a ffff88002917fdc8
[ 1106.400020] Call Trace:
[ 1106.400020] [<ffffffff812a1030>] security_inode_setxattr+0x5d/0x6a
[ 1106.400020] [<ffffffff8116d08a>] vfs_setxattr+0x6b/0x9f
[ 1106.400020] [<ffffffff8116d1e0>] setxattr+0x122/0x16c
[ 1106.400020] [<ffffffff811687e8>] ? mnt_want_write+0x21/0x45
[ 1106.400020] [<ffffffff8114d011>] ? __sb_start_write+0x10f/0x143
[ 1106.400020] [<ffffffff811687e8>] ? mnt_want_write+0x21/0x45
[ 1106.400020] [<ffffffff811687c0>] ? __mnt_want_write+0x48/0x4f
[ 1106.400020] [<ffffffff8116d3e6>] SyS_setxattr+0x6e/0xb0
[ 1106.400020] [<ffffffff81529da9>] system_call_fastpath+0x16/0x1b
[ 1106.400020] Code: c3 0f 1f 44 00 00 55 48 89 e5 41 55 49 89 d5 41 54 49 89 fc 53 48 89 f3 48 c7 c6 d3 36 81 81 48 89 df e8 18 22 04 00 85 c0 75 07 <41> 80 7d 00 02 74 0d 48 89 de 4c 89 e7 e8 5a fe ff ff eb 03 83
[ 1106.400020] RIP [<ffffffff812af7b8>] evm_inode_setxattr+0x2a/0x48
[ 1106.400020] RSP <ffff88002917fd50>
[ 1106.400020] CR2: 0000000000000000
[ 1106.428061] ---[ end trace ae08331628ba3050 ]---

Reported-by: Jan Kara <[email protected]>
Signed-off-by: Dmitry Kasatkin <[email protected]>
Cc: [email protected]
---
security/integrity/evm/evm_main.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index b392fe6..1384e4b 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -324,9 +324,14 @@ int evm_inode_setxattr(struct dentry *dentry, const char *xattr_name,
{
const struct evm_ima_xattr_data *xattr_data = xattr_value;

- if ((strcmp(xattr_name, XATTR_NAME_EVM) == 0)
- && (xattr_data->type == EVM_XATTR_HMAC))
- return -EPERM;
+ if (strcmp(xattr_name, XATTR_NAME_EVM) == 0) {
+ if (!xattr_value_len)
+ return -EINVAL;
+ if (xattr_data->type == EVM_XATTR_HMAC)
+ return -EPERM;
+ if (xattr_data->type != EVM_IMA_XATTR_DIGSIG)
+ return -EINVAL;
+ }
return evm_protect_xattr(dentry, xattr_name, xattr_value,
xattr_value_len);
}
--
1.9.1

2014-10-28 11:32:40

by Dmitry Kasatkin

[permalink] [raw]
Subject: [PATCH v3 2/3] ima: limit file hash setting by user to fix and log modes

File hashes are automatically set and updated and should not be
manually set. This patch limits file hash setting to fix and log
modes.

Signed-off-by: Dmitry Kasatkin <[email protected]>
---
security/integrity/ima/ima_appraise.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 7c8f41e..8549787 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -378,10 +378,14 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
result = ima_protect_xattr(dentry, xattr_name, xattr_value,
xattr_value_len);
if (result == 1) {
+ bool digsig;
+
if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))
return -EINVAL;
- ima_reset_appraise_flags(dentry->d_inode,
- (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0);
+ digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG);
+ if (!digsig && (ima_appraise & IMA_APPRAISE_ENFORCE))
+ return -EPERM;
+ ima_reset_appraise_flags(dentry->d_inode, digsig);
result = 0;
}
return result;
--
1.9.1

2014-10-28 11:32:57

by Dmitry Kasatkin

[permalink] [raw]
Subject: [PATCH v3 1/3] ima: check xattr value length and type in the ima_inode_setxattr()

ima_inode_setxattr() can be called with no value. Function does not
check the length so that following command can be used to produce
kernel oops: setfattr -n security.ima FOO. This patch fixes it.

Changes in v3:
* for stable reverted "allow setting hash only in fix or log mode"
It will be a separate patch.

Changes in v2:
* testing validity of xattr type
* allow setting hash only in fix or log mode (Mimi)

[ 261.562522] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 261.564109] IP: [<ffffffff812af272>] ima_inode_setxattr+0x3e/0x5a
[ 261.564109] PGD 3112f067 PUD 42965067 PMD 0
[ 261.564109] Oops: 0000 [#1] SMP
[ 261.564109] Modules linked in: bridge stp llc evdev serio_raw i2c_piix4 button fuse
[ 261.564109] CPU: 0 PID: 3299 Comm: setxattr Not tainted 3.16.0-kds+ #2924
[ 261.564109] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 261.564109] task: ffff8800428c2430 ti: ffff880042be0000 task.ti: ffff880042be0000
[ 261.564109] RIP: 0010:[<ffffffff812af272>] [<ffffffff812af272>] ima_inode_setxattr+0x3e/0x5a
[ 261.564109] RSP: 0018:ffff880042be3d50 EFLAGS: 00010246
[ 261.564109] RAX: 0000000000000001 RBX: 0000000000000000 RCX: 0000000000000015
[ 261.564109] RDX: 0000001500000000 RSI: 0000000000000000 RDI: ffff8800375cc600
[ 261.564109] RBP: ffff880042be3d68 R08: 0000000000000000 R09: 00000000004d6256
[ 261.564109] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88002149ba00
[ 261.564109] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 261.564109] FS: 00007f6c1e219740(0000) GS:ffff88005da00000(0000) knlGS:0000000000000000
[ 261.564109] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 261.564109] CR2: 0000000000000000 CR3: 000000003b35a000 CR4: 00000000000006f0
[ 261.564109] Stack:
[ 261.564109] ffff88002149ba00 ffff880042be3df8 0000000000000000 ffff880042be3d98
[ 261.564109] ffffffff812a101b ffff88002149ba00 ffff880042be3df8 0000000000000000
[ 261.564109] 0000000000000000 ffff880042be3de0 ffffffff8116d08a ffff880042be3dc8
[ 261.564109] Call Trace:
[ 261.564109] [<ffffffff812a101b>] security_inode_setxattr+0x48/0x6a
[ 261.564109] [<ffffffff8116d08a>] vfs_setxattr+0x6b/0x9f
[ 261.564109] [<ffffffff8116d1e0>] setxattr+0x122/0x16c
[ 261.564109] [<ffffffff811687e8>] ? mnt_want_write+0x21/0x45
[ 261.564109] [<ffffffff8114d011>] ? __sb_start_write+0x10f/0x143
[ 261.564109] [<ffffffff811687e8>] ? mnt_want_write+0x21/0x45
[ 261.564109] [<ffffffff811687c0>] ? __mnt_want_write+0x48/0x4f
[ 261.564109] [<ffffffff8116d3e6>] SyS_setxattr+0x6e/0xb0
[ 261.564109] [<ffffffff81529da9>] system_call_fastpath+0x16/0x1b
[ 261.564109] Code: 48 89 f7 48 c7 c6 58 36 81 81 53 31 db e8 73 27 04 00 85 c0 75 28 bf 15 00 00 00 e8 8a a5 d9 ff 84 c0 75 05 83 cb ff eb 15 31 f6 <41> 80 7d 00 03 49 8b 7c 24 68 40 0f 94 c6 e8 e1 f9 ff ff 89 d8
[ 261.564109] RIP [<ffffffff812af272>] ima_inode_setxattr+0x3e/0x5a
[ 261.564109] RSP <ffff880042be3d50>
[ 261.564109] CR2: 0000000000000000
[ 261.599998] ---[ end trace 39a89a3fc267e652 ]---

Reported-by: Jan Kara <[email protected]>
Signed-off-by: Dmitry Kasatkin <[email protected]>
Cc: [email protected]
---
security/integrity/ima/ima_appraise.c | 2 ++
security/integrity/integrity.h | 1 +
2 files changed, 3 insertions(+)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 9226854..7c8f41e 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -378,6 +378,8 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
result = ima_protect_xattr(dentry, xattr_name, xattr_value,
xattr_value_len);
if (result == 1) {
+ if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))
+ return -EINVAL;
ima_reset_appraise_flags(dentry->d_inode,
(xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0);
result = 0;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index caa1f6c..0fc9519 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -61,6 +61,7 @@ enum evm_ima_xattr_type {
EVM_XATTR_HMAC,
EVM_IMA_XATTR_DIGSIG,
IMA_XATTR_DIGEST_NG,
+ IMA_XATTR_LAST
};

struct evm_ima_xattr_data {
--
1.9.1

2014-10-28 12:30:24

by Dmitry Kasatkin

[permalink] [raw]
Subject: [PATCH v3 1/1] evm: check xattr value length and type in evm_inode_setxattr()

evm_inode_setxattr() can be called with no value. The function does not
check the length so that following command can be used to produce the
kernel oops: setfattr -n security.evm FOO. This patch fixes it.

Changes in v3:
* there is no reason to return different error codes for EVM_XATTR_HMAC
and non EVM_XATTR_HMAC. Remove unnecessary test then.

Changes in v2:
* testing for validity of xattr type

[ 1106.396921] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 1106.398192] IP: [<ffffffff812af7b8>] evm_inode_setxattr+0x2a/0x48
[ 1106.399244] PGD 29048067 PUD 290d7067 PMD 0
[ 1106.399953] Oops: 0000 [#1] SMP
[ 1106.400020] Modules linked in: bridge stp llc evdev serio_raw i2c_piix4 button fuse
[ 1106.400020] CPU: 0 PID: 3635 Comm: setxattr Not tainted 3.16.0-kds+ #2936
[ 1106.400020] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 1106.400020] task: ffff8800291a0000 ti: ffff88002917c000 task.ti: ffff88002917c000
[ 1106.400020] RIP: 0010:[<ffffffff812af7b8>] [<ffffffff812af7b8>] evm_inode_setxattr+0x2a/0x48
[ 1106.400020] RSP: 0018:ffff88002917fd50 EFLAGS: 00010246
[ 1106.400020] RAX: 0000000000000000 RBX: ffff88002917fdf8 RCX: 0000000000000000
[ 1106.400020] RDX: 0000000000000000 RSI: ffffffff818136d3 RDI: ffff88002917fdf8
[ 1106.400020] RBP: ffff88002917fd68 R08: 0000000000000000 R09: 00000000003ec1df
[ 1106.400020] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8800438a0a00
[ 1106.400020] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 1106.400020] FS: 00007f7dfa7d7740(0000) GS:ffff88005da00000(0000) knlGS:0000000000000000
[ 1106.400020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1106.400020] CR2: 0000000000000000 CR3: 000000003763e000 CR4: 00000000000006f0
[ 1106.400020] Stack:
[ 1106.400020] ffff8800438a0a00 ffff88002917fdf8 0000000000000000 ffff88002917fd98
[ 1106.400020] ffffffff812a1030 ffff8800438a0a00 ffff88002917fdf8 0000000000000000
[ 1106.400020] 0000000000000000 ffff88002917fde0 ffffffff8116d08a ffff88002917fdc8
[ 1106.400020] Call Trace:
[ 1106.400020] [<ffffffff812a1030>] security_inode_setxattr+0x5d/0x6a
[ 1106.400020] [<ffffffff8116d08a>] vfs_setxattr+0x6b/0x9f
[ 1106.400020] [<ffffffff8116d1e0>] setxattr+0x122/0x16c
[ 1106.400020] [<ffffffff811687e8>] ? mnt_want_write+0x21/0x45
[ 1106.400020] [<ffffffff8114d011>] ? __sb_start_write+0x10f/0x143
[ 1106.400020] [<ffffffff811687e8>] ? mnt_want_write+0x21/0x45
[ 1106.400020] [<ffffffff811687c0>] ? __mnt_want_write+0x48/0x4f
[ 1106.400020] [<ffffffff8116d3e6>] SyS_setxattr+0x6e/0xb0
[ 1106.400020] [<ffffffff81529da9>] system_call_fastpath+0x16/0x1b
[ 1106.400020] Code: c3 0f 1f 44 00 00 55 48 89 e5 41 55 49 89 d5 41 54 49 89 fc 53 48 89 f3 48 c7 c6 d3 36 81 81 48 89 df e8 18 22 04 00 85 c0 75 07 <41> 80 7d 00 02 74 0d 48 89 de 4c 89 e7 e8 5a fe ff ff eb 03 83
[ 1106.400020] RIP [<ffffffff812af7b8>] evm_inode_setxattr+0x2a/0x48
[ 1106.400020] RSP <ffff88002917fd50>
[ 1106.400020] CR2: 0000000000000000
[ 1106.428061] ---[ end trace ae08331628ba3050 ]---

Reported-by: Jan Kara <[email protected]>
Signed-off-by: Dmitry Kasatkin <[email protected]>
Cc: [email protected]
---
security/integrity/evm/evm_main.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index b392fe6..f589c9a0 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -324,9 +324,12 @@ int evm_inode_setxattr(struct dentry *dentry, const char *xattr_name,
{
const struct evm_ima_xattr_data *xattr_data = xattr_value;

- if ((strcmp(xattr_name, XATTR_NAME_EVM) == 0)
- && (xattr_data->type == EVM_XATTR_HMAC))
- return -EPERM;
+ if (strcmp(xattr_name, XATTR_NAME_EVM) == 0) {
+ if (!xattr_value_len)
+ return -EINVAL;
+ if (xattr_data->type != EVM_IMA_XATTR_DIGSIG)
+ return -EPERM;
+ }
return evm_protect_xattr(dentry, xattr_name, xattr_value,
xattr_value_len);
}
--
1.9.1

2014-10-28 12:34:08

by Dmitry Kasatkin

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] evm: check xattr value length and type in evm_inode_setxattr()

Sorry, this was the wrong version of the patch.
Please ignore this patch and use what is in the reply to this patch:
[PATCH v3 1/1] evm: check xattr value length and type in
evm_inode_setxattr()

- Dmitry

On 28/10/14 13:31, Dmitry Kasatkin wrote:
> evm_inode_setxattr() can be called with no value. The function does not
> check the length so that following command can be used to produce the
> kernel oops: setfattr -n security.evm FOO. This patch fixes it.
>
> Changes in v2:
> * testing for validity of xattr type
>
> [ 1106.396921] BUG: unable to handle kernel NULL pointer dereference at (null)
> [ 1106.398192] IP: [<ffffffff812af7b8>] evm_inode_setxattr+0x2a/0x48
> [ 1106.399244] PGD 29048067 PUD 290d7067 PMD 0
> [ 1106.399953] Oops: 0000 [#1] SMP
> [ 1106.400020] Modules linked in: bridge stp llc evdev serio_raw i2c_piix4 button fuse
> [ 1106.400020] CPU: 0 PID: 3635 Comm: setxattr Not tainted 3.16.0-kds+ #2936
> [ 1106.400020] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [ 1106.400020] task: ffff8800291a0000 ti: ffff88002917c000 task.ti: ffff88002917c000
> [ 1106.400020] RIP: 0010:[<ffffffff812af7b8>] [<ffffffff812af7b8>] evm_inode_setxattr+0x2a/0x48
> [ 1106.400020] RSP: 0018:ffff88002917fd50 EFLAGS: 00010246
> [ 1106.400020] RAX: 0000000000000000 RBX: ffff88002917fdf8 RCX: 0000000000000000
> [ 1106.400020] RDX: 0000000000000000 RSI: ffffffff818136d3 RDI: ffff88002917fdf8
> [ 1106.400020] RBP: ffff88002917fd68 R08: 0000000000000000 R09: 00000000003ec1df
> [ 1106.400020] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8800438a0a00
> [ 1106.400020] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [ 1106.400020] FS: 00007f7dfa7d7740(0000) GS:ffff88005da00000(0000) knlGS:0000000000000000
> [ 1106.400020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1106.400020] CR2: 0000000000000000 CR3: 000000003763e000 CR4: 00000000000006f0
> [ 1106.400020] Stack:
> [ 1106.400020] ffff8800438a0a00 ffff88002917fdf8 0000000000000000 ffff88002917fd98
> [ 1106.400020] ffffffff812a1030 ffff8800438a0a00 ffff88002917fdf8 0000000000000000
> [ 1106.400020] 0000000000000000 ffff88002917fde0 ffffffff8116d08a ffff88002917fdc8
> [ 1106.400020] Call Trace:
> [ 1106.400020] [<ffffffff812a1030>] security_inode_setxattr+0x5d/0x6a
> [ 1106.400020] [<ffffffff8116d08a>] vfs_setxattr+0x6b/0x9f
> [ 1106.400020] [<ffffffff8116d1e0>] setxattr+0x122/0x16c
> [ 1106.400020] [<ffffffff811687e8>] ? mnt_want_write+0x21/0x45
> [ 1106.400020] [<ffffffff8114d011>] ? __sb_start_write+0x10f/0x143
> [ 1106.400020] [<ffffffff811687e8>] ? mnt_want_write+0x21/0x45
> [ 1106.400020] [<ffffffff811687c0>] ? __mnt_want_write+0x48/0x4f
> [ 1106.400020] [<ffffffff8116d3e6>] SyS_setxattr+0x6e/0xb0
> [ 1106.400020] [<ffffffff81529da9>] system_call_fastpath+0x16/0x1b
> [ 1106.400020] Code: c3 0f 1f 44 00 00 55 48 89 e5 41 55 49 89 d5 41 54 49 89 fc 53 48 89 f3 48 c7 c6 d3 36 81 81 48 89 df e8 18 22 04 00 85 c0 75 07 <41> 80 7d 00 02 74 0d 48 89 de 4c 89 e7 e8 5a fe ff ff eb 03 83
> [ 1106.400020] RIP [<ffffffff812af7b8>] evm_inode_setxattr+0x2a/0x48
> [ 1106.400020] RSP <ffff88002917fd50>
> [ 1106.400020] CR2: 0000000000000000
> [ 1106.428061] ---[ end trace ae08331628ba3050 ]---
>
> Reported-by: Jan Kara <[email protected]>
> Signed-off-by: Dmitry Kasatkin <[email protected]>
> Cc: [email protected]
> ---
> security/integrity/evm/evm_main.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index b392fe6..1384e4b 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -324,9 +324,14 @@ int evm_inode_setxattr(struct dentry *dentry, const char *xattr_name,
> {
> const struct evm_ima_xattr_data *xattr_data = xattr_value;
>
> - if ((strcmp(xattr_name, XATTR_NAME_EVM) == 0)
> - && (xattr_data->type == EVM_XATTR_HMAC))
> - return -EPERM;
> + if (strcmp(xattr_name, XATTR_NAME_EVM) == 0) {
> + if (!xattr_value_len)
> + return -EINVAL;
> + if (xattr_data->type == EVM_XATTR_HMAC)
> + return -EPERM;
> + if (xattr_data->type != EVM_IMA_XATTR_DIGSIG)
> + return -EINVAL;
> + }
> return evm_protect_xattr(dentry, xattr_name, xattr_value,
> xattr_value_len);
> }

2014-10-28 14:27:01

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] integrity: NULL pointer dereference fixes

On Tue, 2014-10-28 at 13:31 +0200, Dmitry Kasatkin wrote:
> Hi,
>
> Recently Jan Kara reported about possible NULL pointer dereference
> in IMA and EVM setxattr hooks. This patchset fixes the problem.
>
> http://sourceforge.net/p/linux-ima/mailman/message/32957578
>
> In this version of the patchset we agreed with Mimi to split IMA
> patch into two, where the first patch fixes the oops and must go to
> the stable and the second patch limits setting of file hash to fix
> and log modes and may go to the next kernel version via normal process.

Thanks, Dmitry. James, I'll send you a pull request shortly for Linus,
deferring one patch until the 3.19 open window.

For-Linus:
[PATCH v3 1/3] ima: check xattr value length and type in the ima_inode_setxattr()
[P
ATCH v3 1/1] evm: check xattr value
length and type in
evm_inode_setxattr()

3.19: [v3 2/3] ima: limit file hash setting by user to fix and log modes

Mimi