All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: David Howells <dhowells@redhat.com>
Cc: keyrings@vger.kernel.org, linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 6/7] keys: Add a keyctl to move a key between keyrings
Date: Wed, 29 May 2019 23:25:01 +0000	[thread overview]
Message-ID: <20190529232500.GA131466@gmail.com> (raw)
In-Reply-To: <155856412507.10428.15987388402707639951.stgit@warthog.procyon.org.uk>

On Wed, May 22, 2019 at 11:28:45PM +0100, David Howells wrote:
> Add a keyctl to atomically move a link to a key from one keyring to
> another.  The key must exist in "from" keyring and a flag can be given to
> cause the operation to fail if there's a matching key already in the "to"
> keyring.
> 
> This can be done with:
> 
> 	keyctl(KEYCTL_MOVE,
> 	       key_serial_t key,
> 	       key_serial_t from_keyring,
> 	       key_serial_t to_keyring,
> 	       unsigned int flags);
> 
> The key being moved must grant Link permission and both keyrings must grant
> Write permission.
> 
> flags should be 0 or KEYCTL_MOVE_EXCL, with the latter preventing
> displacement of a matching key from the "to" keyring.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>

This shows up after a few seconds of syzkaller fuzzing with a description of
KEYCTL_MOVE added:

WARNING: possible circular locking dependency detected
5.2.0-rc1 #5 Not tainted
------------------------------------------------------
syz-executor.28/27700 is trying to acquire lock:
00000000049888d8 (keyring_serialise_link_sem){+.+.}, at: __key_link_begin+0x1c2/0x2d0 security/keys/keyring.c:1231

but task is already holding lock:
00000000b171310c (&type->lock_class/1){+.+.}, at: __key_link_begin+0xa4/0x2d0 security/keys/keyring.c:1222

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&type->lock_class/1){+.+.}:
       lock_acquire+0x106/0x330 kernel/locking/lockdep.c:4302
       down_write_nested+0x3c/0xa0 kernel/locking/rwsem.c:177
       __key_unlink_begin+0x6c/0x110 security/keys/keyring.c:1398
       key_move+0x3ad/0x470 security/keys/keyring.c:1538
       keyctl_keyring_move+0xb6/0x120 security/keys/keyctl.c:610
       __do_sys_keyctl security/keys/keyctl.c:1823 [inline]
       __se_sys_keyctl+0x8e/0x2c0 security/keys/keyctl.c:1685
       __x64_sys_keyctl+0xbe/0x150 security/keys/keyctl.c:1685
       do_syscall_64+0x9e/0x4b0 arch/x86/entry/common.c:301
       entry_SYSCALL_64_after_hwframe+0x49/0xbe

-> #0 (keyring_serialise_link_sem){+.+.}:
       check_prevs_add kernel/locking/lockdep.c:2417 [inline]
       validate_chain kernel/locking/lockdep.c:2799 [inline]
       __lock_acquire+0x38a4/0x3c30 kernel/locking/lockdep.c:3792
       lock_acquire+0x106/0x330 kernel/locking/lockdep.c:4302
       down_write+0x38/0xa0 kernel/locking/rwsem.c:66
       __key_link_begin+0x1c2/0x2d0 security/keys/keyring.c:1231
       key_move+0xf0/0x470 security/keys/keyring.c:1529
       keyctl_keyring_move+0xb6/0x120 security/keys/keyctl.c:610
       __do_sys_keyctl security/keys/keyctl.c:1823 [inline]
       __se_sys_keyctl+0x8e/0x2c0 security/keys/keyctl.c:1685
       __x64_sys_keyctl+0xbe/0x150 security/keys/keyctl.c:1685
       do_syscall_64+0x9e/0x4b0 arch/x86/entry/common.c:301
       entry_SYSCALL_64_after_hwframe+0x49/0xbe

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&type->lock_class/1);
                               lock(keyring_serialise_link_sem);
                               lock(&type->lock_class/1);
  lock(keyring_serialise_link_sem);

 *** DEADLOCK ***

