* [syzbot] general protection fault in legacy_parse_param
@ 2021-07-03 5:41 syzbot
2021-07-03 5:51 ` Dmitry Vyukov
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: syzbot @ 2021-07-03 5:41 UTC (permalink / raw)
To: linux-fsdevel, linux-kernel, syzkaller-bugs, viro
Hello,
syzbot found the following issue on:
HEAD commit: 62fb9874 Linux 5.13
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12ffa118300000
kernel config: https://syzkaller.appspot.com/x/.config?x=19404adbea015a58
dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943
compiler: Debian clang version 11.0.1-2
Unfortunately, I don't have any reproducer for this issue yet.
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com
general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
CPU: 0 PID: 20300 Comm: syz-executor.1 Not tainted 5.13.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054
Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db
RSP: 0018:ffffc90001dafd00 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000013 RCX: dffffc0000000000
RDX: 0000000000000013 RSI: 000000000000002c RDI: 0000000000000000
RBP: 0000000000000000 R08: ffffffff81e171bf R09: ffffffff81e16f95
R10: 0000000000000002 R11: ffff88807e96b880 R12: dffffc0000000000
R13: ffff888020894000 R14: 0000000000000000 R15: 000000000000002c
FS: 00007fe01ae27700(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000005645a8 CR3: 0000000018afc000 CR4: 00000000001506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
legacy_parse_param+0x461/0x7e0 fs/fs_context.c:537
vfs_parse_fs_param+0x1e5/0x460 fs/fs_context.c:117
vfs_fsconfig_locked fs/fsopen.c:265 [inline]
__do_sys_fsconfig fs/fsopen.c:439 [inline]
__se_sys_fsconfig+0xba9/0xff0 fs/fsopen.c:314
do_syscall_64+0x3f/0xb0 arch/x86/entry/common.c:47
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x4665d9
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 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 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fe01ae27188 EFLAGS: 00000246 ORIG_RAX: 00000000000001af
RAX: ffffffffffffffda RBX: 000000000056bf80 RCX: 00000000004665d9
RDX: 00000000200002c0 RSI: 0000000000000001 RDI: 0000000000000003
RBP: 00000000004bfcb9 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000020000300 R11: 0000000000000246 R12: 000000000056bf80
R13: 00007ffd4bb7c5bf R14: 00007fe01ae27300 R15: 0000000000022000
Modules linked in:
---[ end trace 5d7119165725bd63 ]---
RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054
Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db
RSP: 0018:ffffc90001dafd00 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000013 RCX: dffffc0000000000
RDX: 0000000000000013 RSI: 000000000000002c RDI: 0000000000000000
RBP: 0000000000000000 R08: ffffffff81e171bf R09: ffffffff81e16f95
R10: 0000000000000002 R11: ffff88807e96b880 R12: dffffc0000000000
R13: ffff888020894000 R14: 0000000000000000 R15: 000000000000002c
FS: 00007fe01ae27700(0000) GS:ffff8880b9b00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000004e4da0 CR3: 0000000018afc000 CR4: 00000000001506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
---
This report 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 issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [syzbot] general protection fault in legacy_parse_param 2021-07-03 5:41 [syzbot] general protection fault in legacy_parse_param syzbot @ 2021-07-03 5:51 ` Dmitry Vyukov 2021-07-03 22:16 ` Casey Schaufler 2021-08-27 14:49 ` syzbot 2021-08-28 2:11 ` syzbot 2 siblings, 1 reply; 17+ messages in thread From: Dmitry Vyukov @ 2021-07-03 5:51 UTC (permalink / raw) To: syzbot, Casey Schaufler, linux-security-module, Paul Moore, Stephen Smalley, selinux Cc: linux-fsdevel, linux-kernel, syzkaller-bugs, viro On Sat, Jul 3, 2021 at 7:41 AM syzbot <syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com> wrote: > > Hello, > > syzbot found the following issue on: > > HEAD commit: 62fb9874 Linux 5.13 > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=12ffa118300000 > kernel config: https://syzkaller.appspot.com/x/.config?x=19404adbea015a58 > dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 > compiler: Debian clang version 11.0.1-2 > > Unfortunately, I don't have any reproducer for this issue yet. > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com +Casey for what looks like a smackfs issue The crash was triggered by this test case: 21:55:33 executing program 1: r0 = fsopen(&(0x7f0000000040)='ext3\x00', 0x1) fsconfig$FSCONFIG_SET_STRING(r0, 0x1, &(0x7f00000002c0)='smackfsroot', &(0x7f0000000300)='default_permissions', 0x0) And I think the issue is in smack_fs_context_parse_param(): https://elixir.bootlin.com/linux/latest/source/security/smack/smack_lsm.c#L691 But it seems that selinux_fs_context_parse_param() contains the same issue: https://elixir.bootlin.com/linux/latest/source/security/selinux/hooks.c#L2919 +So selinux maintainers as well. > general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN > KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] > CPU: 0 PID: 20300 Comm: syz-executor.1 Not tainted 5.13.0-syzkaller #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054 > Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db > RSP: 0018:ffffc90001dafd00 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: 0000000000000013 RCX: dffffc0000000000 > RDX: 0000000000000013 RSI: 000000000000002c RDI: 0000000000000000 > RBP: 0000000000000000 R08: ffffffff81e171bf R09: ffffffff81e16f95 > R10: 0000000000000002 R11: ffff88807e96b880 R12: dffffc0000000000 > R13: ffff888020894000 R14: 0000000000000000 R15: 000000000000002c > FS: 00007fe01ae27700(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00000000005645a8 CR3: 0000000018afc000 CR4: 00000000001506f0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > legacy_parse_param+0x461/0x7e0 fs/fs_context.c:537 > vfs_parse_fs_param+0x1e5/0x460 fs/fs_context.c:117 > vfs_fsconfig_locked fs/fsopen.c:265 [inline] > __do_sys_fsconfig fs/fsopen.c:439 [inline] > __se_sys_fsconfig+0xba9/0xff0 fs/fsopen.c:314 > do_syscall_64+0x3f/0xb0 arch/x86/entry/common.c:47 > entry_SYSCALL_64_after_hwframe+0x44/0xae > RIP: 0033:0x4665d9 > Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 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 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48 > RSP: 002b:00007fe01ae27188 EFLAGS: 00000246 ORIG_RAX: 00000000000001af > RAX: ffffffffffffffda RBX: 000000000056bf80 RCX: 00000000004665d9 > RDX: 00000000200002c0 RSI: 0000000000000001 RDI: 0000000000000003 > RBP: 00000000004bfcb9 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000020000300 R11: 0000000000000246 R12: 000000000056bf80 > R13: 00007ffd4bb7c5bf R14: 00007fe01ae27300 R15: 0000000000022000 > Modules linked in: > ---[ end trace 5d7119165725bd63 ]--- > RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054 > Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db > RSP: 0018:ffffc90001dafd00 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: 0000000000000013 RCX: dffffc0000000000 > RDX: 0000000000000013 RSI: 000000000000002c RDI: 0000000000000000 > RBP: 0000000000000000 R08: ffffffff81e171bf R09: ffffffff81e16f95 > R10: 0000000000000002 R11: ffff88807e96b880 R12: dffffc0000000000 > R13: ffff888020894000 R14: 0000000000000000 R15: 000000000000002c > FS: 00007fe01ae27700(0000) GS:ffff8880b9b00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00000000004e4da0 CR3: 0000000018afc000 CR4: 00000000001506e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > > --- > This report 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 issue. See: > https://goo.gl/tpsmEJ#status for how to communicate with syzbot. > > -- > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/0000000000004e5ec705c6318557%40google.com. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-07-03 5:51 ` Dmitry Vyukov @ 2021-07-03 22:16 ` Casey Schaufler 2021-07-04 14:14 ` Paul Moore 0 siblings, 1 reply; 17+ messages in thread From: Casey Schaufler @ 2021-07-03 22:16 UTC (permalink / raw) To: Dmitry Vyukov, syzbot, linux-security-module, Paul Moore, Stephen Smalley, selinux, David Howells Cc: linux-fsdevel, linux-kernel, syzkaller-bugs, viro On 7/2/2021 10:51 PM, Dmitry Vyukov wrote: > On Sat, Jul 3, 2021 at 7:41 AM syzbot > <syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com> wrote: >> Hello, >> >> syzbot found the following issue on: >> >> HEAD commit: 62fb9874 Linux 5.13 >> git tree: upstream >> console output: https://syzkaller.appspot.com/x/log.txt?x=12ffa118300000 >> kernel config: https://syzkaller.appspot.com/x/.config?x=19404adbea015a58 >> dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 >> compiler: Debian clang version 11.0.1-2 >> >> Unfortunately, I don't have any reproducer for this issue yet. >> >> IMPORTANT: if you fix the issue, please add the following tag to the commit: >> Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com > +Casey for what looks like a smackfs issue This is from the new mount infrastructure introduced by David Howells in November 2018. It makes sense that there may be a problem in SELinux as well, as the code was introduced by the same developer at the same time for the same purpose. > > The crash was triggered by this test case: > > 21:55:33 executing program 1: > r0 = fsopen(&(0x7f0000000040)='ext3\x00', 0x1) > fsconfig$FSCONFIG_SET_STRING(r0, 0x1, &(0x7f00000002c0)='smackfsroot', > &(0x7f0000000300)='default_permissions', 0x0) > > And I think the issue is in smack_fs_context_parse_param(): > https://elixir.bootlin.com/linux/latest/source/security/smack/smack_lsm.c#L691 > > But it seems that selinux_fs_context_parse_param() contains the same issue: > https://elixir.bootlin.com/linux/latest/source/security/selinux/hooks.c#L2919 > +So selinux maintainers as well. > > > >> general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN >> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] >> CPU: 0 PID: 20300 Comm: syz-executor.1 Not tainted 5.13.0-syzkaller #0 >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 >> RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054 >> Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db >> RSP: 0018:ffffc90001dafd00 EFLAGS: 00010246 >> RAX: 0000000000000000 RBX: 0000000000000013 RCX: dffffc0000000000 >> RDX: 0000000000000013 RSI: 000000000000002c RDI: 0000000000000000 >> RBP: 0000000000000000 R08: ffffffff81e171bf R09: ffffffff81e16f95 >> R10: 0000000000000002 R11: ffff88807e96b880 R12: dffffc0000000000 >> R13: ffff888020894000 R14: 0000000000000000 R15: 000000000000002c >> FS: 00007fe01ae27700(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 00000000005645a8 CR3: 0000000018afc000 CR4: 00000000001506f0 >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 >> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 >> Call Trace: >> legacy_parse_param+0x461/0x7e0 fs/fs_context.c:537 >> vfs_parse_fs_param+0x1e5/0x460 fs/fs_context.c:117 >> vfs_fsconfig_locked fs/fsopen.c:265 [inline] >> __do_sys_fsconfig fs/fsopen.c:439 [inline] >> __se_sys_fsconfig+0xba9/0xff0 fs/fsopen.c:314 >> do_syscall_64+0x3f/0xb0 arch/x86/entry/common.c:47 >> entry_SYSCALL_64_after_hwframe+0x44/0xae >> RIP: 0033:0x4665d9 >> Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 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 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48 >> RSP: 002b:00007fe01ae27188 EFLAGS: 00000246 ORIG_RAX: 00000000000001af >> RAX: ffffffffffffffda RBX: 000000000056bf80 RCX: 00000000004665d9 >> RDX: 00000000200002c0 RSI: 0000000000000001 RDI: 0000000000000003 >> RBP: 00000000004bfcb9 R08: 0000000000000000 R09: 0000000000000000 >> R10: 0000000020000300 R11: 0000000000000246 R12: 000000000056bf80 >> R13: 00007ffd4bb7c5bf R14: 00007fe01ae27300 R15: 0000000000022000 >> Modules linked in: >> ---[ end trace 5d7119165725bd63 ]--- >> RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054 >> Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db >> RSP: 0018:ffffc90001dafd00 EFLAGS: 00010246 >> RAX: 0000000000000000 RBX: 0000000000000013 RCX: dffffc0000000000 >> RDX: 0000000000000013 RSI: 000000000000002c RDI: 0000000000000000 >> RBP: 0000000000000000 R08: ffffffff81e171bf R09: ffffffff81e16f95 >> R10: 0000000000000002 R11: ffff88807e96b880 R12: dffffc0000000000 >> R13: ffff888020894000 R14: 0000000000000000 R15: 000000000000002c >> FS: 00007fe01ae27700(0000) GS:ffff8880b9b00000(0000) knlGS:0000000000000000 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 00000000004e4da0 CR3: 0000000018afc000 CR4: 00000000001506e0 >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 >> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 >> >> >> --- >> This report 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 issue. See: >> https://goo.gl/tpsmEJ#status for how to communicate with syzbot. >> >> -- >> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. >> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com. >> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/0000000000004e5ec705c6318557%40google.com. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-07-03 22:16 ` Casey Schaufler @ 2021-07-04 14:14 ` Paul Moore 2021-07-05 5:52 ` Dmitry Vyukov 0 siblings, 1 reply; 17+ messages in thread From: Paul Moore @ 2021-07-04 14:14 UTC (permalink / raw) To: Casey Schaufler Cc: Dmitry Vyukov, syzbot, linux-security-module, Stephen Smalley, selinux, David Howells, linux-fsdevel, linux-kernel, syzkaller-bugs, viro On Sat, Jul 3, 2021 at 6:16 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > On 7/2/2021 10:51 PM, Dmitry Vyukov wrote: > > On Sat, Jul 3, 2021 at 7:41 AM syzbot > > <syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com> wrote: > >> Hello, > >> > >> syzbot found the following issue on: > >> > >> HEAD commit: 62fb9874 Linux 5.13 > >> git tree: upstream > >> console output: https://syzkaller.appspot.com/x/log.txt?x=12ffa118300000 > >> kernel config: https://syzkaller.appspot.com/x/.config?x=19404adbea015a58 > >> dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 > >> compiler: Debian clang version 11.0.1-2 > >> > >> Unfortunately, I don't have any reproducer for this issue yet. > >> > >> IMPORTANT: if you fix the issue, please add the following tag to the commit: > >> Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com > > +Casey for what looks like a smackfs issue > > This is from the new mount infrastructure introduced by > David Howells in November 2018. It makes sense that there > may be a problem in SELinux as well, as the code was introduced > by the same developer at the same time for the same purpose. > > > The crash was triggered by this test case: > > > > 21:55:33 executing program 1: > > r0 = fsopen(&(0x7f0000000040)='ext3\x00', 0x1) > > fsconfig$FSCONFIG_SET_STRING(r0, 0x1, &(0x7f00000002c0)='smackfsroot', > > &(0x7f0000000300)='default_permissions', 0x0) > > > > And I think the issue is in smack_fs_context_parse_param(): > > https://elixir.bootlin.com/linux/latest/source/security/smack/smack_lsm.c#L691 > > > > But it seems that selinux_fs_context_parse_param() contains the same issue: > > https://elixir.bootlin.com/linux/latest/source/security/selinux/hooks.c#L2919 > > +So selinux maintainers as well. > > > >> general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN > >> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] > >> CPU: 0 PID: 20300 Comm: syz-executor.1 Not tainted 5.13.0-syzkaller #0 > >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > >> RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054 > >> Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db > >> RSP: 0018:ffffc90001dafd00 EFLAGS: 00010246 > >> RAX: 0000000000000000 RBX: 0000000000000013 RCX: dffffc0000000000 > >> RDX: 0000000000000013 RSI: 000000000000002c RDI: 0000000000000000 > >> RBP: 0000000000000000 R08: ffffffff81e171bf R09: ffffffff81e16f95 > >> R10: 0000000000000002 R11: ffff88807e96b880 R12: dffffc0000000000 > >> R13: ffff888020894000 R14: 0000000000000000 R15: 000000000000002c > >> FS: 00007fe01ae27700(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000 > >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > >> CR2: 00000000005645a8 CR3: 0000000018afc000 CR4: 00000000001506f0 > >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > >> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > >> Call Trace: > >> legacy_parse_param+0x461/0x7e0 fs/fs_context.c:537 > >> vfs_parse_fs_param+0x1e5/0x460 fs/fs_context.c:117 It's Sunday morning and perhaps my mind is not yet in a "hey, let's look at VFS kernel code!" mindset, but I'm not convinced the problem is the 'param->string = NULL' assignment in the LSM hooks. In both the case of SELinux and Smack that code ends up returning either a 0 (Smack) or a 1 (SELinux) - that's a little odd in it's own way, but I don't believe it is relevant here - either way these return values are not equal to -ENOPARAM so we should end up returning early from vfs_parse_fs_param before it calls down into legacy_parse_param(): Taken from https://elixir.bootlin.com/linux/latest/source/fs/fs_context.c#L109 : ret = security_fs_context_parse_param(fc, param); if (ret != -ENOPARAM) /* Param belongs to the LSM or is disallowed by the LSM; so * don't pass to the FS. */ return ret; if (fc->ops->parse_param) { ret = fc->ops->parse_param(fc, param); if (ret != -ENOPARAM) return ret; } > >> vfs_fsconfig_locked fs/fsopen.c:265 [inline] > >> __do_sys_fsconfig fs/fsopen.c:439 [inline] > >> __se_sys_fsconfig+0xba9/0xff0 fs/fsopen.c:314 > >> do_syscall_64+0x3f/0xb0 arch/x86/entry/common.c:47 > >> entry_SYSCALL_64_after_hwframe+0x44/0xae > >> RIP: 0033:0x4665d9 > >> Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 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 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48 > >> RSP: 002b:00007fe01ae27188 EFLAGS: 00000246 ORIG_RAX: 00000000000001af > >> RAX: ffffffffffffffda RBX: 000000000056bf80 RCX: 00000000004665d9 > >> RDX: 00000000200002c0 RSI: 0000000000000001 RDI: 0000000000000003 > >> RBP: 00000000004bfcb9 R08: 0000000000000000 R09: 0000000000000000 > >> R10: 0000000020000300 R11: 0000000000000246 R12: 000000000056bf80 > >> R13: 00007ffd4bb7c5bf R14: 00007fe01ae27300 R15: 0000000000022000 > >> Modules linked in: > >> ---[ end trace 5d7119165725bd63 ]--- -- paul moore www.paul-moore.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-07-04 14:14 ` Paul Moore @ 2021-07-05 5:52 ` Dmitry Vyukov 2021-07-06 12:50 ` Paul Moore 0 siblings, 1 reply; 17+ messages in thread From: Dmitry Vyukov @ 2021-07-05 5:52 UTC (permalink / raw) To: Paul Moore Cc: Casey Schaufler, syzbot, linux-security-module, Stephen Smalley, selinux, David Howells, linux-fsdevel, linux-kernel, syzkaller-bugs, viro On Sun, Jul 4, 2021 at 4:14 PM Paul Moore <paul@paul-moore.com> wrote: > > On Sat, Jul 3, 2021 at 6:16 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > On 7/2/2021 10:51 PM, Dmitry Vyukov wrote: > > > On Sat, Jul 3, 2021 at 7:41 AM syzbot > > > <syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com> wrote: > > >> Hello, > > >> > > >> syzbot found the following issue on: > > >> > > >> HEAD commit: 62fb9874 Linux 5.13 > > >> git tree: upstream > > >> console output: https://syzkaller.appspot.com/x/log.txt?x=12ffa118300000 > > >> kernel config: https://syzkaller.appspot.com/x/.config?x=19404adbea015a58 > > >> dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 > > >> compiler: Debian clang version 11.0.1-2 > > >> > > >> Unfortunately, I don't have any reproducer for this issue yet. > > >> > > >> IMPORTANT: if you fix the issue, please add the following tag to the commit: > > >> Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com > > > +Casey for what looks like a smackfs issue > > > > This is from the new mount infrastructure introduced by > > David Howells in November 2018. It makes sense that there > > may be a problem in SELinux as well, as the code was introduced > > by the same developer at the same time for the same purpose. > > > > > The crash was triggered by this test case: > > > > > > 21:55:33 executing program 1: > > > r0 = fsopen(&(0x7f0000000040)='ext3\x00', 0x1) > > > fsconfig$FSCONFIG_SET_STRING(r0, 0x1, &(0x7f00000002c0)='smackfsroot', > > > &(0x7f0000000300)='default_permissions', 0x0) > > > > > > And I think the issue is in smack_fs_context_parse_param(): > > > https://elixir.bootlin.com/linux/latest/source/security/smack/smack_lsm.c#L691 > > > > > > But it seems that selinux_fs_context_parse_param() contains the same issue: > > > https://elixir.bootlin.com/linux/latest/source/security/selinux/hooks.c#L2919 > > > +So selinux maintainers as well. > > > > > >> general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN > > >> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] > > >> CPU: 0 PID: 20300 Comm: syz-executor.1 Not tainted 5.13.0-syzkaller #0 > > >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > > >> RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054 > > >> Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db > > >> RSP: 0018:ffffc90001dafd00 EFLAGS: 00010246 > > >> RAX: 0000000000000000 RBX: 0000000000000013 RCX: dffffc0000000000 > > >> RDX: 0000000000000013 RSI: 000000000000002c RDI: 0000000000000000 > > >> RBP: 0000000000000000 R08: ffffffff81e171bf R09: ffffffff81e16f95 > > >> R10: 0000000000000002 R11: ffff88807e96b880 R12: dffffc0000000000 > > >> R13: ffff888020894000 R14: 0000000000000000 R15: 000000000000002c > > >> FS: 00007fe01ae27700(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000 > > >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > >> CR2: 00000000005645a8 CR3: 0000000018afc000 CR4: 00000000001506f0 > > >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > >> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > >> Call Trace: > > >> legacy_parse_param+0x461/0x7e0 fs/fs_context.c:537 > > >> vfs_parse_fs_param+0x1e5/0x460 fs/fs_context.c:117 > > It's Sunday morning and perhaps my mind is not yet in a "hey, let's > look at VFS kernel code!" mindset, but I'm not convinced the problem > is the 'param->string = NULL' assignment in the LSM hooks. In both > the case of SELinux and Smack that code ends up returning either a 0 > (Smack) or a 1 (SELinux) - that's a little odd in it's own way, but I > don't believe it is relevant here - either way these return values are > not equal to -ENOPARAM so we should end up returning early from > vfs_parse_fs_param before it calls down into legacy_parse_param(): > > Taken from https://elixir.bootlin.com/linux/latest/source/fs/fs_context.c#L109 : > > ret = security_fs_context_parse_param(fc, param); > if (ret != -ENOPARAM) > /* Param belongs to the LSM or is disallowed by the LSM; so > * don't pass to the FS. > */ > return ret; > > if (fc->ops->parse_param) { > ret = fc->ops->parse_param(fc, param); > if (ret != -ENOPARAM) > return ret; > } Hi Paul, You are right. I almost connected the dots, but not exactly. Now that I read more code around, setting "param->string = NULL" in smack_fs_context_parse_param() looks correct to me (the fs copies and takes ownership of the string). I don't see how the crash happened... > > >> vfs_fsconfig_locked fs/fsopen.c:265 [inline] > > >> __do_sys_fsconfig fs/fsopen.c:439 [inline] > > >> __se_sys_fsconfig+0xba9/0xff0 fs/fsopen.c:314 > > >> do_syscall_64+0x3f/0xb0 arch/x86/entry/common.c:47 > > >> entry_SYSCALL_64_after_hwframe+0x44/0xae > > >> RIP: 0033:0x4665d9 > > >> Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 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 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48 > > >> RSP: 002b:00007fe01ae27188 EFLAGS: 00000246 ORIG_RAX: 00000000000001af > > >> RAX: ffffffffffffffda RBX: 000000000056bf80 RCX: 00000000004665d9 > > >> RDX: 00000000200002c0 RSI: 0000000000000001 RDI: 0000000000000003 > > >> RBP: 00000000004bfcb9 R08: 0000000000000000 R09: 0000000000000000 > > >> R10: 0000000020000300 R11: 0000000000000246 R12: 000000000056bf80 > > >> R13: 00007ffd4bb7c5bf R14: 00007fe01ae27300 R15: 0000000000022000 > > >> Modules linked in: > > >> ---[ end trace 5d7119165725bd63 ]--- > > -- > paul moore > www.paul-moore.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-07-05 5:52 ` Dmitry Vyukov @ 2021-07-06 12:50 ` Paul Moore 2021-08-27 15:30 ` Christian Brauner 0 siblings, 1 reply; 17+ messages in thread From: Paul Moore @ 2021-07-06 12:50 UTC (permalink / raw) To: Dmitry Vyukov Cc: Casey Schaufler, syzbot, linux-security-module, Stephen Smalley, selinux, David Howells, linux-fsdevel, linux-kernel, syzkaller-bugs, viro On Mon, Jul 5, 2021 at 1:52 AM Dmitry Vyukov <dvyukov@google.com> wrote: > On Sun, Jul 4, 2021 at 4:14 PM Paul Moore <paul@paul-moore.com> wrote: > > On Sat, Jul 3, 2021 at 6:16 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > > On 7/2/2021 10:51 PM, Dmitry Vyukov wrote: > > > > On Sat, Jul 3, 2021 at 7:41 AM syzbot > > > > <syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com> wrote: > > > >> Hello, > > > >> > > > >> syzbot found the following issue on: > > > >> > > > >> HEAD commit: 62fb9874 Linux 5.13 > > > >> git tree: upstream > > > >> console output: https://syzkaller.appspot.com/x/log.txt?x=12ffa118300000 > > > >> kernel config: https://syzkaller.appspot.com/x/.config?x=19404adbea015a58 > > > >> dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 > > > >> compiler: Debian clang version 11.0.1-2 > > > >> > > > >> Unfortunately, I don't have any reproducer for this issue yet. > > > >> > > > >> IMPORTANT: if you fix the issue, please add the following tag to the commit: > > > >> Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com > > > > +Casey for what looks like a smackfs issue > > > > > > This is from the new mount infrastructure introduced by > > > David Howells in November 2018. It makes sense that there > > > may be a problem in SELinux as well, as the code was introduced > > > by the same developer at the same time for the same purpose. > > > > > > > The crash was triggered by this test case: > > > > > > > > 21:55:33 executing program 1: > > > > r0 = fsopen(&(0x7f0000000040)='ext3\x00', 0x1) > > > > fsconfig$FSCONFIG_SET_STRING(r0, 0x1, &(0x7f00000002c0)='smackfsroot', > > > > &(0x7f0000000300)='default_permissions', 0x0) > > > > > > > > And I think the issue is in smack_fs_context_parse_param(): > > > > https://elixir.bootlin.com/linux/latest/source/security/smack/smack_lsm.c#L691 > > > > > > > > But it seems that selinux_fs_context_parse_param() contains the same issue: > > > > https://elixir.bootlin.com/linux/latest/source/security/selinux/hooks.c#L2919 > > > > +So selinux maintainers as well. > > > > > > > >> general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN > > > >> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] > > > >> CPU: 0 PID: 20300 Comm: syz-executor.1 Not tainted 5.13.0-syzkaller #0 > > > >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > > > >> RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054 > > > >> Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db > > > >> RSP: 0018:ffffc90001dafd00 EFLAGS: 00010246 > > > >> RAX: 0000000000000000 RBX: 0000000000000013 RCX: dffffc0000000000 > > > >> RDX: 0000000000000013 RSI: 000000000000002c RDI: 0000000000000000 > > > >> RBP: 0000000000000000 R08: ffffffff81e171bf R09: ffffffff81e16f95 > > > >> R10: 0000000000000002 R11: ffff88807e96b880 R12: dffffc0000000000 > > > >> R13: ffff888020894000 R14: 0000000000000000 R15: 000000000000002c > > > >> FS: 00007fe01ae27700(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000 > > > >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > >> CR2: 00000000005645a8 CR3: 0000000018afc000 CR4: 00000000001506f0 > > > >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > > >> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > > >> Call Trace: > > > >> legacy_parse_param+0x461/0x7e0 fs/fs_context.c:537 > > > >> vfs_parse_fs_param+0x1e5/0x460 fs/fs_context.c:117 > > > > It's Sunday morning and perhaps my mind is not yet in a "hey, let's > > look at VFS kernel code!" mindset, but I'm not convinced the problem > > is the 'param->string = NULL' assignment in the LSM hooks. In both > > the case of SELinux and Smack that code ends up returning either a 0 > > (Smack) or a 1 (SELinux) - that's a little odd in it's own way, but I > > don't believe it is relevant here - either way these return values are > > not equal to -ENOPARAM so we should end up returning early from > > vfs_parse_fs_param before it calls down into legacy_parse_param(): > > > > Taken from https://elixir.bootlin.com/linux/latest/source/fs/fs_context.c#L109 : > > > > ret = security_fs_context_parse_param(fc, param); > > if (ret != -ENOPARAM) > > /* Param belongs to the LSM or is disallowed by the LSM; so > > * don't pass to the FS. > > */ > > return ret; > > > > if (fc->ops->parse_param) { > > ret = fc->ops->parse_param(fc, param); > > if (ret != -ENOPARAM) > > return ret; > > } > > Hi Paul, > > You are right. > I almost connected the dots, but not exactly. > Now that I read more code around, setting "param->string = NULL" in > smack_fs_context_parse_param() looks correct to me (the fs copies and > takes ownership of the string). > > I don't see how the crash happened... FWIW, I poked around a bit too and couldn't see anything obvious either, but I can't pretend to know as much about the VFS layer as the VFS folks. Hopefully they might have better luck. -- paul moore www.paul-moore.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-07-06 12:50 ` Paul Moore @ 2021-08-27 15:30 ` Christian Brauner 2021-08-27 15:40 ` Casey Schaufler 0 siblings, 1 reply; 17+ messages in thread From: Christian Brauner @ 2021-08-27 15:30 UTC (permalink / raw) To: Paul Moore Cc: Dmitry Vyukov, Casey Schaufler, syzbot, linux-security-module, Stephen Smalley, selinux, David Howells, linux-fsdevel, linux-kernel, syzkaller-bugs, viro On Tue, Jul 06, 2021 at 08:50:44AM -0400, Paul Moore wrote: > On Mon, Jul 5, 2021 at 1:52 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Sun, Jul 4, 2021 at 4:14 PM Paul Moore <paul@paul-moore.com> wrote: > > > On Sat, Jul 3, 2021 at 6:16 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > > > On 7/2/2021 10:51 PM, Dmitry Vyukov wrote: > > > > > On Sat, Jul 3, 2021 at 7:41 AM syzbot > > > > > <syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com> wrote: > > > > >> Hello, > > > > >> > > > > >> syzbot found the following issue on: > > > > >> > > > > >> HEAD commit: 62fb9874 Linux 5.13 > > > > >> git tree: upstream > > > > >> console output: https://syzkaller.appspot.com/x/log.txt?x=12ffa118300000 > > > > >> kernel config: https://syzkaller.appspot.com/x/.config?x=19404adbea015a58 > > > > >> dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 > > > > >> compiler: Debian clang version 11.0.1-2 > > > > >> > > > > >> Unfortunately, I don't have any reproducer for this issue yet. > > > > >> > > > > >> IMPORTANT: if you fix the issue, please add the following tag to the commit: > > > > >> Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com > > > > > +Casey for what looks like a smackfs issue > > > > > > > > This is from the new mount infrastructure introduced by > > > > David Howells in November 2018. It makes sense that there > > > > may be a problem in SELinux as well, as the code was introduced > > > > by the same developer at the same time for the same purpose. > > > > > > > > > The crash was triggered by this test case: > > > > > > > > > > 21:55:33 executing program 1: > > > > > r0 = fsopen(&(0x7f0000000040)='ext3\x00', 0x1) > > > > > fsconfig$FSCONFIG_SET_STRING(r0, 0x1, &(0x7f00000002c0)='smackfsroot', > > > > > &(0x7f0000000300)='default_permissions', 0x0) > > > > > > > > > > And I think the issue is in smack_fs_context_parse_param(): > > > > > https://elixir.bootlin.com/linux/latest/source/security/smack/smack_lsm.c#L691 > > > > > > > > > > But it seems that selinux_fs_context_parse_param() contains the same issue: > > > > > https://elixir.bootlin.com/linux/latest/source/security/selinux/hooks.c#L2919 > > > > > +So selinux maintainers as well. > > > > > > > > > >> general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN > > > > >> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] > > > > >> CPU: 0 PID: 20300 Comm: syz-executor.1 Not tainted 5.13.0-syzkaller #0 > > > > >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > > > > >> RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054 > > > > >> Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db > > > > >> RSP: 0018:ffffc90001dafd00 EFLAGS: 00010246 > > > > >> RAX: 0000000000000000 RBX: 0000000000000013 RCX: dffffc0000000000 > > > > >> RDX: 0000000000000013 RSI: 000000000000002c RDI: 0000000000000000 > > > > >> RBP: 0000000000000000 R08: ffffffff81e171bf R09: ffffffff81e16f95 > > > > >> R10: 0000000000000002 R11: ffff88807e96b880 R12: dffffc0000000000 > > > > >> R13: ffff888020894000 R14: 0000000000000000 R15: 000000000000002c > > > > >> FS: 00007fe01ae27700(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000 > > > > >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > > >> CR2: 00000000005645a8 CR3: 0000000018afc000 CR4: 00000000001506f0 > > > > >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > > > >> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > > > >> Call Trace: > > > > >> legacy_parse_param+0x461/0x7e0 fs/fs_context.c:537 > > > > >> vfs_parse_fs_param+0x1e5/0x460 fs/fs_context.c:117 > > > > > > It's Sunday morning and perhaps my mind is not yet in a "hey, let's > > > look at VFS kernel code!" mindset, but I'm not convinced the problem > > > is the 'param->string = NULL' assignment in the LSM hooks. In both > > > the case of SELinux and Smack that code ends up returning either a 0 > > > (Smack) or a 1 (SELinux) - that's a little odd in it's own way, but I > > > don't believe it is relevant here - either way these return values are > > > not equal to -ENOPARAM so we should end up returning early from > > > vfs_parse_fs_param before it calls down into legacy_parse_param(): > > > > > > Taken from https://elixir.bootlin.com/linux/latest/source/fs/fs_context.c#L109 : > > > > > > ret = security_fs_context_parse_param(fc, param); > > > if (ret != -ENOPARAM) > > > /* Param belongs to the LSM or is disallowed by the LSM; so > > > * don't pass to the FS. > > > */ > > > return ret; > > > > > > if (fc->ops->parse_param) { > > > ret = fc->ops->parse_param(fc, param); > > > if (ret != -ENOPARAM) > > > return ret; > > > } > > > > Hi Paul, > > > > You are right. > > I almost connected the dots, but not exactly. > > Now that I read more code around, setting "param->string = NULL" in > > smack_fs_context_parse_param() looks correct to me (the fs copies and > > takes ownership of the string). > > > > I don't see how the crash happened... > > FWIW, I poked around a bit too and couldn't see anything obvious > either, but I can't pretend to know as much about the VFS layer as the > VFS folks. Hopefully they might have better luck. I'm not sure that's right. If the smack hook runs first, it will set param->string = NULL now the selinux hook runs. But the selinux param hook doesn't end up in selinux_add_opt() instead it will fail before opt = fs_parse(fc, selinux_fs_parameters, param, &result); which will return -ENOPARAM since it's not a selinux option subsequently causing the crash. Does that sound plausible? Christian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-08-27 15:30 ` Christian Brauner @ 2021-08-27 15:40 ` Casey Schaufler 2021-08-27 16:26 ` Christian Brauner 0 siblings, 1 reply; 17+ messages in thread From: Casey Schaufler @ 2021-08-27 15:40 UTC (permalink / raw) To: Christian Brauner, Paul Moore Cc: Dmitry Vyukov, syzbot, linux-security-module, Stephen Smalley, selinux, David Howells, linux-fsdevel, linux-kernel, syzkaller-bugs, viro, Casey Schaufler On 8/27/2021 8:30 AM, Christian Brauner wrote: > On Tue, Jul 06, 2021 at 08:50:44AM -0400, Paul Moore wrote: >> On Mon, Jul 5, 2021 at 1:52 AM Dmitry Vyukov <dvyukov@google.com> wrote: >>> On Sun, Jul 4, 2021 at 4:14 PM Paul Moore <paul@paul-moore.com> wrote: >>>> On Sat, Jul 3, 2021 at 6:16 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >>>>> On 7/2/2021 10:51 PM, Dmitry Vyukov wrote: >>>>>> On Sat, Jul 3, 2021 at 7:41 AM syzbot >>>>>> <syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com> wrote: >>>>>>> Hello, >>>>>>> >>>>>>> syzbot found the following issue on: >>>>>>> >>>>>>> HEAD commit: 62fb9874 Linux 5.13 >>>>>>> git tree: upstream >>>>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=12ffa118300000 >>>>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=19404adbea015a58 >>>>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 >>>>>>> compiler: Debian clang version 11.0.1-2 >>>>>>> >>>>>>> Unfortunately, I don't have any reproducer for this issue yet. >>>>>>> >>>>>>> IMPORTANT: if you fix the issue, please add the following tag to the commit: >>>>>>> Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com >>>>>> +Casey for what looks like a smackfs issue >>>>> This is from the new mount infrastructure introduced by >>>>> David Howells in November 2018. It makes sense that there >>>>> may be a problem in SELinux as well, as the code was introduced >>>>> by the same developer at the same time for the same purpose. >>>>> >>>>>> The crash was triggered by this test case: >>>>>> >>>>>> 21:55:33 executing program 1: >>>>>> r0 = fsopen(&(0x7f0000000040)='ext3\x00', 0x1) >>>>>> fsconfig$FSCONFIG_SET_STRING(r0, 0x1, &(0x7f00000002c0)='smackfsroot', >>>>>> &(0x7f0000000300)='default_permissions', 0x0) >>>>>> >>>>>> And I think the issue is in smack_fs_context_parse_param(): >>>>>> https://elixir.bootlin.com/linux/latest/source/security/smack/smack_lsm.c#L691 >>>>>> >>>>>> But it seems that selinux_fs_context_parse_param() contains the same issue: >>>>>> https://elixir.bootlin.com/linux/latest/source/security/selinux/hooks.c#L2919 >>>>>> +So selinux maintainers as well. >>>>>> >>>>>>> general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN >>>>>>> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] >>>>>>> CPU: 0 PID: 20300 Comm: syz-executor.1 Not tainted 5.13.0-syzkaller #0 >>>>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 >>>>>>> RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054 >>>>>>> Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db >>>>>>> RSP: 0018:ffffc90001dafd00 EFLAGS: 00010246 >>>>>>> RAX: 0000000000000000 RBX: 0000000000000013 RCX: dffffc0000000000 >>>>>>> RDX: 0000000000000013 RSI: 000000000000002c RDI: 0000000000000000 >>>>>>> RBP: 0000000000000000 R08: ffffffff81e171bf R09: ffffffff81e16f95 >>>>>>> R10: 0000000000000002 R11: ffff88807e96b880 R12: dffffc0000000000 >>>>>>> R13: ffff888020894000 R14: 0000000000000000 R15: 000000000000002c >>>>>>> FS: 00007fe01ae27700(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000 >>>>>>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >>>>>>> CR2: 00000000005645a8 CR3: 0000000018afc000 CR4: 00000000001506f0 >>>>>>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 >>>>>>> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 >>>>>>> Call Trace: >>>>>>> legacy_parse_param+0x461/0x7e0 fs/fs_context.c:537 >>>>>>> vfs_parse_fs_param+0x1e5/0x460 fs/fs_context.c:117 >>>> It's Sunday morning and perhaps my mind is not yet in a "hey, let's >>>> look at VFS kernel code!" mindset, but I'm not convinced the problem >>>> is the 'param->string = NULL' assignment in the LSM hooks. In both >>>> the case of SELinux and Smack that code ends up returning either a 0 >>>> (Smack) or a 1 (SELinux) - that's a little odd in it's own way, but I >>>> don't believe it is relevant here - either way these return values are >>>> not equal to -ENOPARAM so we should end up returning early from >>>> vfs_parse_fs_param before it calls down into legacy_parse_param(): >>>> >>>> Taken from https://elixir.bootlin.com/linux/latest/source/fs/fs_context.c#L109 : >>>> >>>> ret = security_fs_context_parse_param(fc, param); >>>> if (ret != -ENOPARAM) >>>> /* Param belongs to the LSM or is disallowed by the LSM; so >>>> * don't pass to the FS. >>>> */ >>>> return ret; >>>> >>>> if (fc->ops->parse_param) { >>>> ret = fc->ops->parse_param(fc, param); >>>> if (ret != -ENOPARAM) >>>> return ret; >>>> } >>> Hi Paul, >>> >>> You are right. >>> I almost connected the dots, but not exactly. >>> Now that I read more code around, setting "param->string = NULL" in >>> smack_fs_context_parse_param() looks correct to me (the fs copies and >>> takes ownership of the string). >>> >>> I don't see how the crash happened... >> FWIW, I poked around a bit too and couldn't see anything obvious >> either, but I can't pretend to know as much about the VFS layer as the >> VFS folks. Hopefully they might have better luck. > I'm not sure that's right. > If the smack hook runs first, it will set > > param->string = NULL > > now the selinux hook runs. But the selinux param hook doesn't end up in > selinux_add_opt() instead it will fail before > opt = fs_parse(fc, selinux_fs_parameters, param, &result); > which will return -ENOPARAM since it's not a selinux option subsequently > causing the crash. > > Does that sound plausible? No. You can't (currently) have both Smack and SELinux enabled at the same time. If you're invoking both the Smack hook and the SELinux hook you're doing somthing way wrong. > > Christian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-08-27 15:40 ` Casey Schaufler @ 2021-08-27 16:26 ` Christian Brauner 0 siblings, 0 replies; 17+ messages in thread From: Christian Brauner @ 2021-08-27 16:26 UTC (permalink / raw) To: Casey Schaufler Cc: Paul Moore, Dmitry Vyukov, syzbot, linux-security-module, Stephen Smalley, selinux, David Howells, linux-fsdevel, linux-kernel, syzkaller-bugs, viro On Fri, Aug 27, 2021 at 08:40:15AM -0700, Casey Schaufler wrote: > On 8/27/2021 8:30 AM, Christian Brauner wrote: > > On Tue, Jul 06, 2021 at 08:50:44AM -0400, Paul Moore wrote: > >> On Mon, Jul 5, 2021 at 1:52 AM Dmitry Vyukov <dvyukov@google.com> wrote: > >>> On Sun, Jul 4, 2021 at 4:14 PM Paul Moore <paul@paul-moore.com> wrote: > >>>> On Sat, Jul 3, 2021 at 6:16 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > >>>>> On 7/2/2021 10:51 PM, Dmitry Vyukov wrote: > >>>>>> On Sat, Jul 3, 2021 at 7:41 AM syzbot > >>>>>> <syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com> wrote: > >>>>>>> Hello, > >>>>>>> > >>>>>>> syzbot found the following issue on: > >>>>>>> > >>>>>>> HEAD commit: 62fb9874 Linux 5.13 > >>>>>>> git tree: upstream > >>>>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=12ffa118300000 > >>>>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=19404adbea015a58 > >>>>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 > >>>>>>> compiler: Debian clang version 11.0.1-2 > >>>>>>> > >>>>>>> Unfortunately, I don't have any reproducer for this issue yet. > >>>>>>> > >>>>>>> IMPORTANT: if you fix the issue, please add the following tag to the commit: > >>>>>>> Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com > >>>>>> +Casey for what looks like a smackfs issue > >>>>> This is from the new mount infrastructure introduced by > >>>>> David Howells in November 2018. It makes sense that there > >>>>> may be a problem in SELinux as well, as the code was introduced > >>>>> by the same developer at the same time for the same purpose. > >>>>> > >>>>>> The crash was triggered by this test case: > >>>>>> > >>>>>> 21:55:33 executing program 1: > >>>>>> r0 = fsopen(&(0x7f0000000040)='ext3\x00', 0x1) > >>>>>> fsconfig$FSCONFIG_SET_STRING(r0, 0x1, &(0x7f00000002c0)='smackfsroot', > >>>>>> &(0x7f0000000300)='default_permissions', 0x0) > >>>>>> > >>>>>> And I think the issue is in smack_fs_context_parse_param(): > >>>>>> https://elixir.bootlin.com/linux/latest/source/security/smack/smack_lsm.c#L691 > >>>>>> > >>>>>> But it seems that selinux_fs_context_parse_param() contains the same issue: > >>>>>> https://elixir.bootlin.com/linux/latest/source/security/selinux/hooks.c#L2919 > >>>>>> +So selinux maintainers as well. > >>>>>> > >>>>>>> general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN > >>>>>>> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] > >>>>>>> CPU: 0 PID: 20300 Comm: syz-executor.1 Not tainted 5.13.0-syzkaller #0 > >>>>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > >>>>>>> RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054 > >>>>>>> Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db > >>>>>>> RSP: 0018:ffffc90001dafd00 EFLAGS: 00010246 > >>>>>>> RAX: 0000000000000000 RBX: 0000000000000013 RCX: dffffc0000000000 > >>>>>>> RDX: 0000000000000013 RSI: 000000000000002c RDI: 0000000000000000 > >>>>>>> RBP: 0000000000000000 R08: ffffffff81e171bf R09: ffffffff81e16f95 > >>>>>>> R10: 0000000000000002 R11: ffff88807e96b880 R12: dffffc0000000000 > >>>>>>> R13: ffff888020894000 R14: 0000000000000000 R15: 000000000000002c > >>>>>>> FS: 00007fe01ae27700(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000 > >>>>>>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > >>>>>>> CR2: 00000000005645a8 CR3: 0000000018afc000 CR4: 00000000001506f0 > >>>>>>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > >>>>>>> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > >>>>>>> Call Trace: > >>>>>>> legacy_parse_param+0x461/0x7e0 fs/fs_context.c:537 > >>>>>>> vfs_parse_fs_param+0x1e5/0x460 fs/fs_context.c:117 > >>>> It's Sunday morning and perhaps my mind is not yet in a "hey, let's > >>>> look at VFS kernel code!" mindset, but I'm not convinced the problem > >>>> is the 'param->string = NULL' assignment in the LSM hooks. In both > >>>> the case of SELinux and Smack that code ends up returning either a 0 > >>>> (Smack) or a 1 (SELinux) - that's a little odd in it's own way, but I > >>>> don't believe it is relevant here - either way these return values are > >>>> not equal to -ENOPARAM so we should end up returning early from > >>>> vfs_parse_fs_param before it calls down into legacy_parse_param(): > >>>> > >>>> Taken from https://elixir.bootlin.com/linux/latest/source/fs/fs_context.c#L109 : > >>>> > >>>> ret = security_fs_context_parse_param(fc, param); > >>>> if (ret != -ENOPARAM) > >>>> /* Param belongs to the LSM or is disallowed by the LSM; so > >>>> * don't pass to the FS. > >>>> */ > >>>> return ret; > >>>> > >>>> if (fc->ops->parse_param) { > >>>> ret = fc->ops->parse_param(fc, param); > >>>> if (ret != -ENOPARAM) > >>>> return ret; > >>>> } > >>> Hi Paul, > >>> > >>> You are right. > >>> I almost connected the dots, but not exactly. > >>> Now that I read more code around, setting "param->string = NULL" in > >>> smack_fs_context_parse_param() looks correct to me (the fs copies and > >>> takes ownership of the string). > >>> > >>> I don't see how the crash happened... > >> FWIW, I poked around a bit too and couldn't see anything obvious > >> either, but I can't pretend to know as much about the VFS layer as the > >> VFS folks. Hopefully they might have better luck. > > I'm not sure that's right. > > If the smack hook runs first, it will set > > > > param->string = NULL > > > > now the selinux hook runs. But the selinux param hook doesn't end up in > > selinux_add_opt() instead it will fail before > > opt = fs_parse(fc, selinux_fs_parameters, param, &result); > > which will return -ENOPARAM since it's not a selinux option subsequently > > causing the crash. > > > > Does that sound plausible? > > No. You can't (currently) have both Smack and SELinux enabled at Ah, I thought that already worked. :) I'm EOD here but I'll try to look closer tomorrow or after the weekend. Christian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-07-03 5:41 [syzbot] general protection fault in legacy_parse_param syzbot 2021-07-03 5:51 ` Dmitry Vyukov @ 2021-08-27 14:49 ` syzbot 2021-08-28 2:11 ` syzbot 2 siblings, 0 replies; 17+ messages in thread From: syzbot @ 2021-08-27 14:49 UTC (permalink / raw) To: casey, dhowells, dvyukov, linux-fsdevel, linux-kernel, linux-security-module, paul, selinux, stephen.smalley.work, syzkaller-bugs, tonymarislogistics, viro syzbot has found a reproducer for the following issue on: HEAD commit: 77dd11439b86 Merge tag 'drm-fixes-2021-08-27' of git://ano.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=10636bde300000 kernel config: https://syzkaller.appspot.com/x/.config?x=2fd902af77ff1e56 dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 compiler: Debian clang version 11.0.1-2, GNU ld (GNU Binutils for Debian) 2.35.1 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=126d084d300000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16216eb1300000 IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] CPU: 0 PID: 8435 Comm: syz-executor272 Not tainted 5.14.0-rc7-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054 Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db RSP: 0018:ffffc9000d9f7d08 EFLAGS: 00010246 RAX: 0000000000000000 RBX: 0000000000000001 RCX: ffff88801c1f3880 RDX: 0000000000000001 RSI: 000000000000002c RDI: 0000000000000000 RBP: 0000000000000000 R08: ffffffff81e3db46 R09: ffffffff81e3d8e2 R10: 0000000000000002 R11: ffff88801c1f3880 R12: dffffc0000000000 R13: 1ffff92001b3efcc R14: 0000000000000000 R15: 000000000000002c FS: 0000000000deb300(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000020000044 CR3: 0000000037173000 CR4: 00000000001506f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: legacy_parse_param+0x49b/0x810 fs/fs_context.c:555 vfs_parse_fs_param+0x1df/0x460 fs/fs_context.c:146 vfs_fsconfig_locked fs/fsopen.c:265 [inline] __do_sys_fsconfig fs/fsopen.c:439 [inline] __se_sys_fsconfig+0xba9/0xff0 fs/fsopen.c:314 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae RIP: 0033:0x43ee69 Code: 28 c3 e8 2a 14 00 00 66 2e 0f 1f 84 00 00 00 00 00 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 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48 RSP: 002b:00007ffc5e9e0b98 EFLAGS: 00000246 ORIG_RAX: 00000000000001af RAX: ffffffffffffffda RBX: 0000000000400488 RCX: 000000000043ee69 RDX: 0000000020000080 RSI: 0000000000000001 RDI: 0000000000000003 RBP: 0000000000402e50 R08: 0000000000000000 R09: 0000000000400488 R10: 00000000200000c0 R11: 0000000000000246 R12: 0000000000402ee0 R13: 0000000000000000 R14: 00000000004ac018 R15: 0000000000400488 Modules linked in: ---[ end trace 74baf661f3b47b0a ]--- RIP: 0010:memchr+0x2f/0x70 lib/string.c:1054 Code: 41 54 53 48 89 d3 41 89 f7 45 31 f6 49 bc 00 00 00 00 00 fc ff df 0f 1f 44 00 00 48 85 db 74 3b 48 89 fd 48 89 f8 48 c1 e8 03 <42> 0f b6 04 20 84 c0 75 0f 48 ff cb 48 8d 7d 01 44 38 7d 00 75 db RSP: 0018:ffffc9000d9f7d08 EFLAGS: 00010246 RAX: 0000000000000000 RBX: 0000000000000001 RCX: ffff88801c1f3880 RDX: 0000000000000001 RSI: 000000000000002c RDI: 0000000000000000 RBP: 0000000000000000 R08: ffffffff81e3db46 R09: ffffffff81e3d8e2 R10: 0000000000000002 R11: ffff88801c1f3880 R12: dffffc0000000000 R13: 1ffff92001b3efcc R14: 0000000000000000 R15: 000000000000002c FS: 0000000000deb300(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fed5f8146c0 CR3: 0000000037173000 CR4: 00000000001506e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 ---------------- Code disassembly (best guess): 0: 41 54 push %r12 2: 53 push %rbx 3: 48 89 d3 mov %rdx,%rbx 6: 41 89 f7 mov %esi,%r15d 9: 45 31 f6 xor %r14d,%r14d c: 49 bc 00 00 00 00 00 movabs $0xdffffc0000000000,%r12 13: fc ff df 16: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) 1b: 48 85 db test %rbx,%rbx 1e: 74 3b je 0x5b 20: 48 89 fd mov %rdi,%rbp 23: 48 89 f8 mov %rdi,%rax 26: 48 c1 e8 03 shr $0x3,%rax * 2a: 42 0f b6 04 20 movzbl (%rax,%r12,1),%eax <-- trapping instruction 2f: 84 c0 test %al,%al 31: 75 0f jne 0x42 33: 48 ff cb dec %rbx 36: 48 8d 7d 01 lea 0x1(%rbp),%rdi 3a: 44 38 7d 00 cmp %r15b,0x0(%rbp) 3e: 75 db jne 0x1b ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-07-03 5:41 [syzbot] general protection fault in legacy_parse_param syzbot 2021-07-03 5:51 ` Dmitry Vyukov 2021-08-27 14:49 ` syzbot @ 2021-08-28 2:11 ` syzbot 2021-08-30 12:23 ` Christian Brauner 2 siblings, 1 reply; 17+ messages in thread From: syzbot @ 2021-08-28 2:11 UTC (permalink / raw) To: andriin, ast, bpf, casey, christian.brauner, daniel, dhowells, dvyukov, jmorris, kafai, kpsingh, linux-fsdevel, linux-kernel, linux-security-module, netdev, paul, selinux, songliubraving, stephen.smalley.work, syzkaller-bugs, tonymarislogistics, viro, yhs syzbot has bisected this issue to: commit 54261af473be4c5481f6196064445d2945f2bdab Author: KP Singh <kpsingh@google.com> Date: Thu Apr 30 15:52:40 2020 +0000 security: Fix the default value of fs_context_parse_param hook bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=160c5d75300000 start commit: 77dd11439b86 Merge tag 'drm-fixes-2021-08-27' of git://ano.. git tree: upstream final oops: https://syzkaller.appspot.com/x/report.txt?x=150c5d75300000 console output: https://syzkaller.appspot.com/x/log.txt?x=110c5d75300000 kernel config: https://syzkaller.appspot.com/x/.config?x=2fd902af77ff1e56 dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=126d084d300000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16216eb1300000 Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com Fixes: 54261af473be ("security: Fix the default value of fs_context_parse_param hook") For information about bisection process see: https://goo.gl/tpsmEJ#bisection ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-08-28 2:11 ` syzbot @ 2021-08-30 12:23 ` Christian Brauner 2021-08-30 14:25 ` Casey Schaufler 0 siblings, 1 reply; 17+ messages in thread From: Christian Brauner @ 2021-08-30 12:23 UTC (permalink / raw) To: syzbot Cc: andriin, ast, bpf, casey, daniel, dhowells, dvyukov, jmorris, kafai, kpsingh, linux-fsdevel, linux-kernel, linux-security-module, netdev, paul, selinux, songliubraving, stephen.smalley.work, syzkaller-bugs, tonymarislogistics, viro, yhs On Fri, Aug 27, 2021 at 07:11:18PM -0700, syzbot wrote: > syzbot has bisected this issue to: > > commit 54261af473be4c5481f6196064445d2945f2bdab > Author: KP Singh <kpsingh@google.com> > Date: Thu Apr 30 15:52:40 2020 +0000 > > security: Fix the default value of fs_context_parse_param hook > > bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=160c5d75300000 > start commit: 77dd11439b86 Merge tag 'drm-fixes-2021-08-27' of git://ano.. > git tree: upstream > final oops: https://syzkaller.appspot.com/x/report.txt?x=150c5d75300000 > console output: https://syzkaller.appspot.com/x/log.txt?x=110c5d75300000 > kernel config: https://syzkaller.appspot.com/x/.config?x=2fd902af77ff1e56 > dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=126d084d300000 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16216eb1300000 > > Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com > Fixes: 54261af473be ("security: Fix the default value of fs_context_parse_param hook") > > For information about bisection process see: https://goo.gl/tpsmEJ#bisection So ok, this seems somewhat clear now. When smack and CONFIG_BPF_LSM=y is selected the bpf LSM will register NOP handlers including bpf_lsm_fs_context_fs_param() for the fs_context_fs_param LSM hook. The bpf LSM runs last, i.e. after smack according to: CONFIG_LSM="landlock,lockdown,yama,safesetid,integrity,tomoyo,smack,bpf" in the appended config. The smack hook runs and sets param->string = NULL then the bpf NOP handler runs returning -ENOPARM indicating to the vfs parameter parser that this is not a security module option so it should proceed processing the parameter subsequently causing the crash because param->string is not allowed to be NULL (Which the vfs parameter parser verifies early in fsconfig().). If you take the appended syzkaller config and additionally select kprobes you can observe this by registering bpf kretprobes for: security_fs_context_parse_param() smack_fs_context_parse_param() bpf_lsm_fs_context_parse_param() in different terminal windows and then running the syzkaller provided reproducer: root@f2-vm:~# bpftrace -e 'kretprobe:smack_fs_context_parse_param { printf("returned: %d\n", retval); }' Attaching 1 probe... returned: 0 root@f2-vm:~# bpftrace -e 'kretprobe:bpf_lsm_fs_context_parse_param { printf("returned: %d\n", retval); }' Attaching 1 probe... returned: -519 root@f2-vm:~# bpftrace -e 'kretprobe:security_fs_context_parse_param { printf("returned: %d\n", retval); }' Attaching 1 probe... returned: -519 ^^^^^ This will ultimately tell the vfs to move on causing the crash because param->string is null at that point. Unless I missed something why that can't happen. Christian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-08-30 12:23 ` Christian Brauner @ 2021-08-30 14:25 ` Casey Schaufler 2021-08-30 16:40 ` Casey Schaufler 0 siblings, 1 reply; 17+ messages in thread From: Casey Schaufler @ 2021-08-30 14:25 UTC (permalink / raw) To: Christian Brauner, syzbot Cc: andriin, ast, bpf, daniel, dhowells, dvyukov, jmorris, kafai, kpsingh, linux-fsdevel, linux-kernel, linux-security-module, netdev, paul, selinux, songliubraving, stephen.smalley.work, syzkaller-bugs, tonymarislogistics, viro, yhs, Casey Schaufler On 8/30/2021 5:23 AM, Christian Brauner wrote: > On Fri, Aug 27, 2021 at 07:11:18PM -0700, syzbot wrote: >> syzbot has bisected this issue to: >> >> commit 54261af473be4c5481f6196064445d2945f2bdab >> Author: KP Singh <kpsingh@google.com> >> Date: Thu Apr 30 15:52:40 2020 +0000 >> >> security: Fix the default value of fs_context_parse_param hook >> >> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=160c5d75300000 >> start commit: 77dd11439b86 Merge tag 'drm-fixes-2021-08-27' of git://ano.. >> git tree: upstream >> final oops: https://syzkaller.appspot.com/x/report.txt?x=150c5d75300000 >> console output: https://syzkaller.appspot.com/x/log.txt?x=110c5d75300000 >> kernel config: https://syzkaller.appspot.com/x/.config?x=2fd902af77ff1e56 >> dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 >> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=126d084d300000 >> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16216eb1300000 >> >> Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com >> Fixes: 54261af473be ("security: Fix the default value of fs_context_parse_param hook") >> >> For information about bisection process see: https://goo.gl/tpsmEJ#bisection > So ok, this seems somewhat clear now. When smack and > CONFIG_BPF_LSM=y > is selected the bpf LSM will register NOP handlers including > > bpf_lsm_fs_context_fs_param() > > for the > > fs_context_fs_param > > LSM hook. The bpf LSM runs last, i.e. after smack according to: > > CONFIG_LSM="landlock,lockdown,yama,safesetid,integrity,tomoyo,smack,bpf" > > in the appended config. The smack hook runs and sets > > param->string = NULL > > then the bpf NOP handler runs returning -ENOPARM indicating to the vfs > parameter parser that this is not a security module option so it should > proceed processing the parameter subsequently causing the crash because > param->string is not allowed to be NULL (Which the vfs parameter parser > verifies early in fsconfig().). The security_fs_context_parse_param() function is incorrectly implemented using the call_int_hook() macro. It should return zero if any of the modules return zero. It does not follow the usual failure model of LSM hooks. It could be argued that the code was fine before the addition of the BPF hook, but it was going to fail as soon as any two security modules provided mount options. Regardless, I will have a patch later today. Thank you for tracking this down. > > If you take the appended syzkaller config and additionally select > kprobes you can observe this by registering bpf kretprobes for: > security_fs_context_parse_param() > smack_fs_context_parse_param() > bpf_lsm_fs_context_parse_param() > in different terminal windows and then running the syzkaller provided > reproducer: > > root@f2-vm:~# bpftrace -e 'kretprobe:smack_fs_context_parse_param { printf("returned: %d\n", retval); }' > Attaching 1 probe... > returned: 0 > > root@f2-vm:~# bpftrace -e 'kretprobe:bpf_lsm_fs_context_parse_param { printf("returned: %d\n", retval); }' > Attaching 1 probe... > returned: -519 > > root@f2-vm:~# bpftrace -e 'kretprobe:security_fs_context_parse_param { printf("returned: %d\n", retval); }' > Attaching 1 probe... > returned: -519 > > ^^^^^ > This will ultimately tell the vfs to move on causing the crash because > param->string is null at that point. > > Unless I missed something why that can't happen. > > Christian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-08-30 14:25 ` Casey Schaufler @ 2021-08-30 16:40 ` Casey Schaufler 2021-08-30 16:57 ` Christian Brauner 0 siblings, 1 reply; 17+ messages in thread From: Casey Schaufler @ 2021-08-30 16:40 UTC (permalink / raw) To: Christian Brauner, syzbot Cc: andriin, ast, bpf, daniel, dhowells, dvyukov, jmorris, kafai, kpsingh, linux-fsdevel, linux-kernel, linux-security-module, netdev, paul, selinux, songliubraving, stephen.smalley.work, syzkaller-bugs, tonymarislogistics, viro, yhs, Casey Schaufler On 8/30/2021 7:25 AM, Casey Schaufler wrote: > On 8/30/2021 5:23 AM, Christian Brauner wrote: >> On Fri, Aug 27, 2021 at 07:11:18PM -0700, syzbot wrote: >>> syzbot has bisected this issue to: >>> >>> commit 54261af473be4c5481f6196064445d2945f2bdab >>> Author: KP Singh <kpsingh@google.com> >>> Date: Thu Apr 30 15:52:40 2020 +0000 >>> >>> security: Fix the default value of fs_context_parse_param hook >>> >>> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=160c5d75300000 >>> start commit: 77dd11439b86 Merge tag 'drm-fixes-2021-08-27' of git://ano.. >>> git tree: upstream >>> final oops: https://syzkaller.appspot.com/x/report.txt?x=150c5d75300000 >>> console output: https://syzkaller.appspot.com/x/log.txt?x=110c5d75300000 >>> kernel config: https://syzkaller.appspot.com/x/.config?x=2fd902af77ff1e56 >>> dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 >>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=126d084d300000 >>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16216eb1300000 >>> >>> Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com >>> Fixes: 54261af473be ("security: Fix the default value of fs_context_parse_param hook") >>> >>> For information about bisection process see: https://goo.gl/tpsmEJ#bisection >> So ok, this seems somewhat clear now. When smack and >> CONFIG_BPF_LSM=y >> is selected the bpf LSM will register NOP handlers including >> >> bpf_lsm_fs_context_fs_param() >> >> for the >> >> fs_context_fs_param >> >> LSM hook. The bpf LSM runs last, i.e. after smack according to: >> >> CONFIG_LSM="landlock,lockdown,yama,safesetid,integrity,tomoyo,smack,bpf" >> >> in the appended config. The smack hook runs and sets >> >> param->string = NULL >> >> then the bpf NOP handler runs returning -ENOPARM indicating to the vfs >> parameter parser that this is not a security module option so it should >> proceed processing the parameter subsequently causing the crash because >> param->string is not allowed to be NULL (Which the vfs parameter parser >> verifies early in fsconfig().). > The security_fs_context_parse_param() function is incorrectly > implemented using the call_int_hook() macro. It should return > zero if any of the modules return zero. It does not follow the > usual failure model of LSM hooks. It could be argued that the > code was fine before the addition of the BPF hook, but it was > going to fail as soon as any two security modules provided > mount options. > > Regardless, I will have a patch later today. Thank you for > tracking this down. Here's my proposed patch. I'll tidy it up with a proper commit message if it looks alright to y'all. I've tested with Smack and with and without BPF. security/security.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/security/security.c b/security/security.c index 09533cbb7221..3cf0faaf1c5b 100644 --- a/security/security.c +++ b/security/security.c @@ -885,7 +885,19 @@ int security_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc) int security_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *param) { - return call_int_hook(fs_context_parse_param, -ENOPARAM, fc, param); + struct security_hook_list *hp; + int trc; + int rc = -ENOPARAM; + + hlist_for_each_entry(hp, &security_hook_heads.fs_context_parse_param, + list) { + trc = hp->hook.fs_context_parse_param(fc, param); + if (trc == 0) + rc = 0; + else if (trc != -ENOPARAM) + return trc; + } + return rc; } int security_sb_alloc(struct super_block *sb) > >> If you take the appended syzkaller config and additionally select >> kprobes you can observe this by registering bpf kretprobes for: >> security_fs_context_parse_param() >> smack_fs_context_parse_param() >> bpf_lsm_fs_context_parse_param() >> in different terminal windows and then running the syzkaller provided >> reproducer: >> >> root@f2-vm:~# bpftrace -e 'kretprobe:smack_fs_context_parse_param { printf("returned: %d\n", retval); }' >> Attaching 1 probe... >> returned: 0 >> >> root@f2-vm:~# bpftrace -e 'kretprobe:bpf_lsm_fs_context_parse_param { printf("returned: %d\n", retval); }' >> Attaching 1 probe... >> returned: -519 >> >> root@f2-vm:~# bpftrace -e 'kretprobe:security_fs_context_parse_param { printf("returned: %d\n", retval); }' >> Attaching 1 probe... >> returned: -519 >> >> ^^^^^ >> This will ultimately tell the vfs to move on causing the crash because >> param->string is null at that point. >> >> Unless I missed something why that can't happen. >> >> Christian ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-08-30 16:40 ` Casey Schaufler @ 2021-08-30 16:57 ` Christian Brauner 2021-08-30 17:41 ` Casey Schaufler 0 siblings, 1 reply; 17+ messages in thread From: Christian Brauner @ 2021-08-30 16:57 UTC (permalink / raw) To: Casey Schaufler Cc: syzbot, andriin, ast, bpf, daniel, dhowells, dvyukov, jmorris, kafai, kpsingh, linux-fsdevel, linux-kernel, linux-security-module, netdev, paul, selinux, songliubraving, stephen.smalley.work, syzkaller-bugs, tonymarislogistics, viro, yhs On Mon, Aug 30, 2021 at 09:40:57AM -0700, Casey Schaufler wrote: > On 8/30/2021 7:25 AM, Casey Schaufler wrote: > > On 8/30/2021 5:23 AM, Christian Brauner wrote: > >> On Fri, Aug 27, 2021 at 07:11:18PM -0700, syzbot wrote: > >>> syzbot has bisected this issue to: > >>> > >>> commit 54261af473be4c5481f6196064445d2945f2bdab > >>> Author: KP Singh <kpsingh@google.com> > >>> Date: Thu Apr 30 15:52:40 2020 +0000 > >>> > >>> security: Fix the default value of fs_context_parse_param hook > >>> > >>> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=160c5d75300000 > >>> start commit: 77dd11439b86 Merge tag 'drm-fixes-2021-08-27' of git://ano.. > >>> git tree: upstream > >>> final oops: https://syzkaller.appspot.com/x/report.txt?x=150c5d75300000 > >>> console output: https://syzkaller.appspot.com/x/log.txt?x=110c5d75300000 > >>> kernel config: https://syzkaller.appspot.com/x/.config?x=2fd902af77ff1e56 > >>> dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 > >>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=126d084d300000 > >>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16216eb1300000 > >>> > >>> Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com > >>> Fixes: 54261af473be ("security: Fix the default value of fs_context_parse_param hook") > >>> > >>> For information about bisection process see: https://goo.gl/tpsmEJ#bisection > >> So ok, this seems somewhat clear now. When smack and > >> CONFIG_BPF_LSM=y > >> is selected the bpf LSM will register NOP handlers including > >> > >> bpf_lsm_fs_context_fs_param() > >> > >> for the > >> > >> fs_context_fs_param > >> > >> LSM hook. The bpf LSM runs last, i.e. after smack according to: > >> > >> CONFIG_LSM="landlock,lockdown,yama,safesetid,integrity,tomoyo,smack,bpf" > >> > >> in the appended config. The smack hook runs and sets > >> > >> param->string = NULL > >> > >> then the bpf NOP handler runs returning -ENOPARM indicating to the vfs > >> parameter parser that this is not a security module option so it should > >> proceed processing the parameter subsequently causing the crash because > >> param->string is not allowed to be NULL (Which the vfs parameter parser > >> verifies early in fsconfig().). > > The security_fs_context_parse_param() function is incorrectly > > implemented using the call_int_hook() macro. It should return > > zero if any of the modules return zero. It does not follow the > > usual failure model of LSM hooks. It could be argued that the > > code was fine before the addition of the BPF hook, but it was > > going to fail as soon as any two security modules provided > > mount options. > > > > Regardless, I will have a patch later today. Thank you for > > tracking this down. > > Here's my proposed patch. I'll tidy it up with a proper > commit message if it looks alright to y'all. I've tested > with Smack and with and without BPF. Looks good to me. On question, in contrast to smack, selinux returns 1 instead of 0 on success. So selinux would cause an early return preventing other hooks from running. Just making sure that this is intentional. Iirc, this would mean that selinux causes fsconfig() to return a positive value to userspace which I think is a bug; likely in selinux. So I think selinux should either return 0 or the security hook itself needs to overwrite a positive value with a sensible errno that can be seen by userspace. > > > security/security.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/security/security.c b/security/security.c > index 09533cbb7221..3cf0faaf1c5b 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -885,7 +885,19 @@ int security_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc) > > int security_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *param) > { > - return call_int_hook(fs_context_parse_param, -ENOPARAM, fc, param); > + struct security_hook_list *hp; > + int trc; > + int rc = -ENOPARAM; > + > + hlist_for_each_entry(hp, &security_hook_heads.fs_context_parse_param, > + list) { > + trc = hp->hook.fs_context_parse_param(fc, param); > + if (trc == 0) > + rc = 0; > + else if (trc != -ENOPARAM) > + return trc; > + } > + return rc; > } > > int security_sb_alloc(struct super_block *sb) <snip> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-08-30 16:57 ` Christian Brauner @ 2021-08-30 17:41 ` Casey Schaufler 2021-08-31 7:38 ` Christian Brauner 0 siblings, 1 reply; 17+ messages in thread From: Casey Schaufler @ 2021-08-30 17:41 UTC (permalink / raw) To: Christian Brauner, David Howells Cc: syzbot, andriin, ast, bpf, daniel, dvyukov, jmorris, kafai, kpsingh, linux-fsdevel, linux-kernel, linux-security-module, netdev, paul, selinux, songliubraving, stephen.smalley.work, syzkaller-bugs, tonymarislogistics, viro, yhs On 8/30/2021 9:57 AM, Christian Brauner wrote: > On Mon, Aug 30, 2021 at 09:40:57AM -0700, Casey Schaufler wrote: >> On 8/30/2021 7:25 AM, Casey Schaufler wrote: >>> On 8/30/2021 5:23 AM, Christian Brauner wrote: >>>> On Fri, Aug 27, 2021 at 07:11:18PM -0700, syzbot wrote: >>>>> syzbot has bisected this issue to: >>>>> >>>>> commit 54261af473be4c5481f6196064445d2945f2bdab >>>>> Author: KP Singh <kpsingh@google.com> >>>>> Date: Thu Apr 30 15:52:40 2020 +0000 >>>>> >>>>> security: Fix the default value of fs_context_parse_param hook >>>>> >>>>> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=160c5d75300000 >>>>> start commit: 77dd11439b86 Merge tag 'drm-fixes-2021-08-27' of git://ano.. >>>>> git tree: upstream >>>>> final oops: https://syzkaller.appspot.com/x/report.txt?x=150c5d75300000 >>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=110c5d75300000 >>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=2fd902af77ff1e56 >>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 >>>>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=126d084d300000 >>>>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16216eb1300000 >>>>> >>>>> Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com >>>>> Fixes: 54261af473be ("security: Fix the default value of fs_context_parse_param hook") >>>>> >>>>> For information about bisection process see: https://goo.gl/tpsmEJ#bisection >>>> So ok, this seems somewhat clear now. When smack and >>>> CONFIG_BPF_LSM=y >>>> is selected the bpf LSM will register NOP handlers including >>>> >>>> bpf_lsm_fs_context_fs_param() >>>> >>>> for the >>>> >>>> fs_context_fs_param >>>> >>>> LSM hook. The bpf LSM runs last, i.e. after smack according to: >>>> >>>> CONFIG_LSM="landlock,lockdown,yama,safesetid,integrity,tomoyo,smack,bpf" >>>> >>>> in the appended config. The smack hook runs and sets >>>> >>>> param->string = NULL >>>> >>>> then the bpf NOP handler runs returning -ENOPARM indicating to the vfs >>>> parameter parser that this is not a security module option so it should >>>> proceed processing the parameter subsequently causing the crash because >>>> param->string is not allowed to be NULL (Which the vfs parameter parser >>>> verifies early in fsconfig().). >>> The security_fs_context_parse_param() function is incorrectly >>> implemented using the call_int_hook() macro. It should return >>> zero if any of the modules return zero. It does not follow the >>> usual failure model of LSM hooks. It could be argued that the >>> code was fine before the addition of the BPF hook, but it was >>> going to fail as soon as any two security modules provided >>> mount options. >>> >>> Regardless, I will have a patch later today. Thank you for >>> tracking this down. >> Here's my proposed patch. I'll tidy it up with a proper >> commit message if it looks alright to y'all. I've tested >> with Smack and with and without BPF. > Looks good to me. > On question, in contrast to smack, selinux returns 1 instead of 0 on > success. So selinux would cause an early return preventing other hooks > from running. Just making sure that this is intentional. > > Iirc, this would mean that selinux causes fsconfig() to return a > positive value to userspace which I think is a bug; likely in selinux. > So I think selinux should either return 0 or the security hook itself > needs to overwrite a positive value with a sensible errno that can be > seen by userspace. I think that I agree. The SELinux and Smack versions of the hook are almost identical except for setting rc to 1 in the SELinux case. And returning 1 makes no sense if you follow the callers back. David Howells wrote both the SELinux and Smack versions. David - why are they different? which is correct? > >> >> security/security.c | 14 +++++++++++++- >> 1 file changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/security/security.c b/security/security.c >> index 09533cbb7221..3cf0faaf1c5b 100644 >> --- a/security/security.c >> +++ b/security/security.c >> @@ -885,7 +885,19 @@ int security_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc) >> >> int security_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *param) >> { >> - return call_int_hook(fs_context_parse_param, -ENOPARAM, fc, param); >> + struct security_hook_list *hp; >> + int trc; >> + int rc = -ENOPARAM; >> + >> + hlist_for_each_entry(hp, &security_hook_heads.fs_context_parse_param, >> + list) { >> + trc = hp->hook.fs_context_parse_param(fc, param); >> + if (trc == 0) >> + rc = 0; >> + else if (trc != -ENOPARAM) >> + return trc; >> + } >> + return rc; >> } >> >> int security_sb_alloc(struct super_block *sb) > <snip> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [syzbot] general protection fault in legacy_parse_param 2021-08-30 17:41 ` Casey Schaufler @ 2021-08-31 7:38 ` Christian Brauner 0 siblings, 0 replies; 17+ messages in thread From: Christian Brauner @ 2021-08-31 7:38 UTC (permalink / raw) To: Casey Schaufler Cc: David Howells, syzbot, andriin, ast, bpf, daniel, dvyukov, jmorris, kafai, kpsingh, linux-fsdevel, linux-kernel, linux-security-module, netdev, paul, selinux, songliubraving, stephen.smalley.work, syzkaller-bugs, tonymarislogistics, viro, yhs On Mon, Aug 30, 2021 at 10:41:29AM -0700, Casey Schaufler wrote: > On 8/30/2021 9:57 AM, Christian Brauner wrote: > > On Mon, Aug 30, 2021 at 09:40:57AM -0700, Casey Schaufler wrote: > >> On 8/30/2021 7:25 AM, Casey Schaufler wrote: > >>> On 8/30/2021 5:23 AM, Christian Brauner wrote: > >>>> On Fri, Aug 27, 2021 at 07:11:18PM -0700, syzbot wrote: > >>>>> syzbot has bisected this issue to: > >>>>> > >>>>> commit 54261af473be4c5481f6196064445d2945f2bdab > >>>>> Author: KP Singh <kpsingh@google.com> > >>>>> Date: Thu Apr 30 15:52:40 2020 +0000 > >>>>> > >>>>> security: Fix the default value of fs_context_parse_param hook > >>>>> > >>>>> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=160c5d75300000 > >>>>> start commit: 77dd11439b86 Merge tag 'drm-fixes-2021-08-27' of git://ano.. > >>>>> git tree: upstream > >>>>> final oops: https://syzkaller.appspot.com/x/report.txt?x=150c5d75300000 > >>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=110c5d75300000 > >>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=2fd902af77ff1e56 > >>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=d1e3b1d92d25abf97943 > >>>>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=126d084d300000 > >>>>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16216eb1300000 > >>>>> > >>>>> Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com > >>>>> Fixes: 54261af473be ("security: Fix the default value of fs_context_parse_param hook") > >>>>> > >>>>> For information about bisection process see: https://goo.gl/tpsmEJ#bisection > >>>> So ok, this seems somewhat clear now. When smack and > >>>> CONFIG_BPF_LSM=y > >>>> is selected the bpf LSM will register NOP handlers including > >>>> > >>>> bpf_lsm_fs_context_fs_param() > >>>> > >>>> for the > >>>> > >>>> fs_context_fs_param > >>>> > >>>> LSM hook. The bpf LSM runs last, i.e. after smack according to: > >>>> > >>>> CONFIG_LSM="landlock,lockdown,yama,safesetid,integrity,tomoyo,smack,bpf" > >>>> > >>>> in the appended config. The smack hook runs and sets > >>>> > >>>> param->string = NULL > >>>> > >>>> then the bpf NOP handler runs returning -ENOPARM indicating to the vfs > >>>> parameter parser that this is not a security module option so it should > >>>> proceed processing the parameter subsequently causing the crash because > >>>> param->string is not allowed to be NULL (Which the vfs parameter parser > >>>> verifies early in fsconfig().). > >>> The security_fs_context_parse_param() function is incorrectly > >>> implemented using the call_int_hook() macro. It should return > >>> zero if any of the modules return zero. It does not follow the > >>> usual failure model of LSM hooks. It could be argued that the > >>> code was fine before the addition of the BPF hook, but it was > >>> going to fail as soon as any two security modules provided > >>> mount options. > >>> > >>> Regardless, I will have a patch later today. Thank you for > >>> tracking this down. > >> Here's my proposed patch. I'll tidy it up with a proper > >> commit message if it looks alright to y'all. I've tested > >> with Smack and with and without BPF. > > Looks good to me. > > On question, in contrast to smack, selinux returns 1 instead of 0 on > > success. So selinux would cause an early return preventing other hooks > > from running. Just making sure that this is intentional. > > > > Iirc, this would mean that selinux causes fsconfig() to return a > > positive value to userspace which I think is a bug; likely in selinux. > > So I think selinux should either return 0 or the security hook itself > > needs to overwrite a positive value with a sensible errno that can be > > seen by userspace. > > I think that I agree. The SELinux and Smack versions of the > hook are almost identical except for setting rc to 1 in the > SELinux case. And returning 1 makes no sense if you follow > the callers back. David Howells wrote both the SELinux and > Smack versions. David - why are they different? which is correct? The documentation for fs_context_parse_param notes: * @fs_context_parse_param: * Userspace provided a parameter to configure a superblock. The LSM may * reject it with an error and may use it for itself, in which case it * should return 0; otherwise it should return -ENOPARAM to pass it on to * the filesystem. * @fc indicates the filesystem context. * @param The parameter So we should simply make selinux return 0 on top of your patch when it has consumed the option. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-08-31 7:38 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-03 5:41 [syzbot] general protection fault in legacy_parse_param syzbot 2021-07-03 5:51 ` Dmitry Vyukov 2021-07-03 22:16 ` Casey Schaufler 2021-07-04 14:14 ` Paul Moore 2021-07-05 5:52 ` Dmitry Vyukov 2021-07-06 12:50 ` Paul Moore 2021-08-27 15:30 ` Christian Brauner 2021-08-27 15:40 ` Casey Schaufler 2021-08-27 16:26 ` Christian Brauner 2021-08-27 14:49 ` syzbot 2021-08-28 2:11 ` syzbot 2021-08-30 12:23 ` Christian Brauner 2021-08-30 14:25 ` Casey Schaufler 2021-08-30 16:40 ` Casey Schaufler 2021-08-30 16:57 ` Christian Brauner 2021-08-30 17:41 ` Casey Schaufler 2021-08-31 7:38 ` Christian Brauner
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).