* WARNING in apparmor_cred_free
@ 2019-01-11 9:43 syzbot
2019-01-11 22:11 ` Casey Schaufler
0 siblings, 1 reply; 13+ messages in thread
From: syzbot @ 2019-01-11 9:43 UTC (permalink / raw)
To: jmorris, john.johansen, linux-kernel, linux-security-module,
serge, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: b808822a75a3 Add linux-next specific files for 20190111
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=179c22f7400000
kernel config: https://syzkaller.appspot.com/x/.config?x=c052ead0aed5001b
dashboard link: https://syzkaller.appspot.com/bug?extid=69ca07954461f189e808
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=162d947f400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=139f6c37400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+69ca07954461f189e808@syzkaller.appspotmail.com
------------[ cut here ]------------
AppArmor WARN cred_label: ((!blob)):
WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30 cred_label
security/apparmor/include/cred.h:30 [inline]
WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30
apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.0.0-rc1-next-20190111 #10
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1db/0x2d0 lib/dump_stack.c:113
panic+0x2cb/0x65c kernel/panic.c:214
__warn.cold+0x20/0x48 kernel/panic.c:571
report_bug+0x263/0x2b0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:178 [inline]
fixup_bug arch/x86/kernel/traps.c:173 [inline]
do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:290
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
RIP: 0010:cred_label security/apparmor/include/cred.h:30 [inline]
RIP: 0010:apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
Code: 7c 88 48 c7 c7 00 d0 7c 88 e8 fd 70 f2 fd 0f 0b eb a9 e8 54 3f 29 fe
48 c7 c6 c0 df 7c 88 48 c7 c7 00 d0 7c 88 e8 e1 70 f2 fd <0f> 0b 48 b8 00
00 00 00 00 fc ff df 80 38 00 75 4a 4c 8b 2c 25 00
RSP: 0018:ffff8880ae6079f8 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000100 RSI: ffffffff81687fa6 RDI: 0000000000000006
RBP: ffff8880ae607a18 R08: ffffffff8987dec0 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff8880a86b3100
R13: ffff8880a86b3100 R14: ffff8880a86b3188 R15: dffffc0000000000
security_cred_free+0x4b/0xf0 security/security.c:1490
put_cred_rcu+0x21f/0x6e0 kernel/cred.c:118
__rcu_reclaim kernel/rcu/rcu.h:240 [inline]
rcu_do_batch kernel/rcu/tree.c:2486 [inline]
invoke_rcu_callbacks kernel/rcu/tree.c:2799 [inline]
rcu_core+0xc4a/0x1680 kernel/rcu/tree.c:2780
__do_softirq+0x30b/0xb11 kernel/softirq.c:292
invoke_softirq kernel/softirq.c:373 [inline]
irq_exit+0x180/0x1d0 kernel/softirq.c:413
exiting_irq arch/x86/include/asm/apic.h:536 [inline]
smp_apic_timer_interrupt+0x1b7/0x760 arch/x86/kernel/apic/apic.c:1062
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:807
</IRQ>
RIP: 0010:native_safe_halt+0x2/0x10 arch/x86/include/asm/irqflags.h:58
Code: ff ff ff 48 89 c7 48 89 45 d8 e8 79 6f d0 f9 48 8b 45 d8 e9 ce fe ff
ff 48 89 df e8 68 6f d0 f9 eb 82 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66
2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
RSP: 0018:ffffffff89807c60 EFLAGS: 00000282 ORIG_RAX: ffffffffffffff13
RAX: 1ffffffff1325061 RBX: 1ffffffff1300f8f RCX: 0000000000000000
RDX: dffffc0000000000 RSI: 0000000000000001 RDI: ffffffff8987e73c
RBP: ffffffff89807d20 R08: ffffffff8987dec0 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: ffffffff89807cf8 R14: 0000000000000000 R15: ffffffff899282f8
arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:555
default_idle_call+0x36/0x90 kernel/sched/idle.c:93
cpuidle_idle_call kernel/sched/idle.c:153 [inline]
do_idle+0x386/0x5d0 kernel/sched/idle.c:262
cpu_startup_entry+0x1b/0x20 kernel/sched/idle.c:353
rest_init+0x245/0x37b init/main.c:442
arch_call_rest_init+0xe/0x1b
start_kernel+0x882/0x8bd init/main.c:742
x86_64_start_reservations+0x29/0x2b arch/x86/kernel/head64.c:470
x86_64_start_kernel+0x77/0x7b arch/x86/kernel/head64.c:451
secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:243
Kernel Offset: disabled
Rebooting in 86400 seconds..
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: WARNING in apparmor_cred_free
2019-01-11 9:43 WARNING in apparmor_cred_free syzbot
@ 2019-01-11 22:11 ` Casey Schaufler
2019-01-11 22:30 ` John Johansen
0 siblings, 1 reply; 13+ messages in thread
From: Casey Schaufler @ 2019-01-11 22:11 UTC (permalink / raw)
To: jmorris, john.johansen, linux-kernel, linux-security-module, serge
Cc: Casey Schaufler
On 1/11/2019 1:43 AM, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: b808822a75a3 Add linux-next specific files for 20190111
> git tree: linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=179c22f7400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=c052ead0aed5001b
> dashboard link: https://syzkaller.appspot.com/bug?extid=69ca07954461f189e808
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=162d947f400000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=139f6c37400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+69ca07954461f189e808@syzkaller.appspotmail.com
>
> ------------[ cut here ]------------
> AppArmor WARN cred_label: ((!blob)):
> WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30 cred_label security/apparmor/include/cred.h:30 [inline]
> WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30 apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
> Kernel panic - not syncing: panic_on_warn set ...
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.0.0-rc1-next-20190111 #10
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> <IRQ>
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1db/0x2d0 lib/dump_stack.c:113
> panic+0x2cb/0x65c kernel/panic.c:214
> __warn.cold+0x20/0x48 kernel/panic.c:571
> report_bug+0x263/0x2b0 lib/bug.c:186
> fixup_bug arch/x86/kernel/traps.c:178 [inline]
> fixup_bug arch/x86/kernel/traps.c:173 [inline]
> do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
> do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:290
> invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
> RIP: 0010:cred_label security/apparmor/include/cred.h:30 [inline]
> RIP: 0010:apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
> Code: 7c 88 48 c7 c7 00 d0 7c 88 e8 fd 70 f2 fd 0f 0b eb a9 e8 54 3f 29 fe 48 c7 c6 c0 df 7c 88 48 c7 c7 00 d0 7c 88 e8 e1 70 f2 fd <0f> 0b 48 b8 00 00 00 00 00 fc ff df 80 38 00 75 4a 4c 8b 2c 25 00
> RSP: 0018:ffff8880ae6079f8 EFLAGS: 00010286
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> RDX: 0000000000000100 RSI: ffffffff81687fa6 RDI: 0000000000000006
> RBP: ffff8880ae607a18 R08: ffffffff8987dec0 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: ffff8880a86b3100
> R13: ffff8880a86b3100 R14: ffff8880a86b3188 R15: dffffc0000000000
> security_cred_free+0x4b/0xf0 security/security.c:1490
The obvious thing to do is put a check in security_cred_free
for a NULL cred->security, in which case the LSM hooks
wouldn't get called. It's not clear to me how we got a cred
that doesn't have an allocated security blob.
> put_cred_rcu+0x21f/0x6e0 kernel/cred.c:118
> __rcu_reclaim kernel/rcu/rcu.h:240 [inline]
> rcu_do_batch kernel/rcu/tree.c:2486 [inline]
> invoke_rcu_callbacks kernel/rcu/tree.c:2799 [inline]
> rcu_core+0xc4a/0x1680 kernel/rcu/tree.c:2780
> __do_softirq+0x30b/0xb11 kernel/softirq.c:292
> invoke_softirq kernel/softirq.c:373 [inline]
> irq_exit+0x180/0x1d0 kernel/softirq.c:413
> exiting_irq arch/x86/include/asm/apic.h:536 [inline]
> smp_apic_timer_interrupt+0x1b7/0x760 arch/x86/kernel/apic/apic.c:1062
> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:807
> </IRQ>
> RIP: 0010:native_safe_halt+0x2/0x10 arch/x86/include/asm/irqflags.h:58
> Code: ff ff ff 48 89 c7 48 89 45 d8 e8 79 6f d0 f9 48 8b 45 d8 e9 ce fe ff ff 48 89 df e8 68 6f d0 f9 eb 82 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
> RSP: 0018:ffffffff89807c60 EFLAGS: 00000282 ORIG_RAX: ffffffffffffff13
> RAX: 1ffffffff1325061 RBX: 1ffffffff1300f8f RCX: 0000000000000000
> RDX: dffffc0000000000 RSI: 0000000000000001 RDI: ffffffff8987e73c
> RBP: ffffffff89807d20 R08: ffffffff8987dec0 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> R13: ffffffff89807cf8 R14: 0000000000000000 R15: ffffffff899282f8
> arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:555
> default_idle_call+0x36/0x90 kernel/sched/idle.c:93
> cpuidle_idle_call kernel/sched/idle.c:153 [inline]
> do_idle+0x386/0x5d0 kernel/sched/idle.c:262
> cpu_startup_entry+0x1b/0x20 kernel/sched/idle.c:353
> rest_init+0x245/0x37b init/main.c:442
> arch_call_rest_init+0xe/0x1b
> start_kernel+0x882/0x8bd init/main.c:742
> x86_64_start_reservations+0x29/0x2b arch/x86/kernel/head64.c:470
> x86_64_start_kernel+0x77/0x7b arch/x86/kernel/head64.c:451
> secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:243
> Kernel Offset: disabled
> Rebooting in 86400 seconds..
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: WARNING in apparmor_cred_free
2019-01-11 22:11 ` Casey Schaufler
@ 2019-01-11 22:30 ` John Johansen
2019-01-11 22:43 ` Casey Schaufler
0 siblings, 1 reply; 13+ messages in thread
From: John Johansen @ 2019-01-11 22:30 UTC (permalink / raw)
To: Casey Schaufler, jmorris, linux-kernel, linux-security-module, serge
On 1/11/19 2:11 PM, Casey Schaufler wrote:
> On 1/11/2019 1:43 AM, syzbot wrote:
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit: b808822a75a3 Add linux-next specific files for 20190111
>> git tree: linux-next
>> console output: https://syzkaller.appspot.com/x/log.txt?x=179c22f7400000
>> kernel config: https://syzkaller.appspot.com/x/.config?x=c052ead0aed5001b
>> dashboard link: https://syzkaller.appspot.com/bug?extid=69ca07954461f189e808
>> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=162d947f400000
>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=139f6c37400000
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+69ca07954461f189e808@syzkaller.appspotmail.com
>>
>> ------------[ cut here ]------------
>> AppArmor WARN cred_label: ((!blob)):
>> WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30 cred_label security/apparmor/include/cred.h:30 [inline]
>> WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30 apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
>> Kernel panic - not syncing: panic_on_warn set ...
>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.0.0-rc1-next-20190111 #10
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>> Call Trace:
>> <IRQ>
>> __dump_stack lib/dump_stack.c:77 [inline]
>> dump_stack+0x1db/0x2d0 lib/dump_stack.c:113
>> panic+0x2cb/0x65c kernel/panic.c:214
>> __warn.cold+0x20/0x48 kernel/panic.c:571
>> report_bug+0x263/0x2b0 lib/bug.c:186
>> fixup_bug arch/x86/kernel/traps.c:178 [inline]
>> fixup_bug arch/x86/kernel/traps.c:173 [inline]
>> do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
>> do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:290
>> invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
>> RIP: 0010:cred_label security/apparmor/include/cred.h:30 [inline]
>> RIP: 0010:apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
>> Code: 7c 88 48 c7 c7 00 d0 7c 88 e8 fd 70 f2 fd 0f 0b eb a9 e8 54 3f 29 fe 48 c7 c6 c0 df 7c 88 48 c7 c7 00 d0 7c 88 e8 e1 70 f2 fd <0f> 0b 48 b8 00 00 00 00 00 fc ff df 80 38 00 75 4a 4c 8b 2c 25 00
>> RSP: 0018:ffff8880ae6079f8 EFLAGS: 00010286
>> RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
>> RDX: 0000000000000100 RSI: ffffffff81687fa6 RDI: 0000000000000006
>> RBP: ffff8880ae607a18 R08: ffffffff8987dec0 R09: 0000000000000000
>> R10: 0000000000000000 R11: 0000000000000000 R12: ffff8880a86b3100
>> R13: ffff8880a86b3100 R14: ffff8880a86b3188 R15: dffffc0000000000
>> security_cred_free+0x4b/0xf0 security/security.c:1490
>
> The obvious thing to do is put a check in security_cred_free
> for a NULL cred->security, in which case the LSM hooks
> wouldn't get called.
Right, but the question is should we? To my thinking we shouldn't
ever have a cred without cred->security, unless the cred was
allocated but a later step in its construction, say allocating
->security failed.
In which case I'd rather see the cred directly freed and not
call into security_cred_free() as I like being able to detect
corrupt creds.
We certainly can still do the check for security on only live creds
but I would like to understand this particular failure better first
> It's not clear to me how we got a cred
> that doesn't have an allocated security blob.
I have been trying to figure that one out as well.
>
>> put_cred_rcu+0x21f/0x6e0 kernel/cred.c:118
>> __rcu_reclaim kernel/rcu/rcu.h:240 [inline]
>> rcu_do_batch kernel/rcu/tree.c:2486 [inline]
>> invoke_rcu_callbacks kernel/rcu/tree.c:2799 [inline]
>> rcu_core+0xc4a/0x1680 kernel/rcu/tree.c:2780
>> __do_softirq+0x30b/0xb11 kernel/softirq.c:292
>> invoke_softirq kernel/softirq.c:373 [inline]
>> irq_exit+0x180/0x1d0 kernel/softirq.c:413
>> exiting_irq arch/x86/include/asm/apic.h:536 [inline]
>> smp_apic_timer_interrupt+0x1b7/0x760 arch/x86/kernel/apic/apic.c:1062
>> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:807
>> </IRQ>
>> RIP: 0010:native_safe_halt+0x2/0x10 arch/x86/include/asm/irqflags.h:58
>> Code: ff ff ff 48 89 c7 48 89 45 d8 e8 79 6f d0 f9 48 8b 45 d8 e9 ce fe ff ff 48 89 df e8 68 6f d0 f9 eb 82 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
>> RSP: 0018:ffffffff89807c60 EFLAGS: 00000282 ORIG_RAX: ffffffffffffff13
>> RAX: 1ffffffff1325061 RBX: 1ffffffff1300f8f RCX: 0000000000000000
>> RDX: dffffc0000000000 RSI: 0000000000000001 RDI: ffffffff8987e73c
>> RBP: ffffffff89807d20 R08: ffffffff8987dec0 R09: 0000000000000000
>> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
>> R13: ffffffff89807cf8 R14: 0000000000000000 R15: ffffffff899282f8
>> arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:555
>> default_idle_call+0x36/0x90 kernel/sched/idle.c:93
>> cpuidle_idle_call kernel/sched/idle.c:153 [inline]
>> do_idle+0x386/0x5d0 kernel/sched/idle.c:262
>> cpu_startup_entry+0x1b/0x20 kernel/sched/idle.c:353
>> rest_init+0x245/0x37b init/main.c:442
>> arch_call_rest_init+0xe/0x1b
>> start_kernel+0x882/0x8bd init/main.c:742
>> x86_64_start_reservations+0x29/0x2b arch/x86/kernel/head64.c:470
>> x86_64_start_kernel+0x77/0x7b arch/x86/kernel/head64.c:451
>> secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:243
>> Kernel Offset: disabled
>> Rebooting in 86400 seconds..
>>
>>
>> ---
>> This bug is generated by a bot. It may contain errors.
>> See https://goo.gl/tpsmEJ for more information about syzbot.
>> syzbot engineers can be reached at syzkaller@googlegroups.com.
>>
>> syzbot will keep track of this bug report. See:
>> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot.
>> syzbot can test patches for this bug, for details see:
>> https://goo.gl/tpsmEJ#testing-patches
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: WARNING in apparmor_cred_free
2019-01-11 22:30 ` John Johansen
@ 2019-01-11 22:43 ` Casey Schaufler
2019-01-11 23:20 ` Casey Schaufler
0 siblings, 1 reply; 13+ messages in thread
From: Casey Schaufler @ 2019-01-11 22:43 UTC (permalink / raw)
To: John Johansen, jmorris, linux-kernel, linux-security-module, serge
On 1/11/2019 2:30 PM, John Johansen wrote:
> On 1/11/19 2:11 PM, Casey Schaufler wrote:
>> On 1/11/2019 1:43 AM, syzbot wrote:
>>> Hello,
>>>
>>> syzbot found the following crash on:
>>>
>>> HEAD commit: b808822a75a3 Add linux-next specific files for 20190111
>>> git tree: linux-next
>>> console output: https://syzkaller.appspot.com/x/log.txt?x=179c22f7400000
>>> kernel config: https://syzkaller.appspot.com/x/.config?x=c052ead0aed5001b
>>> dashboard link: https://syzkaller.appspot.com/bug?extid=69ca07954461f189e808
>>> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=162d947f400000
>>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=139f6c37400000
>>>
>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>> Reported-by: syzbot+69ca07954461f189e808@syzkaller.appspotmail.com
>>>
>>> ------------[ cut here ]------------
>>> AppArmor WARN cred_label: ((!blob)):
>>> WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30 cred_label security/apparmor/include/cred.h:30 [inline]
>>> WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30 apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
>>> Kernel panic - not syncing: panic_on_warn set ...
>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.0.0-rc1-next-20190111 #10
>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>>> Call Trace:
>>> <IRQ>
>>> __dump_stack lib/dump_stack.c:77 [inline]
>>> dump_stack+0x1db/0x2d0 lib/dump_stack.c:113
>>> panic+0x2cb/0x65c kernel/panic.c:214
>>> __warn.cold+0x20/0x48 kernel/panic.c:571
>>> report_bug+0x263/0x2b0 lib/bug.c:186
>>> fixup_bug arch/x86/kernel/traps.c:178 [inline]
>>> fixup_bug arch/x86/kernel/traps.c:173 [inline]
>>> do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
>>> do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:290
>>> invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
>>> RIP: 0010:cred_label security/apparmor/include/cred.h:30 [inline]
>>> RIP: 0010:apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
>>> Code: 7c 88 48 c7 c7 00 d0 7c 88 e8 fd 70 f2 fd 0f 0b eb a9 e8 54 3f 29 fe 48 c7 c6 c0 df 7c 88 48 c7 c7 00 d0 7c 88 e8 e1 70 f2 fd <0f> 0b 48 b8 00 00 00 00 00 fc ff df 80 38 00 75 4a 4c 8b 2c 25 00
>>> RSP: 0018:ffff8880ae6079f8 EFLAGS: 00010286
>>> RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
>>> RDX: 0000000000000100 RSI: ffffffff81687fa6 RDI: 0000000000000006
>>> RBP: ffff8880ae607a18 R08: ffffffff8987dec0 R09: 0000000000000000
>>> R10: 0000000000000000 R11: 0000000000000000 R12: ffff8880a86b3100
>>> R13: ffff8880a86b3100 R14: ffff8880a86b3188 R15: dffffc0000000000
>>> security_cred_free+0x4b/0xf0 security/security.c:1490
>> The obvious thing to do is put a check in security_cred_free
>> for a NULL cred->security, in which case the LSM hooks
>> wouldn't get called.
> Right, but the question is should we? To my thinking we shouldn't
> ever have a cred without cred->security, unless the cred was
> allocated but a later step in its construction, say allocating
> ->security failed.
If allocating ->security fails in security_cred_alloc_blank()
or security_prepare_creds() you don't have to do anything but
fail because the LSM hooks are not called before the allocation.
> In which case I'd rather see the cred directly freed and not
> call into security_cred_free() as I like being able to detect
> corrupt creds.
I think we need to look for some bit of code that's setting
cred->security to NULL inappropriately.
> We certainly can still do the check for security on only live creds
> but I would like to understand this particular failure better first
>
>> It's not clear to me how we got a cred
>> that doesn't have an allocated security blob.
> I have been trying to figure that one out as well.
>
>
>>> put_cred_rcu+0x21f/0x6e0 kernel/cred.c:118
>>> __rcu_reclaim kernel/rcu/rcu.h:240 [inline]
>>> rcu_do_batch kernel/rcu/tree.c:2486 [inline]
>>> invoke_rcu_callbacks kernel/rcu/tree.c:2799 [inline]
>>> rcu_core+0xc4a/0x1680 kernel/rcu/tree.c:2780
>>> __do_softirq+0x30b/0xb11 kernel/softirq.c:292
>>> invoke_softirq kernel/softirq.c:373 [inline]
>>> irq_exit+0x180/0x1d0 kernel/softirq.c:413
>>> exiting_irq arch/x86/include/asm/apic.h:536 [inline]
>>> smp_apic_timer_interrupt+0x1b7/0x760 arch/x86/kernel/apic/apic.c:1062
>>> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:807
>>> </IRQ>
>>> RIP: 0010:native_safe_halt+0x2/0x10 arch/x86/include/asm/irqflags.h:58
>>> Code: ff ff ff 48 89 c7 48 89 45 d8 e8 79 6f d0 f9 48 8b 45 d8 e9 ce fe ff ff 48 89 df e8 68 6f d0 f9 eb 82 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
>>> RSP: 0018:ffffffff89807c60 EFLAGS: 00000282 ORIG_RAX: ffffffffffffff13
>>> RAX: 1ffffffff1325061 RBX: 1ffffffff1300f8f RCX: 0000000000000000
>>> RDX: dffffc0000000000 RSI: 0000000000000001 RDI: ffffffff8987e73c
>>> RBP: ffffffff89807d20 R08: ffffffff8987dec0 R09: 0000000000000000
>>> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
>>> R13: ffffffff89807cf8 R14: 0000000000000000 R15: ffffffff899282f8
>>> arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:555
>>> default_idle_call+0x36/0x90 kernel/sched/idle.c:93
>>> cpuidle_idle_call kernel/sched/idle.c:153 [inline]
>>> do_idle+0x386/0x5d0 kernel/sched/idle.c:262
>>> cpu_startup_entry+0x1b/0x20 kernel/sched/idle.c:353
>>> rest_init+0x245/0x37b init/main.c:442
>>> arch_call_rest_init+0xe/0x1b
>>> start_kernel+0x882/0x8bd init/main.c:742
>>> x86_64_start_reservations+0x29/0x2b arch/x86/kernel/head64.c:470
>>> x86_64_start_kernel+0x77/0x7b arch/x86/kernel/head64.c:451
>>> secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:243
>>> Kernel Offset: disabled
>>> Rebooting in 86400 seconds..
>>>
>>>
>>> ---
>>> This bug is generated by a bot. It may contain errors.
>>> See https://goo.gl/tpsmEJ for more information about syzbot.
>>> syzbot engineers can be reached at syzkaller@googlegroups.com.
>>>
>>> syzbot will keep track of this bug report. See:
>>> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot.
>>> syzbot can test patches for this bug, for details see:
>>> https://goo.gl/tpsmEJ#testing-patches
>>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: WARNING in apparmor_cred_free
2019-01-11 22:43 ` Casey Schaufler
@ 2019-01-11 23:20 ` Casey Schaufler
2019-01-12 1:46 ` Casey Schaufler
2019-01-12 1:48 ` Casey Schaufler
0 siblings, 2 replies; 13+ messages in thread
From: Casey Schaufler @ 2019-01-11 23:20 UTC (permalink / raw)
To: John Johansen, jmorris, linux-kernel, linux-security-module, serge
Cc: Casey Schaufler
On 1/11/2019 2:43 PM, Casey Schaufler wrote:
> On 1/11/2019 2:30 PM, John Johansen wrote:
>> On 1/11/19 2:11 PM, Casey Schaufler wrote:
>>> On 1/11/2019 1:43 AM, syzbot wrote:
>>>> Hello,
>>>>
>>>> syzbot found the following crash on:
>>>>
>>>> HEAD commit: b808822a75a3 Add linux-next specific files for 20190111
>>>> git tree: linux-next
>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=179c22f7400000
>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=c052ead0aed5001b
>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=69ca07954461f189e808
>>>> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>>>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=162d947f400000
>>>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=139f6c37400000
>>>>
>>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>>> Reported-by: syzbot+69ca07954461f189e808@syzkaller.appspotmail.com
>>>>
>>>> ------------[ cut here ]------------
>>>> AppArmor WARN cred_label: ((!blob)):
>>>> WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30 cred_label security/apparmor/include/cred.h:30 [inline]
>>>> WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30 apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
>>>> Kernel panic - not syncing: panic_on_warn set ...
>>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.0.0-rc1-next-20190111 #10
>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>>>> Call Trace:
>>>> <IRQ>
>>>> __dump_stack lib/dump_stack.c:77 [inline]
>>>> dump_stack+0x1db/0x2d0 lib/dump_stack.c:113
>>>> panic+0x2cb/0x65c kernel/panic.c:214
>>>> __warn.cold+0x20/0x48 kernel/panic.c:571
>>>> report_bug+0x263/0x2b0 lib/bug.c:186
>>>> fixup_bug arch/x86/kernel/traps.c:178 [inline]
>>>> fixup_bug arch/x86/kernel/traps.c:173 [inline]
>>>> do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
>>>> do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:290
>>>> invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
>>>> RIP: 0010:cred_label security/apparmor/include/cred.h:30 [inline]
>>>> RIP: 0010:apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
>>>> Code: 7c 88 48 c7 c7 00 d0 7c 88 e8 fd 70 f2 fd 0f 0b eb a9 e8 54 3f 29 fe 48 c7 c6 c0 df 7c 88 48 c7 c7 00 d0 7c 88 e8 e1 70 f2 fd <0f> 0b 48 b8 00 00 00 00 00 fc ff df 80 38 00 75 4a 4c 8b 2c 25 00
>>>> RSP: 0018:ffff8880ae6079f8 EFLAGS: 00010286
>>>> RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
>>>> RDX: 0000000000000100 RSI: ffffffff81687fa6 RDI: 0000000000000006
>>>> RBP: ffff8880ae607a18 R08: ffffffff8987dec0 R09: 0000000000000000
>>>> R10: 0000000000000000 R11: 0000000000000000 R12: ffff8880a86b3100
>>>> R13: ffff8880a86b3100 R14: ffff8880a86b3188 R15: dffffc0000000000
>>>> security_cred_free+0x4b/0xf0 security/security.c:1490
>>> The obvious thing to do is put a check in security_cred_free
>>> for a NULL cred->security, in which case the LSM hooks
>>> wouldn't get called.
>> Right, but the question is should we? To my thinking we shouldn't
>> ever have a cred without cred->security, unless the cred was
>> allocated but a later step in its construction, say allocating
>> ->security failed.
> If allocating ->security fails in security_cred_alloc_blank()
> or security_prepare_creds() you don't have to do anything but
> fail because the LSM hooks are not called before the allocation.
>
>> In which case I'd rather see the cred directly freed and not
>> call into security_cred_free() as I like being able to detect
>> corrupt creds.
> I think we need to look for some bit of code that's setting
> cred->security to NULL inappropriately.
If security_cred_alloc_blank() fails for lack of memory
in cred_alloc_blank() abort_creds() will be called. This
in turn calls put_cred() and put_cred_rcu(), which will
call security_cred_free() with ->security set to NULL.
put_cred_rcu() is the only caller of security_cred_free().
The ->security == NULL check can be in either put_cred_rcu()
or in security_cred_free(). I suggest the latter as the
cleanest option.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: WARNING in apparmor_cred_free
2019-01-11 23:20 ` Casey Schaufler
@ 2019-01-12 1:46 ` Casey Schaufler
2019-01-14 11:12 ` Dmitry Vyukov
` (2 more replies)
2019-01-12 1:48 ` Casey Schaufler
1 sibling, 3 replies; 13+ messages in thread
From: Casey Schaufler @ 2019-01-12 1:46 UTC (permalink / raw)
To: John Johansen, jmorris, linux-kernel, linux-security-module, serge
On 1/11/2019 3:20 PM, Casey Schaufler wrote:
> On 1/11/2019 2:43 PM, Casey Schaufler wrote:
>> On 1/11/2019 2:30 PM, John Johansen wrote:
>>> On 1/11/19 2:11 PM, Casey Schaufler wrote:
>>>> On 1/11/2019 1:43 AM, syzbot wrote:
>>>>> Hello,
>>>>>
>>>>> syzbot found the following crash on:
>>>>>
>>>>> HEAD commit: b808822a75a3 Add linux-next specific files for 20190111
>>>>> git tree: linux-next
>>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=179c22f7400000
>>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=c052ead0aed5001b
>>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=69ca07954461f189e808
>>>>> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>>>>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=162d947f400000
>>>>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=139f6c37400000
>>>>>
>>>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>>>> Reported-by: syzbot+69ca07954461f189e808@syzkaller.appspotmail.com
>>>>>
>>>>> ------------[ cut here ]------------
>>>>> AppArmor WARN cred_label: ((!blob)):
>>>>> WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30 cred_label security/apparmor/include/cred.h:30 [inline]
>>>>> WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30 apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
>>>>> Kernel panic - not syncing: panic_on_warn set ...
>>>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.0.0-rc1-next-20190111 #10
>>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>>>>> Call Trace:
>>>>> <IRQ>
>>>>> __dump_stack lib/dump_stack.c:77 [inline]
>>>>> dump_stack+0x1db/0x2d0 lib/dump_stack.c:113
>>>>> panic+0x2cb/0x65c kernel/panic.c:214
>>>>> __warn.cold+0x20/0x48 kernel/panic.c:571
>>>>> report_bug+0x263/0x2b0 lib/bug.c:186
>>>>> fixup_bug arch/x86/kernel/traps.c:178 [inline]
>>>>> fixup_bug arch/x86/kernel/traps.c:173 [inline]
>>>>> do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
>>>>> do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:290
>>>>> invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
>>>>> RIP: 0010:cred_label security/apparmor/include/cred.h:30 [inline]
>>>>> RIP: 0010:apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
>>>>> Code: 7c 88 48 c7 c7 00 d0 7c 88 e8 fd 70 f2 fd 0f 0b eb a9 e8 54 3f 29 fe 48 c7 c6 c0 df 7c 88 48 c7 c7 00 d0 7c 88 e8 e1 70 f2 fd <0f> 0b 48 b8 00 00 00 00 00 fc ff df 80 38 00 75 4a 4c 8b 2c 25 00
>>>>> RSP: 0018:ffff8880ae6079f8 EFLAGS: 00010286
>>>>> RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
>>>>> RDX: 0000000000000100 RSI: ffffffff81687fa6 RDI: 0000000000000006
>>>>> RBP: ffff8880ae607a18 R08: ffffffff8987dec0 R09: 0000000000000000
>>>>> R10: 0000000000000000 R11: 0000000000000000 R12: ffff8880a86b3100
>>>>> R13: ffff8880a86b3100 R14: ffff8880a86b3188 R15: dffffc0000000000
>>>>> security_cred_free+0x4b/0xf0 security/security.c:1490
>>>> The obvious thing to do is put a check in security_cred_free
>>>> for a NULL cred->security, in which case the LSM hooks
>>>> wouldn't get called.
>>> Right, but the question is should we? To my thinking we shouldn't
>>> ever have a cred without cred->security, unless the cred was
>>> allocated but a later step in its construction, say allocating
>>> ->security failed.
>> If allocating ->security fails in security_cred_alloc_blank()
>> or security_prepare_creds() you don't have to do anything but
>> fail because the LSM hooks are not called before the allocation.
>>
>>> In which case I'd rather see the cred directly freed and not
>>> call into security_cred_free() as I like being able to detect
>>> corrupt creds.
>> I think we need to look for some bit of code that's setting
>> cred->security to NULL inappropriately.
> If security_cred_alloc_blank() fails for lack of memory
> in cred_alloc_blank() abort_creds() will be called. This
> in turn calls put_cred() and put_cred_rcu(), which will
> call security_cred_free() with ->security set to NULL.
>
> put_cred_rcu() is the only caller of security_cred_free().
> The ->security == NULL check can be in either put_cred_rcu()
> or in security_cred_free(). I suggest the latter as the
> cleanest option.
From 47134986133c822e1d88860fa2b108f92c97a7ff Mon Sep 17 00:00:00 2001
From: Casey Schaufler <casey@schaufler-ca.com>
Date: Fri, 11 Jan 2019 17:31:50 -0800
Subject: [PATCH 1/2] LSM: Check for NULL cred-security on free
Check that the cred security blob has been set before trying
to clean it up. There is a case during credential initialization
that could result in this.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
security/security.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/security/security.c b/security/security.c
index a618e22df5c6..7bffc86d4e87 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1477,6 +1477,13 @@ int security_cred_alloc_blank(struct cred *cred, gfp_t gfp)
void security_cred_free(struct cred *cred)
{
+ /*
+ * There is a failure case in prepare_creds() that
+ * may result in a call here with ->security being NULL.
+ */
+ if (unlikely(cred->security == NULL))
+ return;
+
call_void_hook(cred_free, cred);
kfree(cred->security);
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: WARNING in apparmor_cred_free
2019-01-11 23:20 ` Casey Schaufler
2019-01-12 1:46 ` Casey Schaufler
@ 2019-01-12 1:48 ` Casey Schaufler
2019-01-16 23:30 ` James Morris
2019-01-17 0:19 ` John Johansen
1 sibling, 2 replies; 13+ messages in thread
From: Casey Schaufler @ 2019-01-12 1:48 UTC (permalink / raw)
To: John Johansen, jmorris, linux-security-module, serge; +Cc: Casey Schaufler
From 1a2a99f3cfd069baf9aa0ef91529558bcbe64c14 Mon Sep 17 00:00:00 2001
From: Casey Schaufler <casey@schaufler-ca.com>
Date: Fri, 11 Jan 2019 17:37:56 -0800
Subject: [PATCH 2/2] AppArmor: Fix credential blob assertions
The assertions for NULL credential blobs don't take blob
offsetting into account. Check the raw data, not th eoffset value.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
security/apparmor/include/cred.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/apparmor/include/cred.h b/security/apparmor/include/cred.h
index b9504a05fddc..29cef1b5687c 100644
--- a/security/apparmor/include/cred.h
+++ b/security/apparmor/include/cred.h
@@ -27,7 +27,7 @@ static inline struct aa_label *cred_label(const struct cred *cred)
{
struct aa_label **blob = cred->security + apparmor_blob_sizes.lbs_cred;
- AA_BUG(!blob);
+ AA_BUG(!cred->security);
return *blob;
}
@@ -36,7 +36,7 @@ static inline void set_cred_label(const struct cred *cred,
{
struct aa_label **blob = cred->security + apparmor_blob_sizes.lbs_cred;
- AA_BUG(!blob);
+ AA_BUG(!cred->security);
*blob = label;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: WARNING in apparmor_cred_free
2019-01-12 1:46 ` Casey Schaufler
@ 2019-01-14 11:12 ` Dmitry Vyukov
2019-01-16 21:14 ` James Morris
2019-01-16 23:44 ` James Morris
2 siblings, 0 replies; 13+ messages in thread
From: Dmitry Vyukov @ 2019-01-14 11:12 UTC (permalink / raw)
To: Casey Schaufler
Cc: John Johansen, James Morris, LKML, linux-security-module,
Serge E. Hallyn
On Sat, Jan 12, 2019 at 2:47 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 1/11/2019 3:20 PM, Casey Schaufler wrote:
> > On 1/11/2019 2:43 PM, Casey Schaufler wrote:
> >> On 1/11/2019 2:30 PM, John Johansen wrote:
> >>> On 1/11/19 2:11 PM, Casey Schaufler wrote:
> >>>> On 1/11/2019 1:43 AM, syzbot wrote:
> >>>>> Hello,
> >>>>>
> >>>>> syzbot found the following crash on:
> >>>>>
> >>>>> HEAD commit: b808822a75a3 Add linux-next specific files for 20190111
> >>>>> git tree: linux-next
> >>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=179c22f7400000
> >>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=c052ead0aed5001b
> >>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=69ca07954461f189e808
> >>>>> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> >>>>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=162d947f400000
> >>>>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=139f6c37400000
> >>>>>
> >>>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> >>>>> Reported-by: syzbot+69ca07954461f189e808@syzkaller.appspotmail.com
/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\
Please include the tag for tracking purposes.
> >>>>> ------------[ cut here ]------------
> >>>>> AppArmor WARN cred_label: ((!blob)):
> >>>>> WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30 cred_label security/apparmor/include/cred.h:30 [inline]
> >>>>> WARNING: CPU: 0 PID: 0 at security/apparmor/include/cred.h:30 apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
> >>>>> Kernel panic - not syncing: panic_on_warn set ...
> >>>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.0.0-rc1-next-20190111 #10
> >>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> >>>>> Call Trace:
> >>>>> <IRQ>
> >>>>> __dump_stack lib/dump_stack.c:77 [inline]
> >>>>> dump_stack+0x1db/0x2d0 lib/dump_stack.c:113
> >>>>> panic+0x2cb/0x65c kernel/panic.c:214
> >>>>> __warn.cold+0x20/0x48 kernel/panic.c:571
> >>>>> report_bug+0x263/0x2b0 lib/bug.c:186
> >>>>> fixup_bug arch/x86/kernel/traps.c:178 [inline]
> >>>>> fixup_bug arch/x86/kernel/traps.c:173 [inline]
> >>>>> do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
> >>>>> do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:290
> >>>>> invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
> >>>>> RIP: 0010:cred_label security/apparmor/include/cred.h:30 [inline]
> >>>>> RIP: 0010:apparmor_cred_free+0x12f/0x1a0 security/apparmor/lsm.c:62
> >>>>> Code: 7c 88 48 c7 c7 00 d0 7c 88 e8 fd 70 f2 fd 0f 0b eb a9 e8 54 3f 29 fe 48 c7 c6 c0 df 7c 88 48 c7 c7 00 d0 7c 88 e8 e1 70 f2 fd <0f> 0b 48 b8 00 00 00 00 00 fc ff df 80 38 00 75 4a 4c 8b 2c 25 00
> >>>>> RSP: 0018:ffff8880ae6079f8 EFLAGS: 00010286
> >>>>> RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> >>>>> RDX: 0000000000000100 RSI: ffffffff81687fa6 RDI: 0000000000000006
> >>>>> RBP: ffff8880ae607a18 R08: ffffffff8987dec0 R09: 0000000000000000
> >>>>> R10: 0000000000000000 R11: 0000000000000000 R12: ffff8880a86b3100
> >>>>> R13: ffff8880a86b3100 R14: ffff8880a86b3188 R15: dffffc0000000000
> >>>>> security_cred_free+0x4b/0xf0 security/security.c:1490
> >>>> The obvious thing to do is put a check in security_cred_free
> >>>> for a NULL cred->security, in which case the LSM hooks
> >>>> wouldn't get called.
> >>> Right, but the question is should we? To my thinking we shouldn't
> >>> ever have a cred without cred->security, unless the cred was
> >>> allocated but a later step in its construction, say allocating
> >>> ->security failed.
> >> If allocating ->security fails in security_cred_alloc_blank()
> >> or security_prepare_creds() you don't have to do anything but
> >> fail because the LSM hooks are not called before the allocation.
> >>
> >>> In which case I'd rather see the cred directly freed and not
> >>> call into security_cred_free() as I like being able to detect
> >>> corrupt creds.
> >> I think we need to look for some bit of code that's setting
> >> cred->security to NULL inappropriately.
> > If security_cred_alloc_blank() fails for lack of memory
> > in cred_alloc_blank() abort_creds() will be called. This
> > in turn calls put_cred() and put_cred_rcu(), which will
> > call security_cred_free() with ->security set to NULL.
> >
> > put_cred_rcu() is the only caller of security_cred_free().
> > The ->security == NULL check can be in either put_cred_rcu()
> > or in security_cred_free(). I suggest the latter as the
> > cleanest option.
>
> From 47134986133c822e1d88860fa2b108f92c97a7ff Mon Sep 17 00:00:00 2001
> From: Casey Schaufler <casey@schaufler-ca.com>
> Date: Fri, 11 Jan 2019 17:31:50 -0800
> Subject: [PATCH 1/2] LSM: Check for NULL cred-security on free
>
> Check that the cred security blob has been set before trying
> to clean it up. There is a case during credential initialization
> that could result in this.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> security/security.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/security/security.c b/security/security.c
> index a618e22df5c6..7bffc86d4e87 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1477,6 +1477,13 @@ int security_cred_alloc_blank(struct cred *cred, gfp_t gfp)
>
> void security_cred_free(struct cred *cred)
> {
> + /*
> + * There is a failure case in prepare_creds() that
> + * may result in a call here with ->security being NULL.
> + */
> + if (unlikely(cred->security == NULL))
> + return;
> +
> call_void_hook(cred_free, cred);
>
> kfree(cred->security);
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: WARNING in apparmor_cred_free
2019-01-12 1:46 ` Casey Schaufler
2019-01-14 11:12 ` Dmitry Vyukov
@ 2019-01-16 21:14 ` James Morris
2019-01-16 22:36 ` John Johansen
2019-01-16 23:44 ` James Morris
2 siblings, 1 reply; 13+ messages in thread
From: James Morris @ 2019-01-16 21:14 UTC (permalink / raw)
To: Casey Schaufler; +Cc: John Johansen, linux-kernel, linux-security-module, serge
On Fri, 11 Jan 2019, Casey Schaufler wrote:
> >From 47134986133c822e1d88860fa2b108f92c97a7ff Mon Sep 17 00:00:00 2001
> From: Casey Schaufler <casey@schaufler-ca.com>
> Date: Fri, 11 Jan 2019 17:31:50 -0800
> Subject: [PATCH 1/2] LSM: Check for NULL cred-security on free
>
> Check that the cred security blob has been set before trying
> to clean it up. There is a case during credential initialization
> that could result in this.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
JJ: does this fix the problem?
> ---
> security/security.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/security/security.c b/security/security.c
> index a618e22df5c6..7bffc86d4e87 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1477,6 +1477,13 @@ int security_cred_alloc_blank(struct cred *cred, gfp_t gfp)
>
> void security_cred_free(struct cred *cred)
> {
> + /*
> + * There is a failure case in prepare_creds() that
> + * may result in a call here with ->security being NULL.
> + */
> + if (unlikely(cred->security == NULL))
> + return;
> +
> call_void_hook(cred_free, cred);
>
> kfree(cred->security);
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: WARNING in apparmor_cred_free
2019-01-16 21:14 ` James Morris
@ 2019-01-16 22:36 ` John Johansen
0 siblings, 0 replies; 13+ messages in thread
From: John Johansen @ 2019-01-16 22:36 UTC (permalink / raw)
To: James Morris, Casey Schaufler; +Cc: linux-kernel, linux-security-module, serge
On 1/16/19 1:14 PM, James Morris wrote:
> On Fri, 11 Jan 2019, Casey Schaufler wrote:
>
>> >From 47134986133c822e1d88860fa2b108f92c97a7ff Mon Sep 17 00:00:00 2001
>> From: Casey Schaufler <casey@schaufler-ca.com>
>> Date: Fri, 11 Jan 2019 17:31:50 -0800
>> Subject: [PATCH 1/2] LSM: Check for NULL cred-security on free
>>
>> Check that the cred security blob has been set before trying
>> to clean it up. There is a case during credential initialization
>> that could result in this.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>
> JJ: does this fix the problem?
>
sorry for not responding earlier, yes it does.
Acked-by: John Johansen <john.johansen@canonical.com>
>> ---
>> security/security.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/security/security.c b/security/security.c
>> index a618e22df5c6..7bffc86d4e87 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -1477,6 +1477,13 @@ int security_cred_alloc_blank(struct cred *cred, gfp_t gfp)
>>
>> void security_cred_free(struct cred *cred)
>> {
>> + /*
>> + * There is a failure case in prepare_creds() that
>> + * may result in a call here with ->security being NULL.
>> + */
>> + if (unlikely(cred->security == NULL))
>> + return;
>> +
>> call_void_hook(cred_free, cred);
>>
>> kfree(cred->security);
>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: WARNING in apparmor_cred_free
2019-01-12 1:48 ` Casey Schaufler
@ 2019-01-16 23:30 ` James Morris
2019-01-17 0:19 ` John Johansen
1 sibling, 0 replies; 13+ messages in thread
From: James Morris @ 2019-01-16 23:30 UTC (permalink / raw)
To: Casey Schaufler; +Cc: John Johansen, linux-security-module, serge
On Fri, 11 Jan 2019, Casey Schaufler wrote:
> >From 1a2a99f3cfd069baf9aa0ef91529558bcbe64c14 Mon Sep 17 00:00:00 2001
> From: Casey Schaufler <casey@schaufler-ca.com>
> Date: Fri, 11 Jan 2019 17:37:56 -0800
> Subject: [PATCH 2/2] AppArmor: Fix credential blob assertions
>
> The assertions for NULL credential blobs don't take blob
> offsetting into account. Check the raw data, not th eoffset value.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
JJ: can you ack this if it loogs good to you?
> ---
> security/apparmor/include/cred.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/apparmor/include/cred.h b/security/apparmor/include/cred.h
> index b9504a05fddc..29cef1b5687c 100644
> --- a/security/apparmor/include/cred.h
> +++ b/security/apparmor/include/cred.h
> @@ -27,7 +27,7 @@ static inline struct aa_label *cred_label(const struct cred *cred)
> {
> struct aa_label **blob = cred->security + apparmor_blob_sizes.lbs_cred;
>
> - AA_BUG(!blob);
> + AA_BUG(!cred->security);
> return *blob;
> }
>
> @@ -36,7 +36,7 @@ static inline void set_cred_label(const struct cred *cred,
> {
> struct aa_label **blob = cred->security + apparmor_blob_sizes.lbs_cred;
>
> - AA_BUG(!blob);
> + AA_BUG(!cred->security);
> *blob = label;
> }
>
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: WARNING in apparmor_cred_free
2019-01-12 1:46 ` Casey Schaufler
2019-01-14 11:12 ` Dmitry Vyukov
2019-01-16 21:14 ` James Morris
@ 2019-01-16 23:44 ` James Morris
2 siblings, 0 replies; 13+ messages in thread
From: James Morris @ 2019-01-16 23:44 UTC (permalink / raw)
To: Casey Schaufler; +Cc: John Johansen, linux-kernel, linux-security-module, serge
On Fri, 11 Jan 2019, Casey Schaufler wrote:
> >From 47134986133c822e1d88860fa2b108f92c97a7ff Mon Sep 17 00:00:00 2001
> From: Casey Schaufler <casey@schaufler-ca.com>
> Date: Fri, 11 Jan 2019 17:31:50 -0800
> Subject: [PATCH 1/2] LSM: Check for NULL cred-security on free
>
> Check that the cred security blob has been set before trying
> to clean it up. There is a case during credential initialization
> that could result in this.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git fixes-v5.0-rc2
NOTE: For patches which you are submitting for inclusion, start a new
thread and include [PATCH] in the subject line.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: WARNING in apparmor_cred_free
2019-01-12 1:48 ` Casey Schaufler
2019-01-16 23:30 ` James Morris
@ 2019-01-17 0:19 ` John Johansen
1 sibling, 0 replies; 13+ messages in thread
From: John Johansen @ 2019-01-17 0:19 UTC (permalink / raw)
To: Casey Schaufler, jmorris, linux-security-module, serge
On 1/11/19 5:48 PM, Casey Schaufler wrote:
> From 1a2a99f3cfd069baf9aa0ef91529558bcbe64c14 Mon Sep 17 00:00:00 2001
> From: Casey Schaufler <casey@schaufler-ca.com>
> Date: Fri, 11 Jan 2019 17:37:56 -0800
> Subject: [PATCH 2/2] AppArmor: Fix credential blob assertions
>
> The assertions for NULL credential blobs don't take blob
> offsetting into account. Check the raw data, not th eoffset value.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
in the none stacking case cred->security is the aa_label pointer so
the test should be
AA_BUG(!*blob);
and I would just drop the check from set_cred_label()
> ---
> security/apparmor/include/cred.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/apparmor/include/cred.h b/security/apparmor/include/cred.h
> index b9504a05fddc..29cef1b5687c 100644
> --- a/security/apparmor/include/cred.h
> +++ b/security/apparmor/include/cred.h
> @@ -27,7 +27,7 @@ static inline struct aa_label *cred_label(const struct cred *cred)
> {
> struct aa_label **blob = cred->security + apparmor_blob_sizes.lbs_cred;
>
> - AA_BUG(!blob);
> + AA_BUG(!cred->security);
> return *blob;
> }
>
> @@ -36,7 +36,7 @@ static inline void set_cred_label(const struct cred *cred,
> {
> struct aa_label **blob = cred->security + apparmor_blob_sizes.lbs_cred;
>
> - AA_BUG(!blob);
> + AA_BUG(!cred->security);
> *blob = label;
> }
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-01-17 0:19 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-11 9:43 WARNING in apparmor_cred_free syzbot
2019-01-11 22:11 ` Casey Schaufler
2019-01-11 22:30 ` John Johansen
2019-01-11 22:43 ` Casey Schaufler
2019-01-11 23:20 ` Casey Schaufler
2019-01-12 1:46 ` Casey Schaufler
2019-01-14 11:12 ` Dmitry Vyukov
2019-01-16 21:14 ` James Morris
2019-01-16 22:36 ` John Johansen
2019-01-16 23:44 ` James Morris
2019-01-12 1:48 ` Casey Schaufler
2019-01-16 23:30 ` James Morris
2019-01-17 0:19 ` John Johansen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).