2 locks held by syz-executor.28/27700:
 #0: 000000002a03f208 (&type->lock_class){++++}, at: __key_unlink_begin+0x6c/0x110 security/keys/keyring.c:1398
 #1: 00000000b171310c (&type->lock_class/1){+.+.}, at: __key_link_begin+0xa4/0x2d0 security/keys/keyring.c:1222

stack backtrace:
CPU: 8 PID: 27700 Comm: syz-executor.28 Not tainted 5.2.0-rc1 #5
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xb1/0x118 lib/dump_stack.c:113
 print_circular_bug+0x4a4/0x4b5 kernel/locking/lockdep.c:1564
 check_prev_add+0xd1f/0x1af7 kernel/locking/lockdep.c:2309
 check_prevs_add kernel/locking/lockdep.c:2417 [inline]
 validate_chain kernel/locking/lockdep.c:2799 [inline]
 __lock_acquire+0x38a4/0x3c30 kernel/locking/lockdep.c:3792
 lock_acquire+0x106/0x330 kernel/locking/lockdep.c:4302
 down_write+0x38/0xa0 kernel/locking/rwsem.c:66
 __key_link_begin+0x1c2/0x2d0 security/keys/keyring.c:1231
 key_move+0xf0/0x470 security/keys/keyring.c:1529
 keyctl_keyring_move+0xb6/0x120 security/keys/keyctl.c:610
 __do_sys_keyctl security/keys/keyctl.c:1823 [inline]
 __se_sys_keyctl+0x8e/0x2c0 security/keys/keyctl.c:1685
 __x64_sys_keyctl+0xbe/0x150 security/keys/keyctl.c:1685
 do_syscall_64+0x9e/0x4b0 arch/x86/entry/common.c:301
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x458a09
Code: dd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 ab b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f9d53755c88 EFLAGS: 00000246 ORIG_RAX: 00000000000000fa
RAX: ffffffffffffffda RBX: 000000000071bf00 RCX: 0000000000458a09
RDX: 000000001f62d16e RSI: 000000002490e642 RDI: 000000000000001e
RBP: 00007f9d53755ca0 R08: 0000000000000000 R09: 0000000000000000
R10: 000000001afbc80a R11: 0000000000000246 R12: 00007f9d537566d4
R13: 00000000004ac12c R14: 00000000006ebd68 R15: 0000000000000003
FAULT_INJECTION: forcing a failure.
name failslab, interval 1, probability 0, space 0, times 0
CPU: 8 PID: 27700 Comm: syz-executor.28 Not tainted 5.2.0-rc1 #5
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xb1/0x118 lib/dump_stack.c:113
 fail_dump lib/fault-inject.c:51 [inline]
 should_fail+0x61e/0x720 lib/fault-inject.c:143
 __should_failslab+0xec/0x120 mm/failslab.c:32
 should_failslab+0x9/0x14 mm/slab_common.c:1610
 slab_pre_alloc_hook mm/slab.h:420 [inline]
 slab_alloc mm/slab.c:3312 [inline]
 kmem_cache_alloc_trace+0x146/0x2a0 mm/slab.c:3553
 kmalloc include/linux/slab.h:547 [inline]
 kzalloc include/linux/slab.h:742 [inline]
 assoc_array_insert+0xcc/0x440 lib/assoc_array.c:985
 __key_link_begin+0x120/0x2d0 security/keys/keyring.c:1236
 key_move+0xf0/0x470 security/keys/keyring.c:1529
 keyctl_keyring_move+0xb6/0x120 security/keys/keyctl.c:610
 __do_sys_keyctl security/keys/keyctl.c:1823 [inline]
 __se_sys_keyctl+0x8e/0x2c0 security/keys/keyctl.c:1685
 __x64_sys_keyctl+0xbe/0x150 security/keys/keyctl.c:1685
 do_syscall_64+0x9e/0x4b0 arch/x86/entry/common.c:301
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x458a09
Code: dd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 ab b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f9d53755c88 EFLAGS: 00000246 ORIG_RAX: 00000000000000fa
RAX: ffffffffffffffda RBX: 000000000071bf00 RCX: 0000000000458a09
RDX: 000000001f62d16e RSI: 000000002490e642 RDI: 000000000000001e
RBP: 00007f9d53755ca0 R08: 0000000000000000 R09: 0000000000000000
R10: 000000001afbc80a R11: 0000000000000246 R12: 00007f9d537566d4
R13: 00000000004ac12c R14: 00000000006ebd68 R15: 0000000000000003

WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org>
To: David Howells <dhowells@redhat.com>
Cc: keyrings@vger.kernel.org, linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 6/7] keys: Add a keyctl to move a key between keyrings
Date: Wed, 29 May 2019 16:25:01 -0700	[thread overview]
Message-ID: <20190529232500.GA131466@gmail.com> (raw)
In-Reply-To: <155856412507.10428.15987388402707639951.stgit@warthog.procyon.org.uk>

On Wed, May 22, 2019 at 11:28:45PM +0100, David Howells wrote:
> Add a keyctl to atomically move a link to a key from one keyring to
> another.  The key must exist in "from" keyring and a flag can be given to
> cause the operation to fail if there's a matching key already in the "to"
> keyring.
> 
> This can be done with:
> 
> 	keyctl(KEYCTL_MOVE,
> 	       key_serial_t key,
> 	       key_serial_t from_keyring,
> 	       key_serial_t to_keyring,
> 	       unsigned int flags);
> 
> The key being moved must grant Link permission and both keyrings must grant
> Write permission.
> 
> flags should be 0 or KEYCTL_MOVE_EXCL, with the latter preventing
> displacement of a matching key from the "to" keyring.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>

This shows up after a few seconds of syzkaller fuzzing with a description of
KEYCTL_MOVE added:

WARNING: possible circular locking dependency detected
5.2.0-rc1 #5 Not tainted
------------------------------------------------------
syz-executor.28/27700 is trying to acquire lock:
00000000049888d8 (keyring_serialise_link_sem){+.+.}, at: __key_link_begin+0x1c2/0x2d0 security/keys/keyring.c:1231

but task is already holding lock:
00000000b171310c (&type->lock_class/1){+.+.}, at: __key_link_begin+0xa4/0x2d0 security/keys/keyring.c:1222

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&type->lock_class/1){+.+.}:
       lock_acquire+0x106/0x330 kernel/locking/lockdep.c:4302
       down_write_nested+0x3c/0xa0 kernel/locking/rwsem.c:177
       __key_unlink_begin+0x6c/0x110 security/keys/keyring.c:1398
       key_move+0x3ad/0x470 security/keys/keyring.c:1538
       keyctl_keyring_move+0xb6/0x120 security/keys/keyctl.c:610
       __do_sys_keyctl security/keys/keyctl.c:1823 [inline]
       __se_sys_keyctl+0x8e/0x2c0 security/keys/keyctl.c:1685
       __x64_sys_keyctl+0xbe/0x150 security/keys/keyctl.c:1685
       do_syscall_64+0x9e/0x4b0 arch/x86/entry/common.c:301
       entry_SYSCALL_64_after_hwframe+0x49/0xbe

