general protection fault in strcmp

[syzbot]

Hello,

syzkaller hit the following crash on
ef0010a30935de4e0211cbc7bdffc30446cdee9b
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
compiler: gcc (GCC) 7.1.1 20170620
.config is attached
Raw console output is attached.

Unfortunately, I don't have any reproducer for this bug yet.


kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 0 PID: 9245 Comm: syz-executor4 Not tainted 4.15.0-rc1+ #200
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
task: 00000000b39f3d07 task.stack: 000000009c61786b
RIP: 0010:strcmp+0x5a/0xb0 lib/string.c:328
RSP: 0018:ffff8801913e7d18 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000062 RCX: ffffffff823b0528
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff853eb000
RBP: ffff8801913e7d40 R08: 0000000000000000 R09: 0000000000000005
R10: 0000000000000000 R11: ffffffff87489d60 R12: ffffffff853eb001
R13: 0000000000000001 R14: dffffc0000000000 R15: ffffffff823b04b0
FS: 00007f38862b5700(0000) GS:ffff8801db400000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020006000 CR3: 00000001d274e000 CR4: 00000000001426f0
DR0: 0000000000102000 DR1: 0000000000000004 DR2: 0000000000000000
DR3: 0000000000101000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
asymmetric_lookup_restriction+0x87/0x660
crypto/asymmetric_keys/asymmetric_type.c:485
keyring_restrict+0xc8/0x4e0 security/keys/keyring.c:1018
keyctl_restrict_keyring+0xea/0x170 security/keys/keyctl.c:1629
SYSC_keyctl security/keys/keyctl.c:1750 [inline]
SyS_keyctl+0x46/0x2c0 security/keys/keyctl.c:1641
entry_SYSCALL_64_fastpath+0x1f/0x96
RIP: 0033:0x4529d9
RSP: 002b:00007f38862b4c58 EFLAGS: 00000212 ORIG_RAX: 00000000000000fa
RAX: ffffffffffffffda RBX: 0000000000758020 RCX: 00000000004529d9
RDX: 0000000020011000 RSI: ffffffffffffffff RDI: 000000000000001d
RBP: 000000000000039b R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000212 R12: 00000000006f2728
R13: 00000000ffffffff R14: 00007f38862b56d4 R15: 0000000000000000
Code: 48 c1 e8 03 83 e2 07 42 0f b6 04 30 38 d0 7f 04 84 c0 75 55 48 89 f0
48 89 f2 41 0f b6 5c 24 ff 48 c1 e8 03 83 e2 07 4c 8d 6e 01 <42> 0f b6 04
30 38 d0 7f 04 84 c0 75 27 41 3a 5d ff 74 ae 19 c0
RIP: strcmp+0x5a/0xb0 lib/string.c:328 RSP: ffff8801913e7d18
---[ end trace f71019dcf623ceee ]---
Kernel panic - not syncing: Fatal exception
Dumping ftrace buffer:
(ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzk...@googlegroups.com.
Please credit me with: Reported-by: syzbot <syzk...@googlegroups.com>

syzbot will keep track of this bug report.
Once a fix for this bug is committed, please reply to this email with:
#syz fix: exact-commit-title
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug
report.
Note: all commands must start from beginning of the line in the email body.

[Eric Biggers]

From: Eric Biggers <ebig...@google.com>

keyctl_restrict_keyring() allows through a NULL restriction when the
"type" is non-NULL, which causes a NULL pointer dereference in
asymmetric_lookup_restriction() when it calls strcmp() on the
restriction string.

But no key types actually use a "NULL restriction" to mean anything, so
update keyctl_restrict_keyring() to reject it with EINVAL.

Reported-by: syzbot <syzk...@googlegroups.com>
Fixes: 97d3aa0f3134 ("KEYS: Add a lookup_restriction function for the asymmetric key type")
Cc: <sta...@vger.kernel.org> # v4.12+
Signed-off-by: Eric Biggers <ebig...@google.com>
---
security/keys/keyctl.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 76d22f726ae4..1ffe60bb2845 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1588,9 +1588,8 @@ long keyctl_session_to_parent(void)
* The caller must have Setattr permission to change keyring restrictions.
*
* The requested type name may be a NULL pointer to reject all attempts
- * to link to the keyring. If _type is non-NULL, _restriction can be
- * NULL or a pointer to a string describing the restriction. If _type is
- * NULL, _restriction must also be NULL.
+ * to link to the keyring. In this case, _restriction must also be NULL.
+ * Otherwise, both _type and _restriction must be non-NULL.
*
* Returns 0 if successful.
*/
@@ -1598,7 +1597,6 @@ long keyctl_restrict_keyring(key_serial_t id, const char __user *_type,
const char __user *_restriction)
{
key_ref_t key_ref;
- bool link_reject = !_type;
char type[32];
char *restriction = NULL;
long ret;
@@ -1607,31 +1605,29 @@ long keyctl_restrict_keyring(key_serial_t id, const char __user *_type,
if (IS_ERR(key_ref))
return PTR_ERR(key_ref);

+ ret = -EINVAL;
if (_type) {
- ret = key_get_type_from_user(type, _type, sizeof(type));
- if (ret < 0)
+ if (!_restriction)
goto error;
- }

- if (_restriction) {
- if (!_type) {
- ret = -EINVAL;
+ ret = key_get_type_from_user(type, _type, sizeof(type));
+ if (ret < 0)
goto error;
- }

restriction = strndup_user(_restriction, PAGE_SIZE);
if (IS_ERR(restriction)) {
ret = PTR_ERR(restriction);
goto error;
}
+ } else {
+ if (_restriction)
+ goto error;
}

- ret = keyring_restrict(key_ref, link_reject ? NULL : type, restriction);
+ ret = keyring_restrict(key_ref, _type ? type : NULL, restriction);
kfree(restriction);
-
error:
key_ref_put(key_ref);
-
return ret;
}

--
2.15.0.531.g2ccb3012c9-goog

[Mat Martineau]

Eric,

On Thu, 30 Nov 2017, Eric Biggers wrote:

> From: Eric Biggers <ebig...@google.com>
>
> keyctl_restrict_keyring() allows through a NULL restriction when the
> "type" is non-NULL, which causes a NULL pointer dereference in
> asymmetric_lookup_restriction() when it calls strcmp() on the
> restriction string.
>
> But no key types actually use a "NULL restriction" to mean anything, so
> update keyctl_restrict_keyring() to reject it with EINVAL.

Since this fixes the bug for the asymmetric key type and ensures that
other key types won't make the same mistake, I agree this is the way to
fix it. I did not find any issues in the patch.

Thanks,

Mat

--
Mat Martineau
Intel OTC

[David Howells]

Mat Martineau <mathew.j....@linux.intel.com> wrote:

> Since this fixes the bug for the asymmetric key type and ensures that other
> key types won't make the same mistake, I agree this is the way to fix it. I
> did not find any issues in the patch.

Can I put that down as a Reviewed-by?

David

[Mat Martineau]

Yes. Looks like I missed the window for your pull request, though - I'll
be sure to add Reviewed-by in future reviews.

[Eric Biggers]

On Thu, Nov 30, 2017 at 12:44:01PM -0800, syzbot wrote:
> syzbot will keep track of this bug report.
> Once a fix for this bug is committed, please reply to this email with:
> #syz fix: exact-commit-title
> To mark this as a duplicate of another syzbot report, please reply with:
> #syz dup: exact-subject-of-another-report
> If it's a one-off invalid bug report, please reply with:
> #syz invalid
> Note: if the crash happens again, it will cause creation of a new
> bug report.
> Note: all commands must start from beginning of the line in the email body.
>

#syz fix: KEYS: reject NULL restriction string when type is specified