-> #0 (keyring_serialise_link_sem){+.+.}:
       check_prevs_add kernel/locking/lockdep.c:2417 [inline]
       validate_chain kernel/locking/lockdep.c:2799 [inline]
       __lock_acquire+0x38a4/0x3c30 kernel/locking/lockdep.c:3792
       lock_acquire+0x106/0x330 kernel/locking/lockdep.c:4302
       down_write+0x38/0xa0 kernel/locking/rwsem.c:66
       __key_link_begin+0x1c2/0x2d0 security/keys/keyring.c:1231
       key_move+0xf0/0x470 security/keys/keyring.c:1529
       keyctl_keyring_move+0xb6/0x120 security/keys/keyctl.c:610
       __do_sys_keyctl security/keys/keyctl.c:1823 [inline]
       __se_sys_keyctl+0x8e/0x2c0 security/keys/keyctl.c:1685
       __x64_sys_keyctl+0xbe/0x150 security/keys/keyctl.c:1685
       do_syscall_64+0x9e/0x4b0 arch/x86/entry/common.c:301
       entry_SYSCALL_64_after_hwframe+0x49/0xbe

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&type->lock_class/1);
                               lock(keyring_serialise_link_sem);
                               lock(&type->lock_class/1);
  lock(keyring_serialise_link_sem);

 *** DEADLOCK ***

2 locks held by syz-executor.28/27700:
 #0: 000000002a03f208 (&type->lock_class){++++}, at: __key_unlink_begin+0x6c/0x110 security/keys/keyring.c:1398
 #1: 00000000b171310c (&type->lock_class/1){+.+.}, at: __key_link_begin+0xa4/0x2d0 security/keys/keyring.c:1222

stack backtrace:
CPU: 8 PID: 27700 Comm: syz-executor.28 Not tainted 5.2.0-rc1 #5
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xb1/0x118 lib/dump_stack.c:113
 print_circular_bug+0x4a4/0x4b5 kernel/locking/lockdep.c:1564
 check_prev_add+0xd1f/0x1af7 kernel/locking/lockdep.c:2309
 check_prevs_add kernel/locking/lockdep.c:2417 [inline]
 validate_chain kernel/locking/lockdep.c:2799 [inline]
 __lock_acquire+0x38a4/0x3c30 kernel/locking/lockdep.c:3792
 lock_acquire+0x106/0x330 kernel/locking/lockdep.c:4302
 down_write+0x38/0xa0 kernel/locking/rwsem.c:66
 __key_link_begin+0x1c2/0x2d0 security/keys/keyring.c:1231
 key_move+0xf0/0x470 security/keys/keyring.c:1529
 keyctl_keyring_move+0xb6/0x120 security/keys/keyctl.c:610
 __do_sys_keyctl security/keys/keyctl.c:1823 [inline]
 __se_sys_keyctl+0x8e/0x2c0 security/keys/keyctl.c:1685
 __x64_sys_keyctl+0xbe/0x150 security/keys/keyctl.c:1685
 do_syscall_64+0x9e/0x4b0 arch/x86/entry/common.c:301
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x458a09
Code: dd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 ab b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f9d53755c88 EFLAGS: 00000246 ORIG_RAX: 00000000000000fa
RAX: ffffffffffffffda RBX: 000000000071bf00 RCX: 0000000000458a09
RDX: 000000001f62d16e RSI: 000000002490e642 RDI: 000000000000001e
RBP: 00007f9d53755ca0 R08: 0000000000000000 R09: 0000000000000000
R10: 000000001afbc80a R11: 0000000000000246 R12: 00007f9d537566d4
R13: 00000000004ac12c R14: 00000000006ebd68 R15: 0000000000000003
FAULT_INJECTION: forcing a failure.
name failslab, interval 1, probability 0, space 0, times 0
CPU: 8 PID: 27700 Comm: syz-executor.28 Not tainted 5.2.0-rc1 #5
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xb1/0x118 lib/dump_stack.c:113
 fail_dump lib/fault-inject.c:51 [inline]
 should_fail+0x61e/0x720 lib/fault-inject.c:143
 __should_failslab+0xec/0x120 mm/failslab.c:32
 should_failslab+0x9/0x14 mm/slab_common.c:1610
 slab_pre_alloc_hook mm/slab.h:420 [inline]
 slab_alloc mm/slab.c:3312 [inline]
 kmem_cache_alloc_trace+0x146/0x2a0 mm/slab.c:3553
 kmalloc include/linux/slab.h:547 [inline]
 kzalloc include/linux/slab.h:742 [inline]
 assoc_array_insert+0xcc/0x440 lib/assoc_array.c:985
 __key_link_begin+0x120/0x2d0 security/keys/keyring.c:1236
 key_move+0xf0/0x470 security/keys/keyring.c:1529
 keyctl_keyring_move+0xb6/0x120 security/keys/keyctl.c:610
 __do_sys_keyctl security/keys/keyctl.c:1823 [inline]
 __se_sys_keyctl+0x8e/0x2c0 security/keys/keyctl.c:1685
 __x64_sys_keyctl+0xbe/0x150 security/keys/keyctl.c:1685
 do_syscall_64+0x9e/0x4b0 arch/x86/entry/common.c:301
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x458a09
Code: dd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 ab b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f9d53755c88 EFLAGS: 00000246 ORIG_RAX: 00000000000000fa
RAX: ffffffffffffffda RBX: 000000000071bf00 RCX: 0000000000458a09
RDX: 000000001f62d16e RSI: 000000002490e642 RDI: 000000000000001e
RBP: 00007f9d53755ca0 R08: 0000000000000000 R09: 0000000000000000
R10: 000000001afbc80a R11: 0000000000000246 R12: 00007f9d537566d4
R13: 00000000004ac12c R14: 00000000006ebd68 R15: 0000000000000003

  parent reply	other threads:[~2019-05-29 23:25 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-22 22:28 [PATCH 0/7] keys: Miscellany David Howells
2019-05-22 22:28 ` David Howells
2019-05-22 22:28 ` [PATCH 1/7] keys: sparse: Fix key_fs[ug]id_changed() David Howells
2019-05-22 22:28   ` David Howells
2019-05-24 19:38   ` James Morris
2019-05-24 19:38     ` James Morris
2019-05-22 22:28 ` [PATCH 2/7] keys: sparse: Fix incorrect RCU accesses David Howells
2019-05-22 22:28   ` David Howells
2019-05-25  3:57   ` James Morris
2019-05-25  3:57     ` James Morris
2019-05-22 22:28 ` [PATCH 3/7] keys: sparse: Fix kdoc mismatches David Howells
2019-05-22 22:28   ` David Howells
2019-05-25  3:57   ` James Morris
2019-05-25  3:57     ` James Morris
2019-05-22 22:28 ` [PATCH 4/7] keys: Break bits out of key_unlink() David Howells
2019-05-22 22:28   ` David Howells
2019-05-28 20:41   ` James Morris
2019-05-28 20:41     ` James Morris
2019-05-22 22:28 ` [PATCH 5/7] keys: Make __key_link_begin() handle lockdep nesting David Howells
2019-05-22 22:28   ` David Howells
2019-05-28 20:42   ` James Morris
2019-05-28 20:42     ` James Morris
2019-05-22 22:28 ` [PATCH 6/7] keys: Add a keyctl to move a key between keyrings David Howells
2019-05-22 22:28   ` David Howells
2019-05-28 20:51   ` James Morris
2019-05-28 20:51     ` James Morris
2019-05-29 21:34     ` David Howells
2019-05-29 21:34       ` David Howells
2019-05-29 23:25   ` Eric Biggers [this message]
2019-05-29 23:25     ` Eric Biggers
2019-05-30 13:31     ` David Howells
2019-05-30 13:31       ` David Howells
2019-05-22 22:28 ` [PATCH 7/7] keys: Grant Link permission to possessers of request_key auth keys David Howells
2019-05-22 22:28   ` David Howells
2019-05-28 21:01   ` James Morris
2019-05-28 21:01     ` James Morris

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=20190529232500.GA131466@gmail.com \
    --to=ebiggers@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.