Discussion:
KASAN: slab-out-of-bounds Read in strcmp
syzbot
2017-11-28 06:29:00 UTC
Permalink
Hello,

syzkaller hit the following crash on
dbf9f9124937139e6fcd3f6c64154a699408b481
git://git.cmpxchg.org/linux-mmots.git/master
compiler: gcc (GCC) 7.1.1 20170620
.config is attached
Raw console output is attached.

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


==================================================================
BUG: KASAN: slab-out-of-bounds in strcmp+0x96/0xb0 lib/string.c:328
Read of size 1 at addr ffff8801cc131681 by task syz-executor1/5346

CPU: 0 PID: 5346 Comm: syz-executor1 Not tainted 4.15.0-rc1-mm1+ #27
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:53
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x25b/0x340 mm/kasan/report.c:409
__asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:427
strcmp+0x96/0xb0 lib/string.c:328
security_context_to_sid_core+0x437/0x620
security/selinux/ss/services.c:1420
security_context_to_sid+0x32/0x40 security/selinux/ss/services.c:1479
selinux_setprocattr+0x51c/0xb50 security/selinux/hooks.c:5986
security_setprocattr+0x85/0xc0 security/security.c:1264
proc_pid_attr_write+0x1e6/0x280 fs/proc/base.c:2575
__vfs_write+0xef/0x970 fs/read_write.c:480
__kernel_write+0xfe/0x350 fs/read_write.c:501
write_pipe_buf+0x175/0x220 fs/splice.c:797
splice_from_pipe_feed fs/splice.c:502 [inline]
__splice_from_pipe+0x328/0x730 fs/splice.c:626
splice_from_pipe+0x1e9/0x330 fs/splice.c:661
default_file_splice_write+0x40/0x90 fs/splice.c:809
do_splice_from fs/splice.c:851 [inline]
direct_splice_actor+0x125/0x180 fs/splice.c:1018
splice_direct_to_actor+0x2c1/0x820 fs/splice.c:973
do_splice_direct+0x2a7/0x3d0 fs/splice.c:1061
do_sendfile+0x5d5/0xe90 fs/read_write.c:1413
SYSC_sendfile64 fs/read_write.c:1468 [inline]
SyS_sendfile64+0xbd/0x160 fs/read_write.c:1460
entry_SYSCALL_64_fastpath+0x1f/0x96
RIP: 0033:0x4529d9
RSP: 002b:00007fd50afcbc58 EFLAGS: 00000212 ORIG_RAX: 0000000000000028
RAX: ffffffffffffffda RBX: 0000000000758020 RCX: 00000000004529d9
RDX: 0000000020000000 RSI: 0000000000000013 RDI: 0000000000000013
RBP: 000000000000060b R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000212 R12: 00000000006f61a8
R13: 00000000ffffffff R14: 00007fd50afcc6d4 R15: 0000000000000000

Allocated by task 5346:
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
__do_kmalloc mm/slab.c:3712 [inline]
__kmalloc_track_caller+0x15e/0x760 mm/slab.c:3727
memdup_user+0x2c/0x90 mm/util.c:164
proc_pid_attr_write+0x115/0x280 fs/proc/base.c:2564
__vfs_write+0xef/0x970 fs/read_write.c:480
__kernel_write+0xfe/0x350 fs/read_write.c:501
write_pipe_buf+0x175/0x220 fs/splice.c:797
splice_from_pipe_feed fs/splice.c:502 [inline]
__splice_from_pipe+0x328/0x730 fs/splice.c:626
splice_from_pipe+0x1e9/0x330 fs/splice.c:661
default_file_splice_write+0x40/0x90 fs/splice.c:809
do_splice_from fs/splice.c:851 [inline]
direct_splice_actor+0x125/0x180 fs/splice.c:1018
splice_direct_to_actor+0x2c1/0x820 fs/splice.c:973
do_splice_direct+0x2a7/0x3d0 fs/splice.c:1061
do_sendfile+0x5d5/0xe90 fs/read_write.c:1413
SYSC_sendfile64 fs/read_write.c:1468 [inline]
SyS_sendfile64+0xbd/0x160 fs/read_write.c:1460
entry_SYSCALL_64_fastpath+0x1f/0x96

Freed by task 4101:
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_slab_free+0x71/0xc0 mm/kasan/kasan.c:524
__cache_free mm/slab.c:3492 [inline]
kfree+0xca/0x250 mm/slab.c:3807
selinux_cred_free+0x48/0x70 security/selinux/hooks.c:3815
security_cred_free+0x48/0x80 security/security.c:995
put_cred_rcu+0x106/0x400 kernel/cred.c:117
__rcu_reclaim kernel/rcu/rcu.h:195 [inline]
rcu_do_batch kernel/rcu/tree.c:2758 [inline]
invoke_rcu_callbacks kernel/rcu/tree.c:3012 [inline]
__rcu_process_callbacks kernel/rcu/tree.c:2979 [inline]
rcu_process_callbacks+0xd74/0x17d0 kernel/rcu/tree.c:2996
__do_softirq+0x29d/0xbb2 kernel/softirq.c:285

The buggy address belongs to the object at ffff8801cc131680
which belongs to the cache kmalloc-32 of size 32
The buggy address is located 1 bytes inside of
32-byte region [ffff8801cc131680, ffff8801cc1316a0)
The buggy address belongs to the page:
page:ffffea0007304c40 count:1 mapcount:0 mapping:ffff8801cc131000
index:0xffff8801cc131fc1
flags: 0x2fffc0000000100(slab)
raw: 02fffc0000000100 ffff8801cc131000 ffff8801cc131fc1 0000000100000035
raw: ffffea00073a96e0 ffffea0007420ce0 ffff8801db0001c0 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff8801cc131580: fb fb fb fb fc fc fc fc fb fb fb fb fc fc fc fc
ffff8801cc131600: fb fb fb fb fc fc fc fc 00 07 fc fc fc fc fc fc
ffff8801cc131680: 01 fc fc fc fc fc fc fc fb fb fb fb fc fc fc fc
^
ffff8801cc131700: fb fb fb fb fc fc fc fc 00 00 fc fc fc fc fc fc
ffff8801cc131780: 00 fc fc fc fc fc fc fc fb fb fb fb fc fc fc fc
==================================================================


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

syzbot will keep track of this bug report.
Once a fix for this bug is committed, please reply to this email with:
#syz fix: exact-commit-title
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug
report.
Note: all commands must start from beginning of the line in the email body.
syzbot
2017-12-01 18:52:01 UTC
Permalink
syzkaller has found reproducer for the following crash on
fb20eb9d798d2f4c1a75b7fe981d72dfa8d7270d
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master
compiler: gcc (GCC) 7.1.1 20170620
.config is attached
Raw console output is attached.
C reproducer is attached
syzkaller reproducer is attached. See https://goo.gl/kgGztJ
for information about syzkaller reproducers


==================================================================
BUG: KASAN: slab-out-of-bounds in strcmp+0x96/0xb0 lib/string.c:328
Read of size 1 at addr ffff8801cd99d2c1 by task syzkaller242593/3087

CPU: 0 PID: 3087 Comm: syzkaller242593 Not tainted
4.15.0-rc1-next-20171201+ #57
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:53
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x25b/0x340 mm/kasan/report.c:409
__asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:427
strcmp+0x96/0xb0 lib/string.c:328
security_context_to_sid_core+0x437/0x620
security/selinux/ss/services.c:1420
security_context_to_sid+0x32/0x40 security/selinux/ss/services.c:1479
selinux_setprocattr+0x51c/0xb50 security/selinux/hooks.c:5986
security_setprocattr+0x85/0xc0 security/security.c:1264
proc_pid_attr_write+0x1e6/0x280 fs/proc/base.c:2574
__vfs_write+0xef/0x970 fs/read_write.c:480
__kernel_write+0xfe/0x350 fs/read_write.c:501
write_pipe_buf+0x175/0x220 fs/splice.c:797
splice_from_pipe_feed fs/splice.c:502 [inline]
__splice_from_pipe+0x328/0x730 fs/splice.c:626
splice_from_pipe+0x1e9/0x330 fs/splice.c:661
default_file_splice_write+0x40/0x90 fs/splice.c:809
do_splice_from fs/splice.c:851 [inline]
direct_splice_actor+0x125/0x180 fs/splice.c:1018
splice_direct_to_actor+0x2c1/0x820 fs/splice.c:973
do_splice_direct+0x2a7/0x3d0 fs/splice.c:1061
do_sendfile+0x5d5/0xe90 fs/read_write.c:1413
SYSC_sendfile64 fs/read_write.c:1468 [inline]
SyS_sendfile64+0xbd/0x160 fs/read_write.c:1460
entry_SYSCALL_64_fastpath+0x1f/0x96
RIP: 0033:0x440189
RSP: 002b:00007fffa5fa4c08 EFLAGS: 00000207 ORIG_RAX: 0000000000000028
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440189
RDX: 0000000020004ff8 RSI: 0000000000000003 RDI: 0000000000000003
RBP: 00000000006ca018 R08: 68742f636f72702f R09: 68742f636f72702f
R10: 0000000000000001 R11: 0000000000000207 R12: 0000000000401a50
R13: 0000000000401ae0 R14: 0000000000000000 R15: 0000000000000000

Allocated by task 3087:
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
__do_kmalloc mm/slab.c:3712 [inline]
__kmalloc_track_caller+0x15e/0x760 mm/slab.c:3727
memdup_user+0x2c/0x90 mm/util.c:164
proc_pid_attr_write+0x115/0x280 fs/proc/base.c:2563
__vfs_write+0xef/0x970 fs/read_write.c:480
__kernel_write+0xfe/0x350 fs/read_write.c:501
write_pipe_buf+0x175/0x220 fs/splice.c:797
splice_from_pipe_feed fs/splice.c:502 [inline]
__splice_from_pipe+0x328/0x730 fs/splice.c:626
splice_from_pipe+0x1e9/0x330 fs/splice.c:661
default_file_splice_write+0x40/0x90 fs/splice.c:809
do_splice_from fs/splice.c:851 [inline]
direct_splice_actor+0x125/0x180 fs/splice.c:1018
splice_direct_to_actor+0x2c1/0x820 fs/splice.c:973
do_splice_direct+0x2a7/0x3d0 fs/splice.c:1061
do_sendfile+0x5d5/0xe90 fs/read_write.c:1413
SYSC_sendfile64 fs/read_write.c:1468 [inline]
SyS_sendfile64+0xbd/0x160 fs/read_write.c:1460
entry_SYSCALL_64_fastpath+0x1f/0x96

Freed by task 1668:
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_slab_free+0x71/0xc0 mm/kasan/kasan.c:524
__cache_free mm/slab.c:3492 [inline]
kfree+0xca/0x250 mm/slab.c:3807
load_elf_binary+0x1def/0x4c50 fs/binfmt_elf.c:1096
search_binary_handler+0x142/0x6b0 fs/exec.c:1640
exec_binprm fs/exec.c:1682 [inline]
do_execveat_common.isra.30+0x1711/0x22a0 fs/exec.c:1804
do_execve fs/exec.c:1849 [inline]
SYSC_execve fs/exec.c:1930 [inline]
SyS_execve+0x39/0x50 fs/exec.c:1925
do_syscall_64+0x26c/0x920 arch/x86/entry/common.c:285
return_from_SYSCALL_64+0x0/0x75

The buggy address belongs to the object at ffff8801cd99d2c0
which belongs to the cache kmalloc-32 of size 32
The buggy address is located 1 bytes inside of
32-byte region [ffff8801cd99d2c0, ffff8801cd99d2e0)
The buggy address belongs to the page:
page:000000007121f5c4 count:1 mapcount:0 mapping:00000000d98add9a
index:0xffff8801cd99dfc1
flags: 0x2fffc0000000100(slab)
raw: 02fffc0000000100 ffff8801cd99d000 ffff8801cd99dfc1 000000010000003f
raw: ffffea000736b3a0 ffffea0007347860 ffff8801db0001c0 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff8801cd99d180: fb fb fb fb fc fc fc fc fb fb fb fb fc fc fc fc
ffff8801cd99d200: fb fb fb fb fc fc fc fc fb fb fb fb fc fc fc fc
ffff8801cd99d280: fb fb fb fb fc fc fc fc 01 fc fc fc fc fc fc fc
^
ffff8801cd99d300: fb fb fb fb fc fc fc fc fb fb fb fb fc fc fc fc
ffff8801cd99d380: fb fb fb fb fc fc fc fc fb fb fb fb fc fc fc fc
==================================================================
Tetsuo Handa
2017-12-03 11:33:00 UTC
Permalink
Post by syzbot
==================================================================
BUG: KASAN: slab-out-of-bounds in strcmp+0x96/0xb0 lib/string.c:328
Read of size 1 at addr ffff8801cd99d2c1 by task syzkaller242593/3087
CPU: 0 PID: 3087 Comm: syzkaller242593 Not tainted 4.15.0-rc1-next-20171201+ #57
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:53
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x25b/0x340 mm/kasan/report.c:409
__asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:427
strcmp+0x96/0xb0 lib/string.c:328
This seems to be out of bound read for "scontext" at

for (i = 1; i < SECINITSID_NUM; i++) {
if (!strcmp(initial_sid_to_string[i], scontext)) {
*sid = i;
return 0;
}
}

because "initial_sid_to_string[i]" is "const char *".
Post by syzbot
security_context_to_sid_core+0x437/0x620 security/selinux/ss/services.c:1420
security_context_to_sid+0x32/0x40 security/selinux/ss/services.c:1479
selinux_setprocattr+0x51c/0xb50 security/selinux/hooks.c:5986
security_setprocattr+0x85/0xc0 security/security.c:1264
If "value" does not terminate with '\0' or '\n', "value" and
"size" are as-is passed to "scontext" and "scontext_len" above

/* Obtain a SID for the context, if one was specified. */
if (size && str[0] && str[0] != '\n') {
if (str[size-1] == '\n') {
str[size-1] = 0;
size--;
}
error = security_context_to_sid(value, size, &sid, GFP_KERNEL);

which will allow strcmp() to trigger out of bound read when "size" is
larger than strlen(initial_sid_to_string[i]).

Thus, I guess the simplest fix is to use strncmp() instead of strcmp().
Post by syzbot
proc_pid_attr_write+0x1e6/0x280 fs/proc/base.c:2574
__vfs_write+0xef/0x970 fs/read_write.c:480
__kernel_write+0xfe/0x350 fs/read_write.c:501
write_pipe_buf+0x175/0x220 fs/splice.c:797
splice_from_pipe_feed fs/splice.c:502 [inline]
__splice_from_pipe+0x328/0x730 fs/splice.c:626
splice_from_pipe+0x1e9/0x330 fs/splice.c:661
default_file_splice_write+0x40/0x90 fs/splice.c:809
do_splice_from fs/splice.c:851 [inline]
direct_splice_actor+0x125/0x180 fs/splice.c:1018
splice_direct_to_actor+0x2c1/0x820 fs/splice.c:973
do_splice_direct+0x2a7/0x3d0 fs/splice.c:1061
do_sendfile+0x5d5/0xe90 fs/read_write.c:1413
SYSC_sendfile64 fs/read_write.c:1468 [inline]
SyS_sendfile64+0xbd/0x160 fs/read_write.c:1460
entry_SYSCALL_64_fastpath+0x1f/0x96
Tetsuo Handa
2017-12-03 13:27:50 UTC
Permalink
Post by Tetsuo Handa
which will allow strcmp() to trigger out of bound read when "size" is
larger than strlen(initial_sid_to_string[i]).
Oops. "smaller" than.
Post by Tetsuo Handa
Thus, I guess the simplest fix is to use strncmp() instead of strcmp().
Can somebody test below patch? (My CentOS 7 environment does not support
enabling SELinux in linux.git . Userspace tool is too old to support?)
----------
Post by Tetsuo Handa
From 3efab617f7c22360361a2bd89a0ccaf3bcd47951 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-***@I-love.SAKURA.ne.jp>
Date: Sun, 3 Dec 2017 22:12:17 +0900
Subject: [PATCH] selinux: Fix out of bounds read at
security_context_to_sid_core()

Syzbot caught an out of bounds read at security_context_to_sid_core()
because security_context_to_sid_core() assumed that the value written to
/proc/pid/attr interface is terminated with either '\0' or '\n'.
When the value is not terminated with either '\0' or '\n' and
scontext_len < strlen(initial_sid_to_string[i]) is true, strcmp() will
trigger out of bounds read.

----------
BUG: KASAN: slab-out-of-bounds in strcmp+0x96/0xb0 lib/string.c:328
Read of size 1 at addr ffff8801cd99d2c1 by task syzkaller242593/3087

CPU: 0 PID: 3087 Comm: syzkaller242593 Not tainted 4.15.0-rc1-next-20171201+ #57
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:53
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x25b/0x340 mm/kasan/report.c:409
__asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:427
strcmp+0x96/0xb0 lib/string.c:328
security_context_to_sid_core+0x437/0x620 security/selinux/ss/services.c:1420
security_context_to_sid+0x32/0x40 security/selinux/ss/services.c:1479
selinux_setprocattr+0x51c/0xb50 security/selinux/hooks.c:5986
security_setprocattr+0x85/0xc0 security/security.c:1264
proc_pid_attr_write+0x1e6/0x280 fs/proc/base.c:2574
__vfs_write+0xef/0x970 fs/read_write.c:480
__kernel_write+0xfe/0x350 fs/read_write.c:501
write_pipe_buf+0x175/0x220 fs/splice.c:797
splice_from_pipe_feed fs/splice.c:502 [inline]
__splice_from_pipe+0x328/0x730 fs/splice.c:626
splice_from_pipe+0x1e9/0x330 fs/splice.c:661
default_file_splice_write+0x40/0x90 fs/splice.c:809
do_splice_from fs/splice.c:851 [inline]
direct_splice_actor+0x125/0x180 fs/splice.c:1018
splice_direct_to_actor+0x2c1/0x820 fs/splice.c:973
do_splice_direct+0x2a7/0x3d0 fs/splice.c:1061
do_sendfile+0x5d5/0xe90 fs/read_write.c:1413
SYSC_sendfile64 fs/read_write.c:1468 [inline]
SyS_sendfile64+0xbd/0x160 fs/read_write.c:1460
entry_SYSCALL_64_fastpath+0x1f/0x96
----------

Signed-off-by: Tetsuo Handa <penguin-***@I-love.SAKURA.ne.jp>
Reported-by: syzbot <***@googlegroups.com>
---
security/selinux/ss/services.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 33cfe5d..2b2ce3e 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1417,7 +1417,9 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
int i;

for (i = 1; i < SECINITSID_NUM; i++) {
- if (!strcmp(initial_sid_to_string[i], scontext)) {
+ if (!strncmp(initial_sid_to_string[i], scontext,
+ scontext_len) &&
+ !initial_sid_to_string[i][scontext_len]) {
*sid = i;
return 0;
}
--
1.8.3.1
James Morris
2017-12-04 00:51:55 UTC
Permalink
Post by Tetsuo Handa
Post by Tetsuo Handa
which will allow strcmp() to trigger out of bound read when "size" is
larger than strlen(initial_sid_to_string[i]).
Oops. "smaller" than.
Post by Tetsuo Handa
Thus, I guess the simplest fix is to use strncmp() instead of strcmp().
Can somebody test below patch? (My CentOS 7 environment does not support
enabling SELinux in linux.git . Userspace tool is too old to support?)
You mean enabling KASAN? Yep, you need gcc 4.9.2 or better. Recent
Fedora has it.
--
James Morris
<***@oracle.com>
Tetsuo Handa
2017-12-04 10:49:10 UTC
Permalink
Post by James Morris
Post by Tetsuo Handa
Post by Tetsuo Handa
which will allow strcmp() to trigger out of bound read when "size" is
larger than strlen(initial_sid_to_string[i]).
Oops. "smaller" than.
Post by Tetsuo Handa
Thus, I guess the simplest fix is to use strncmp() instead of strcmp().
Can somebody test below patch? (My CentOS 7 environment does not support
enabling SELinux in linux.git . Userspace tool is too old to support?)
You mean enabling KASAN? Yep, you need gcc 4.9.2 or better. Recent
Fedora has it.
I was not able to find "SELinux: Initializing." line for some reason, and
it turned out that I just forgot to run "make install". ;-)
I tested using debug printk() and init for built-in initramfs shown below.
It is strange that KASAN does not trigger upon strcmp()ing
initial_sid_to_string[1]. But anyway, my patch fixes this problem.
Sorry for noise. It was just initial_sid_to_string[10] which compared the second byte.
Thus, nothing is strange.
Dmitry Vyukov via Selinux
2017-12-04 04:53:10 UTC
Permalink
On Sun, Dec 3, 2017 at 2:27 PM, Tetsuo Handa
Post by Tetsuo Handa
Post by Tetsuo Handa
which will allow strcmp() to trigger out of bound read when "size" is
larger than strlen(initial_sid_to_string[i]).
Oops. "smaller" than.
Post by Tetsuo Handa
Thus, I guess the simplest fix is to use strncmp() instead of strcmp().
Can somebody test below patch? (My CentOS 7 environment does not support
enabling SELinux in linux.git . Userspace tool is too old to support?)
Hi Tetsuo,

syzbot supports testing of patches. See footer of the first email in thread.
Post by Tetsuo Handa
----------
Post by Tetsuo Handa
From 3efab617f7c22360361a2bd89a0ccaf3bcd47951 Mon Sep 17 00:00:00 2001
Date: Sun, 3 Dec 2017 22:12:17 +0900
Subject: [PATCH] selinux: Fix out of bounds read at
security_context_to_sid_core()
Syzbot caught an out of bounds read at security_context_to_sid_core()
because security_context_to_sid_core() assumed that the value written to
/proc/pid/attr interface is terminated with either '\0' or '\n'.
When the value is not terminated with either '\0' or '\n' and
scontext_len < strlen(initial_sid_to_string[i]) is true, strcmp() will
trigger out of bounds read.
----------
BUG: KASAN: slab-out-of-bounds in strcmp+0x96/0xb0 lib/string.c:328
Read of size 1 at addr ffff8801cd99d2c1 by task syzkaller242593/3087
CPU: 0 PID: 3087 Comm: syzkaller242593 Not tainted 4.15.0-rc1-next-20171201+ #57
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:53
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x25b/0x340 mm/kasan/report.c:409
__asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:427
strcmp+0x96/0xb0 lib/string.c:328
security_context_to_sid_core+0x437/0x620 security/selinux/ss/services.c:1420
security_context_to_sid+0x32/0x40 security/selinux/ss/services.c:1479
selinux_setprocattr+0x51c/0xb50 security/selinux/hooks.c:5986
security_setprocattr+0x85/0xc0 security/security.c:1264
proc_pid_attr_write+0x1e6/0x280 fs/proc/base.c:2574
__vfs_write+0xef/0x970 fs/read_write.c:480
__kernel_write+0xfe/0x350 fs/read_write.c:501
write_pipe_buf+0x175/0x220 fs/splice.c:797
splice_from_pipe_feed fs/splice.c:502 [inline]
__splice_from_pipe+0x328/0x730 fs/splice.c:626
splice_from_pipe+0x1e9/0x330 fs/splice.c:661
default_file_splice_write+0x40/0x90 fs/splice.c:809
do_splice_from fs/splice.c:851 [inline]
direct_splice_actor+0x125/0x180 fs/splice.c:1018
splice_direct_to_actor+0x2c1/0x820 fs/splice.c:973
do_splice_direct+0x2a7/0x3d0 fs/splice.c:1061
do_sendfile+0x5d5/0xe90 fs/read_write.c:1413
SYSC_sendfile64 fs/read_write.c:1468 [inline]
SyS_sendfile64+0xbd/0x160 fs/read_write.c:1460
entry_SYSCALL_64_fastpath+0x1f/0x96
----------
---
security/selinux/ss/services.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 33cfe5d..2b2ce3e 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1417,7 +1417,9 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
int i;
for (i = 1; i < SECINITSID_NUM; i++) {
- if (!strcmp(initial_sid_to_string[i], scontext)) {
+ if (!strncmp(initial_sid_to_string[i], scontext,
+ scontext_len) &&
+ !initial_sid_to_string[i][scontext_len]) {
*sid = i;
return 0;
}
--
1.8.3.1
--
You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/201712032227.JCH90603.HQOOtVFMJOFLSF%40I-love.SAKURA.ne.jp.
For more options, visit https://groups.google.com/d/optout.
Stephen Smalley
2017-12-04 13:43:45 UTC
Permalink
Post by Tetsuo Handa
Post by syzbot
==================================================================
BUG: KASAN: slab-out-of-bounds in strcmp+0x96/0xb0 lib/string.c:328
Read of size 1 at addr ffff8801cd99d2c1 by task
syzkaller242593/3087
CPU: 0 PID: 3087 Comm: syzkaller242593 Not tainted 4.15.0-rc1-next-
20171201+ #57
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS Google 01/01/2011
 __dump_stack lib/dump_stack.c:17 [inline]
 dump_stack+0x194/0x257 lib/dump_stack.c:53
 print_address_description+0x73/0x250 mm/kasan/report.c:252
 kasan_report_error mm/kasan/report.c:351 [inline]
 kasan_report+0x25b/0x340 mm/kasan/report.c:409
 __asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:427
 strcmp+0x96/0xb0 lib/string.c:328
This seems to be out of bound read for "scontext" at
for (i = 1; i < SECINITSID_NUM; i++) {
if (!strcmp(initial_sid_to_string[i], scontext)) {
*sid = i;
return 0;
}
}
because "initial_sid_to_string[i]" is "const char *".
Post by syzbot
 security_context_to_sid_core+0x437/0x620
security/selinux/ss/services.c:1420
 security_context_to_sid+0x32/0x40
security/selinux/ss/services.c:1479
 selinux_setprocattr+0x51c/0xb50 security/selinux/hooks.c:5986
 security_setprocattr+0x85/0xc0 security/security.c:1264
If "value" does not terminate with '\0' or '\n', "value" and
"size" are as-is passed to "scontext" and "scontext_len" above
/* Obtain a SID for the context, if one was specified. */
if (size && str[0] && str[0] != '\n') {
if (str[size-1] == '\n') {
str[size-1] = 0;
size--;
}
error = security_context_to_sid(value, size, &sid,
GFP_KERNEL);
which will allow strcmp() to trigger out of bound read when "size" is
larger than strlen(initial_sid_to_string[i]).
Thus, I guess the simplest fix is to use strncmp() instead of
strcmp().
Already fixed by
https://www.spinics.net/lists/selinux/msg23589.html
Post by Tetsuo Handa
Post by syzbot
 proc_pid_attr_write+0x1e6/0x280 fs/proc/base.c:2574
 __vfs_write+0xef/0x970 fs/read_write.c:480
 __kernel_write+0xfe/0x350 fs/read_write.c:501
 write_pipe_buf+0x175/0x220 fs/splice.c:797
 splice_from_pipe_feed fs/splice.c:502 [inline]
 __splice_from_pipe+0x328/0x730 fs/splice.c:626
 splice_from_pipe+0x1e9/0x330 fs/splice.c:661
 default_file_splice_write+0x40/0x90 fs/splice.c:809
 do_splice_from fs/splice.c:851 [inline]
 direct_splice_actor+0x125/0x180 fs/splice.c:1018
 splice_direct_to_actor+0x2c1/0x820 fs/splice.c:973
 do_splice_direct+0x2a7/0x3d0 fs/splice.c:1061
 do_sendfile+0x5d5/0xe90 fs/read_write.c:1413
 SYSC_sendfile64 fs/read_write.c:1468 [inline]
 SyS_sendfile64+0xbd/0x160 fs/read_write.c:1460
 entry_SYSCALL_64_fastpath+0x1f/0x96
Dmitry Vyukov via Selinux
2017-12-04 13:47:34 UTC
Permalink
Post by Stephen Smalley
Post by Tetsuo Handa
Post by syzbot
==================================================================
BUG: KASAN: slab-out-of-bounds in strcmp+0x96/0xb0 lib/string.c:328
Read of size 1 at addr ffff8801cd99d2c1 by task
syzkaller242593/3087
CPU: 0 PID: 3087 Comm: syzkaller242593 Not tainted 4.15.0-rc1-next-
20171201+ #57
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS Google 01/01/2011
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:53
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x25b/0x340 mm/kasan/report.c:409
__asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:427
strcmp+0x96/0xb0 lib/string.c:328
This seems to be out of bound read for "scontext" at
for (i = 1; i < SECINITSID_NUM; i++) {
if (!strcmp(initial_sid_to_string[i], scontext)) {
*sid = i;
return 0;
}
}
because "initial_sid_to_string[i]" is "const char *".
Post by syzbot
security_context_to_sid_core+0x437/0x620
security/selinux/ss/services.c:1420
security_context_to_sid+0x32/0x40
security/selinux/ss/services.c:1479
selinux_setprocattr+0x51c/0xb50 security/selinux/hooks.c:5986
security_setprocattr+0x85/0xc0 security/security.c:1264
If "value" does not terminate with '\0' or '\n', "value" and
"size" are as-is passed to "scontext" and "scontext_len" above
/* Obtain a SID for the context, if one was specified. */
if (size && str[0] && str[0] != '\n') {
if (str[size-1] == '\n') {
str[size-1] = 0;
size--;
}
error = security_context_to_sid(value, size, &sid, GFP_KERNEL);
which will allow strcmp() to trigger out of bound read when "size" is
larger than strlen(initial_sid_to_string[i]).
Thus, I guess the simplest fix is to use strncmp() instead of
strcmp().
Already fixed by
https://www.spinics.net/lists/selinux/msg23589.html
syzbot will keep track of this bug report.
#syz fix: exact-commit-title
Note: all commands must start from beginning of the line in the email body.
This will greatly help to keep overall process running. Thanks.
Dmitry Vyukov via Selinux
2017-12-04 16:29:49 UTC
Permalink
Post by Dmitry Vyukov via Selinux
Post by Stephen Smalley
Post by Tetsuo Handa
Post by syzbot
==================================================================
BUG: KASAN: slab-out-of-bounds in strcmp+0x96/0xb0 lib/string.c:328
Read of size 1 at addr ffff8801cd99d2c1 by task
syzkaller242593/3087
CPU: 0 PID: 3087 Comm: syzkaller242593 Not tainted 4.15.0-rc1-next-
20171201+ #57
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS Google 01/01/2011
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:53
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x25b/0x340 mm/kasan/report.c:409
__asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:427
strcmp+0x96/0xb0 lib/string.c:328
This seems to be out of bound read for "scontext" at
for (i = 1; i < SECINITSID_NUM; i++) {
if (!strcmp(initial_sid_to_string[i], scontext)) {
*sid = i;
return 0;
}
}
because "initial_sid_to_string[i]" is "const char *".
Post by syzbot
security_context_to_sid_core+0x437/0x620
security/selinux/ss/services.c:1420
security_context_to_sid+0x32/0x40
security/selinux/ss/services.c:1479
selinux_setprocattr+0x51c/0xb50 security/selinux/hooks.c:5986
security_setprocattr+0x85/0xc0 security/security.c:1264
If "value" does not terminate with '\0' or '\n', "value" and
"size" are as-is passed to "scontext" and "scontext_len" above
/* Obtain a SID for the context, if one was specified. */
if (size && str[0] && str[0] != '\n') {
if (str[size-1] == '\n') {
str[size-1] = 0;
size--;
}
error = security_context_to_sid(value, size, &sid, GFP_KERNEL);
which will allow strcmp() to trigger out of bound read when "size" is
larger than strlen(initial_sid_to_string[i]).
Thus, I guess the simplest fix is to use strncmp() instead of strcmp().
Already fixed by
https://www.spinics.net/lists/selinux/msg23589.html
syzbot will keep track of this bug report.
#syz fix: exact-commit-title
Note: all commands must start from beginning of the line in the email body.
This will greatly help to keep overall process running. Thanks.
When is the right time to do this? The text say to reply when a patch
has been committed, but where? My selinux/next branch? Linus'
master? Your docs and the end of the email needs to be more clear on
this.
Hi Paul,

We are just rolling in the process. Feedback is much appreciated!

The idea is that we need to know the title as it will appear in Linus
tree and in other tested trees. It's also possible to override the
title later, if there is any mess with it. So sending "#syz fix" as
soon as it is merged into any tree looks like the best option (to not
require you to keep in mind that you also need to do that tiny bit in
a month).

Are the following changes look good to you?
For email footer:

-Once a fix for this bug is committed, please reply to this email with:
+Once a fix for this bug is merged into any tree, reply to this email with:
#syz fix: exact-commit-title


And for the https://github.com/google/syzkaller/blob/master/docs/syzbot.md page:

to attach a fixing commit to the bug:
#syz fix: exact-commit-title
+It's enough that the commit is merged into any tree, in particular,
+you don't need to wait for the commit to be merged into upstream tree.
+syzbot only needs to know the title by which it will appear in tested trees.
+In case of an error or a title change, you can override the commit simply
+by sending another #syz fix command.
For the record, I did see that part of the syzbot mail but I was
Then sorry for pinging. We are trying to establish the process, and
some developers don't notice that part, so I just wanted to make sure.
waiting until I merged that patch; v2 was posted late in the week and
I was giving it a few days in case someone saw something
objectionable.
In such case you can do either way. You can wait, or you can post
commit title as soon as you have enough assurance that that's the
title with which it will appear in trees. We don't want to put too
much burden on developers. As I said, it's possible to override it
later, or we will notice that there is a commit that bot is waiting
for too long.

Thanks
Paul Moore
2017-12-04 21:10:15 UTC
Permalink
Post by Dmitry Vyukov via Selinux
Hi Paul,
We are just rolling in the process. Feedback is much appreciated!
The idea is that we need to know the title as it will appear in Linus
tree and in other tested trees. It's also possible to override the
title later, if there is any mess with it. So sending "#syz fix" as
soon as it is merged into any tree looks like the best option (to not
require you to keep in mind that you also need to do that tiny bit in
a month).
Are the following changes look good to you?
#syz fix: exact-commit-title
#syz fix: exact-commit-title
+It's enough that the commit is merged into any tree, in particular,
+you don't need to wait for the commit to be merged into upstream tree.
+syzbot only needs to know the title by which it will appear in tested trees.
+In case of an error or a title change, you can override the commit simply
+by sending another #syz fix command.
That is an improvement, yes. I might also mention that you would
prefer if the syzkaller-bugs list is CC'd on the commands; that wasn't
clear to me until I got a message back from syzbot just now.
Post by Dmitry Vyukov via Selinux
For the record, I did see that part of the syzbot mail but I was
Then sorry for pinging. We are trying to establish the process, and
some developers don't notice that part, so I just wanted to make sure.
I would *strongly* suggest spending some time trying to find a
mechanism that doesn't rely on developers needing to send special
commands to your system to register fixes; that seems prone to failure
if you want my honest opinion.

At the very least you might consider moving the instructions to the
top of the message.
Post by Dmitry Vyukov via Selinux
waiting until I merged that patch; v2 was posted late in the week and
I was giving it a few days in case someone saw something
objectionable.
In such case you can do either way. You can wait, or you can post
commit title as soon as you have enough assurance that that's the
title with which it will appear in trees. We don't want to put too
much burden on developers. As I said, it's possible to override it
later, or we will notice that there is a commit that bot is waiting
for too long.
Thanks
--
paul moore
security @ redhat
Dmitry Vyukov via Selinux
2017-12-05 09:39:25 UTC
Permalink
Post by Paul Moore
Post by Dmitry Vyukov via Selinux
Hi Paul,
We are just rolling in the process. Feedback is much appreciated!
The idea is that we need to know the title as it will appear in Linus
tree and in other tested trees. It's also possible to override the
title later, if there is any mess with it. So sending "#syz fix" as
soon as it is merged into any tree looks like the best option (to not
require you to keep in mind that you also need to do that tiny bit in
a month).
Are the following changes look good to you?
#syz fix: exact-commit-title
#syz fix: exact-commit-title
+It's enough that the commit is merged into any tree, in particular,
+you don't need to wait for the commit to be merged into upstream tree.
+syzbot only needs to know the title by which it will appear in tested trees.
+In case of an error or a title change, you can override the commit simply
+by sending another #syz fix command.
That is an improvement, yes. I might also mention that you would
prefer if the syzkaller-bugs list is CC'd on the commands; that wasn't
clear to me until I got a message back from syzbot just now.
Post by Dmitry Vyukov via Selinux
For the record, I did see that part of the syzbot mail but I was
Then sorry for pinging. We are trying to establish the process, and
some developers don't notice that part, so I just wanted to make sure.
I would *strongly* suggest spending some time trying to find a
mechanism that doesn't rely on developers needing to send special
commands to your system to register fixes; that seems prone to failure
if you want my honest opinion.
I completely agree. I've spent time trying to find such scheme, and
I've talked to other kernel developers, and we were not able to come
up with anything working.
Similar user-space system assume that the target is single-threaded
and deterministic and that they always have a _reliable_ reproducer
and that re-testing is fast; and than they just re-test everything on
every build. Nothing of this is true for kernel: it's highly
concurrent and non-deterministic, we don't always have reproducers,
when we have them they are not reliable and testing takes whole lot of
time. If we try to do the same, we will produce email churn declaring
bugs as fixed and then filing them as new bugs again.

One option that come up, though, is that a different of doing this
could be adding something like:

Syzbot-bug: 015afdb01dbf2abb6a6bfdd5430b72e5503fca6d

tag to commits. But then again, people will miss that, people will
mess that, and we will have to add "#syz fix" tags here and there.

A full-fledged bug tracking system would help, but as far as I
understand kernel bugzilla is ignored by most, and when it's not
ignored, from what I saw, people don't bother even marking fixed bugs
as fixed.

If somebody has other ideas, I am listening.
Post by Paul Moore
At the very least you might consider moving the instructions to the
top of the message.
Yes, this is probably something we need to consider.
Post by Paul Moore
Post by Dmitry Vyukov via Selinux
waiting until I merged that patch; v2 was posted late in the week and
I was giving it a few days in case someone saw something
objectionable.
In such case you can do either way. You can wait, or you can post
commit title as soon as you have enough assurance that that's the
title with which it will appear in trees. We don't want to put too
much burden on developers. As I said, it's possible to override it
later, or we will notice that there is a commit that bot is waiting
for too long.
Thanks
--
paul moore
Dmitry Vyukov via Selinux
2017-12-08 17:50:00 UTC
Permalink
Post by Paul Moore
Post by Dmitry Vyukov via Selinux
Hi Paul,
We are just rolling in the process. Feedback is much appreciated!
The idea is that we need to know the title as it will appear in Linus
tree and in other tested trees. It's also possible to override the
title later, if there is any mess with it. So sending "#syz fix" as
soon as it is merged into any tree looks like the best option (to not
require you to keep in mind that you also need to do that tiny bit in
a month).
Are the following changes look good to you?
#syz fix: exact-commit-title
#syz fix: exact-commit-title
+It's enough that the commit is merged into any tree, in particular,
+you don't need to wait for the commit to be merged into upstream tree.
+syzbot only needs to know the title by which it will appear in tested trees.
+In case of an error or a title change, you can override the commit simply
+by sending another #syz fix command.
That is an improvement, yes. I might also mention that you would
prefer if the syzkaller-bugs list is CC'd on the commands; that wasn't
clear to me until I got a message back from syzbot just now.
Fixed with:

https://github.com/google/syzkaller/commit/2f6fb923683af1397d3d6abea0bde51a9bdcdca7
https://github.com/google/syzkaller/commit/8e1e4403ac29a60350169da5449a1497635ee13b
Post by Paul Moore
Post by Dmitry Vyukov via Selinux
For the record, I did see that part of the syzbot mail but I was
Then sorry for pinging. We are trying to establish the process, and
some developers don't notice that part, so I just wanted to make sure.
I would *strongly* suggest spending some time trying to find a
mechanism that doesn't rely on developers needing to send special
commands to your system to register fixes; that seems prone to failure
if you want my honest opinion.
At the very least you might consider moving the instructions to the
top of the message.
Post by Dmitry Vyukov via Selinux
waiting until I merged that patch; v2 was posted late in the week and
I was giving it a few days in case someone saw something
objectionable.
In such case you can do either way. You can wait, or you can post
commit title as soon as you have enough assurance that that's the
title with which it will appear in trees. We don't want to put too
much burden on developers. As I said, it's possible to override it
later, or we will notice that there is a commit that bot is waiting
for too long.
Thanks
--
paul moore
Dmitry Vyukov via Selinux
2017-12-04 16:39:46 UTC
Permalink
Post by Dmitry Vyukov via Selinux
Post by Stephen Smalley
Post by Tetsuo Handa
Post by syzbot
==================================================================
BUG: KASAN: slab-out-of-bounds in strcmp+0x96/0xb0 lib/string.c:328
Read of size 1 at addr ffff8801cd99d2c1 by task
syzkaller242593/3087
CPU: 0 PID: 3087 Comm: syzkaller242593 Not tainted 4.15.0-rc1-next-
20171201+ #57
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS Google 01/01/2011
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:53
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x25b/0x340 mm/kasan/report.c:409
__asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:427
strcmp+0x96/0xb0 lib/string.c:328
This seems to be out of bound read for "scontext" at
for (i = 1; i < SECINITSID_NUM; i++) {
if (!strcmp(initial_sid_to_string[i], scontext)) {
*sid = i;
return 0;
}
}
because "initial_sid_to_string[i]" is "const char *".
Post by syzbot
security_context_to_sid_core+0x437/0x620
security/selinux/ss/services.c:1420
security_context_to_sid+0x32/0x40
security/selinux/ss/services.c:1479
selinux_setprocattr+0x51c/0xb50 security/selinux/hooks.c:5986
security_setprocattr+0x85/0xc0 security/security.c:1264
If "value" does not terminate with '\0' or '\n', "value" and
"size" are as-is passed to "scontext" and "scontext_len" above
/* Obtain a SID for the context, if one was specified. */
if (size && str[0] && str[0] != '\n') {
if (str[size-1] == '\n') {
str[size-1] = 0;
size--;
}
error = security_context_to_sid(value, size, &sid, GFP_KERNEL);
which will allow strcmp() to trigger out of bound read when "size" is
larger than strlen(initial_sid_to_string[i]).
Thus, I guess the simplest fix is to use strncmp() instead of strcmp().
Already fixed by
https://www.spinics.net/lists/selinux/msg23589.html
syzbot will keep track of this bug report.
#syz fix: exact-commit-title
Note: all commands must start from beginning of the line in the email body.
This will greatly help to keep overall process running. Thanks.
When is the right time to do this? The text say to reply when a patch
has been committed, but where? My selinux/next branch? Linus'
master? Your docs and the end of the email needs to be more clear on
this.
For the record, I did see that part of the syzbot mail but I was
waiting until I merged that patch; v2 was posted late in the week and
I was giving it a few days in case someone saw something
objectionable.
Also, while we are on the topic of syzbot, what SELinux policy (if
any) do you load on the system that is undergoing testing? Based on
some of the recent reports it would appear that you are running a
SELinux enabled kernel but might not be loading a SELinux policy, is
that correct?
This is good question. The problem is that we are testing almost all
kernel subsystems, but are not experts in most of them. So frequently
we doing some non-sense. And that's where we need you help.
That's what we have for grep SECURITY .config:

CONFIG_EXT4_FS_SECURITY=y
# CONFIG_9P_FS_SECURITY is not set
# CONFIG_SECURITY_DMESG_RESTRICT is not set
CONFIG_SECURITY=y
CONFIG_SECURITY_WRITABLE_HOOKS=y
# CONFIG_SECURITYFS is not set
CONFIG_SECURITY_NETWORK=y
CONFIG_SECURITY_NETWORK_XFRM=y
CONFIG_SECURITY_PATH=y
CONFIG_SECURITY_SELINUX=y
CONFIG_SECURITY_SELINUX_BOOTPARAM=y
CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE=1
CONFIG_SECURITY_SELINUX_DISABLE=y
CONFIG_SECURITY_SELINUX_DEVELOP=y
CONFIG_SECURITY_SELINUX_AVC_STATS=y
CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE=0
# CONFIG_SECURITY_SMACK is not set
# CONFIG_SECURITY_TOMOYO is not set
# CONFIG_SECURITY_APPARMOR is not set
# CONFIG_SECURITY_LOADPIN is not set
# CONFIG_SECURITY_YAMA is not set
CONFIG_DEFAULT_SECURITY_SELINUX=y
# CONFIG_DEFAULT_SECURITY_DAC is not set
CONFIG_DEFAULT_SECURITY="selinux"


I don't think we do anything else besides that.
To be fair I don't know what is SELinux policy nor how to load it. Can
you suggest a policy that would be good for testing of kernel in
general and SELinux in particular? Obviously, we don't want to
prohibit access to any major parts of kernel APIs entirely (because
then we won't test them). syzkaller also creates a local temp dir per
test process, and the process mostly accesses files there (except for
opening /dev/* and /proc/self/*). Is it possible to selectively
prohibit something there, so that we test both positive and negative
cases?

Thanks
Stephen Smalley
2017-12-04 17:33:02 UTC
Permalink
Post by Dmitry Vyukov via Selinux
Post by Dmitry Vyukov via Selinux
Post by Stephen Smalley
Post by Tetsuo Handa
Post by syzbot
===========================================================
=======
BUG: KASAN: slab-out-of-bounds in strcmp+0x96/0xb0
lib/string.c:328
Read of size 1 at addr ffff8801cd99d2c1 by task
syzkaller242593/3087
CPU: 0 PID: 3087 Comm: syzkaller242593 Not tainted 4.15.0-
rc1-next-
20171201+ #57
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS Google 01/01/2011
 __dump_stack lib/dump_stack.c:17 [inline]
 dump_stack+0x194/0x257 lib/dump_stack.c:53
 print_address_description+0x73/0x250 mm/kasan/report.c:252
 kasan_report_error mm/kasan/report.c:351 [inline]
 kasan_report+0x25b/0x340 mm/kasan/report.c:409
 __asan_report_load1_noabort+0x14/0x20
mm/kasan/report.c:427
 strcmp+0x96/0xb0 lib/string.c:328
This seems to be out of bound read for "scontext" at
      for (i = 1; i < SECINITSID_NUM; i++) {
              if (!strcmp(initial_sid_to_string[i],
scontext)) {
                      *sid = i;
                      return 0;
              }
      }
because "initial_sid_to_string[i]" is "const char *".
Post by syzbot
 security_context_to_sid_core+0x437/0x620
security/selinux/ss/services.c:1420
 security_context_to_sid+0x32/0x40
security/selinux/ss/services.c:1479
 selinux_setprocattr+0x51c/0xb50
security/selinux/hooks.c:5986
 security_setprocattr+0x85/0xc0 security/security.c:1264
If "value" does not terminate with '\0' or '\n', "value" and
"size" are as-is passed to "scontext" and "scontext_len" above
      /* Obtain a SID for the context, if one was specified.
*/
      if (size && str[0] && str[0] != '\n') {
              if (str[size-1] == '\n') {
                      str[size-1] = 0;
                      size--;
              }
              error = security_context_to_sid(value, size,
&sid,
GFP_KERNEL);
which will allow strcmp() to trigger out of bound read when "size" is
larger than strlen(initial_sid_to_string[i]).
Thus, I guess the simplest fix is to use strncmp() instead of strcmp().
Already fixed by
https://www.spinics.net/lists/selinux/msg23589.html
syzbot will keep track of this bug report.
#syz fix: exact-commit-title
Note: all commands must start from beginning of the line in the email body.
This will greatly help to keep overall process running. Thanks.
When is the right time to do this?  The text say to reply when a
patch
has been committed, but where?  My selinux/next branch?  Linus'
master?  Your docs and the end of the email needs to be more clear
on
this.
For the record, I did see that part of the syzbot mail but I was
waiting until I merged that patch; v2 was posted late in the week and
I was giving it a few days in case someone saw something
objectionable.
Also, while we are on the topic of syzbot, what SELinux policy (if
any) do you load on the system that is undergoing testing?  Based
on
some of the recent reports it would appear that you are running a
SELinux enabled kernel but might not be loading a SELinux policy, is
that correct?
This is good question. The problem is that we are testing almost all
kernel subsystems, but are not experts in most of them. So frequently
we doing some non-sense. And that's where we need you help.
CONFIG_EXT4_FS_SECURITY=y
# CONFIG_9P_FS_SECURITY is not set
# CONFIG_SECURITY_DMESG_RESTRICT is not set
CONFIG_SECURITY=y
CONFIG_SECURITY_WRITABLE_HOOKS=y
# CONFIG_SECURITYFS is not set
CONFIG_SECURITY_NETWORK=y
CONFIG_SECURITY_NETWORK_XFRM=y
CONFIG_SECURITY_PATH=y
CONFIG_SECURITY_SELINUX=y
CONFIG_SECURITY_SELINUX_BOOTPARAM=y
CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE=1
CONFIG_SECURITY_SELINUX_DISABLE=y
CONFIG_SECURITY_SELINUX_DEVELOP=y
CONFIG_SECURITY_SELINUX_AVC_STATS=y
CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE=0
# CONFIG_SECURITY_SMACK is not set
# CONFIG_SECURITY_TOMOYO is not set
# CONFIG_SECURITY_APPARMOR is not set
# CONFIG_SECURITY_LOADPIN is not set
# CONFIG_SECURITY_YAMA is not set
CONFIG_DEFAULT_SECURITY_SELINUX=y
# CONFIG_DEFAULT_SECURITY_DAC is not set
CONFIG_DEFAULT_SECURITY="selinux"
I don't think we do anything else besides that.
To be fair I don't know what is SELinux policy nor how to load it. Can
you suggest a policy that would be good for testing of kernel in
general and SELinux in particular? Obviously, we don't want to
prohibit access to any major parts of kernel APIs entirely (because
then we won't test them). syzkaller also creates a local temp dir per
test process, and the process mostly accesses files there (except for
opening /dev/* and /proc/self/*). Is it possible to selectively
prohibit something there, so that we test both positive and negative
cases?
SELinux policy is loaded by userspace; the support is integrated into
the various init programs, but you need to have a policy installed.

Best way would just be to run the test on Fedora (or CentOS) with
selinux-policy-targeted installed. Then you would be testing with a
real policy loaded. You could run syzkaller from the unconfined_t
domain and it should be largely unimpeded, and it could transition to a
different domain if you want to test denials. For reference, the
selinux testsuite itself is at
 https://github.com/SELinuxProject/selinux-testsuite/
It includes a defconfig fragment that can be merged, although portions
of that are only required for particular test programs (see the
comments in it).

If that's not viable, then another approach would be to generate a
minimal allow-all policy from the kernel source tree, see
Documentation/admin-guide/LSM/SELinux.rst and scripts/selinux/*.
The downside of that approach is that SELinux developers and users
don't use that policy themselves for anything, and it won't exercise
any permission denial code paths.
Dmitry Vyukov via Selinux
2017-12-05 10:00:44 UTC
Permalink
Post by Stephen Smalley
Post by Dmitry Vyukov via Selinux
Post by Dmitry Vyukov via Selinux
Post by Stephen Smalley
Post by Tetsuo Handa
Post by syzbot
===========================================================
=======
BUG: KASAN: slab-out-of-bounds in strcmp+0x96/0xb0
lib/string.c:328
Read of size 1 at addr ffff8801cd99d2c1 by task
syzkaller242593/3087
CPU: 0 PID: 3087 Comm: syzkaller242593 Not tainted 4.15.0-
rc1-next-
20171201+ #57
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS Google 01/01/2011
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:53
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x25b/0x340 mm/kasan/report.c:409
__asan_report_load1_noabort+0x14/0x20
mm/kasan/report.c:427
strcmp+0x96/0xb0 lib/string.c:328
This seems to be out of bound read for "scontext" at
for (i = 1; i < SECINITSID_NUM; i++) {
if (!strcmp(initial_sid_to_string[i],
scontext)) {
*sid = i;
return 0;
}
}
because "initial_sid_to_string[i]" is "const char *".
Post by syzbot
security_context_to_sid_core+0x437/0x620
security/selinux/ss/services.c:1420
security_context_to_sid+0x32/0x40
security/selinux/ss/services.c:1479
selinux_setprocattr+0x51c/0xb50
security/selinux/hooks.c:5986
security_setprocattr+0x85/0xc0 security/security.c:1264
If "value" does not terminate with '\0' or '\n', "value" and
"size" are as-is passed to "scontext" and "scontext_len" above
/* Obtain a SID for the context, if one was specified. */
if (size && str[0] && str[0] != '\n') {
if (str[size-1] == '\n') {
str[size-1] = 0;
size--;
}
error = security_context_to_sid(value, size, &sid,
GFP_KERNEL);
which will allow strcmp() to trigger out of bound read when "size" is
larger than strlen(initial_sid_to_string[i]).
Thus, I guess the simplest fix is to use strncmp() instead of strcmp().
Already fixed by
https://www.spinics.net/lists/selinux/msg23589.html
syzbot will keep track of this bug report.
#syz fix: exact-commit-title
Note: all commands must start from beginning of the line in the email body.
This will greatly help to keep overall process running. Thanks.
When is the right time to do this? The text say to reply when a patch
has been committed, but where? My selinux/next branch? Linus'
master? Your docs and the end of the email needs to be more clear on
this.
For the record, I did see that part of the syzbot mail but I was
waiting until I merged that patch; v2 was posted late in the week and
I was giving it a few days in case someone saw something
objectionable.
Also, while we are on the topic of syzbot, what SELinux policy (if
any) do you load on the system that is undergoing testing? Based on
some of the recent reports it would appear that you are running a
SELinux enabled kernel but might not be loading a SELinux policy, is
that correct?
This is good question. The problem is that we are testing almost all
kernel subsystems, but are not experts in most of them. So frequently
we doing some non-sense. And that's where we need you help.
CONFIG_EXT4_FS_SECURITY=y
# CONFIG_9P_FS_SECURITY is not set
# CONFIG_SECURITY_DMESG_RESTRICT is not set
CONFIG_SECURITY=y
CONFIG_SECURITY_WRITABLE_HOOKS=y
# CONFIG_SECURITYFS is not set
CONFIG_SECURITY_NETWORK=y
CONFIG_SECURITY_NETWORK_XFRM=y
CONFIG_SECURITY_PATH=y
CONFIG_SECURITY_SELINUX=y
CONFIG_SECURITY_SELINUX_BOOTPARAM=y
CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE=1
CONFIG_SECURITY_SELINUX_DISABLE=y
CONFIG_SECURITY_SELINUX_DEVELOP=y
CONFIG_SECURITY_SELINUX_AVC_STATS=y
CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE=0
# CONFIG_SECURITY_SMACK is not set
# CONFIG_SECURITY_TOMOYO is not set
# CONFIG_SECURITY_APPARMOR is not set
# CONFIG_SECURITY_LOADPIN is not set
# CONFIG_SECURITY_YAMA is not set
CONFIG_DEFAULT_SECURITY_SELINUX=y
# CONFIG_DEFAULT_SECURITY_DAC is not set
CONFIG_DEFAULT_SECURITY="selinux"
I don't think we do anything else besides that.
To be fair I don't know what is SELinux policy nor how to load it. Can
you suggest a policy that would be good for testing of kernel in
general and SELinux in particular? Obviously, we don't want to
prohibit access to any major parts of kernel APIs entirely (because
then we won't test them). syzkaller also creates a local temp dir per
test process, and the process mostly accesses files there (except for
opening /dev/* and /proc/self/*). Is it possible to selectively
prohibit something there, so that we test both positive and negative
cases?
SELinux policy is loaded by userspace; the support is integrated into
the various init programs, but you need to have a policy installed.
Best way would just be to run the test on Fedora (or CentOS) with
selinux-policy-targeted installed. Then you would be testing with a
real policy loaded. You could run syzkaller from the unconfined_t
domain and it should be largely unimpeded, and it could transition to a
different domain if you want to test denials. For reference, the
selinux testsuite itself is at
https://github.com/SELinuxProject/selinux-testsuite/
It includes a defconfig fragment that can be merged, although portions
of that are only required for particular test programs (see the
comments in it).
If that's not viable, then another approach would be to generate a
minimal allow-all policy from the kernel source tree, see
Documentation/admin-guide/LSM/SELinux.rst and scripts/selinux/*.
The downside of that approach is that SELinux developers and users
don't use that policy themselves for anything, and it won't exercise
any permission denial code paths.
Thanks, I will see what we can do here. Still lots to learn for me.
Dmitry Vyukov via Selinux
2017-12-08 12:22:45 UTC
Permalink
Post by Dmitry Vyukov via Selinux
Post by Stephen Smalley
Post by Dmitry Vyukov via Selinux
Post by Dmitry Vyukov via Selinux
Post by Stephen Smalley
Post by Tetsuo Handa
Post by syzbot
===========================================================
=======
BUG: KASAN: slab-out-of-bounds in strcmp+0x96/0xb0
lib/string.c:328
Read of size 1 at addr ffff8801cd99d2c1 by task
syzkaller242593/3087
CPU: 0 PID: 3087 Comm: syzkaller242593 Not tainted 4.15.0-
rc1-next-
20171201+ #57
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS Google 01/01/2011
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:53
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x25b/0x340 mm/kasan/report.c:409
__asan_report_load1_noabort+0x14/0x20
mm/kasan/report.c:427
strcmp+0x96/0xb0 lib/string.c:328
This seems to be out of bound read for "scontext" at
for (i = 1; i < SECINITSID_NUM; i++) {
if (!strcmp(initial_sid_to_string[i],
scontext)) {
*sid = i;
return 0;
}
}
because "initial_sid_to_string[i]" is "const char *".
Post by syzbot
security_context_to_sid_core+0x437/0x620
security/selinux/ss/services.c:1420
security_context_to_sid+0x32/0x40
security/selinux/ss/services.c:1479
selinux_setprocattr+0x51c/0xb50
security/selinux/hooks.c:5986
security_setprocattr+0x85/0xc0 security/security.c:1264
If "value" does not terminate with '\0' or '\n', "value" and
"size" are as-is passed to "scontext" and "scontext_len" above
/* Obtain a SID for the context, if one was specified. */
if (size && str[0] && str[0] != '\n') {
if (str[size-1] == '\n') {
str[size-1] = 0;
size--;
}
error = security_context_to_sid(value, size, &sid,
GFP_KERNEL);
which will allow strcmp() to trigger out of bound read when "size" is
larger than strlen(initial_sid_to_string[i]).
Thus, I guess the simplest fix is to use strncmp() instead of strcmp().
Already fixed by
https://www.spinics.net/lists/selinux/msg23589.html
syzbot will keep track of this bug report.
#syz fix: exact-commit-title
Note: all commands must start from beginning of the line in the email body.
This will greatly help to keep overall process running. Thanks.
When is the right time to do this? The text say to reply when a patch
has been committed, but where? My selinux/next branch? Linus'
master? Your docs and the end of the email needs to be more clear on
this.
For the record, I did see that part of the syzbot mail but I was
waiting until I merged that patch; v2 was posted late in the week and
I was giving it a few days in case someone saw something
objectionable.
Also, while we are on the topic of syzbot, what SELinux policy (if
any) do you load on the system that is undergoing testing? Based on
some of the recent reports it would appear that you are running a
SELinux enabled kernel but might not be loading a SELinux policy, is
that correct?
This is good question. The problem is that we are testing almost all
kernel subsystems, but are not experts in most of them. So frequently
we doing some non-sense. And that's where we need you help.
CONFIG_EXT4_FS_SECURITY=y
# CONFIG_9P_FS_SECURITY is not set
# CONFIG_SECURITY_DMESG_RESTRICT is not set
CONFIG_SECURITY=y
CONFIG_SECURITY_WRITABLE_HOOKS=y
# CONFIG_SECURITYFS is not set
CONFIG_SECURITY_NETWORK=y
CONFIG_SECURITY_NETWORK_XFRM=y
CONFIG_SECURITY_PATH=y
CONFIG_SECURITY_SELINUX=y
CONFIG_SECURITY_SELINUX_BOOTPARAM=y
CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE=1
CONFIG_SECURITY_SELINUX_DISABLE=y
CONFIG_SECURITY_SELINUX_DEVELOP=y
CONFIG_SECURITY_SELINUX_AVC_STATS=y
CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE=0
# CONFIG_SECURITY_SMACK is not set
# CONFIG_SECURITY_TOMOYO is not set
# CONFIG_SECURITY_APPARMOR is not set
# CONFIG_SECURITY_LOADPIN is not set
# CONFIG_SECURITY_YAMA is not set
CONFIG_DEFAULT_SECURITY_SELINUX=y
# CONFIG_DEFAULT_SECURITY_DAC is not set
CONFIG_DEFAULT_SECURITY="selinux"
I don't think we do anything else besides that.
To be fair I don't know what is SELinux policy nor how to load it. Can
you suggest a policy that would be good for testing of kernel in
general and SELinux in particular? Obviously, we don't want to
prohibit access to any major parts of kernel APIs entirely (because
then we won't test them). syzkaller also creates a local temp dir per
test process, and the process mostly accesses files there (except for
opening /dev/* and /proc/self/*). Is it possible to selectively
prohibit something there, so that we test both positive and negative
cases?
SELinux policy is loaded by userspace; the support is integrated into
the various init programs, but you need to have a policy installed.
Best way would just be to run the test on Fedora (or CentOS) with
selinux-policy-targeted installed. Then you would be testing with a
real policy loaded. You could run syzkaller from the unconfined_t
domain and it should be largely unimpeded, and it could transition to a
different domain if you want to test denials. For reference, the
selinux testsuite itself is at
https://github.com/SELinuxProject/selinux-testsuite/
It includes a defconfig fragment that can be merged, although portions
of that are only required for particular test programs (see the
comments in it).
If that's not viable, then another approach would be to generate a
minimal allow-all policy from the kernel source tree, see
Documentation/admin-guide/LSM/SELinux.rst and scripts/selinux/*.
The downside of that approach is that SELinux developers and users
don't use that policy themselves for anything, and it won't exercise
any permission denial code paths.
Thanks, I will see what we can do here. Still lots to learn for me.
I figured out why selinux wasn't initialized in our setup. The image
missed selinux packages, so init could not initialize it. I've added
these packages:
https://github.com/google/syzkaller/commit/c0e5b8c81f054a301aeaaa16bf9c6220ba73d833
and now sestatus says that selinux is initialized.

I've also added some very basic description of selinux interface:
https://github.com/google/syzkaller/commit/fadd10ac0525a437fc32e412327bdf11587f4946#diff-517fae0da2ea50677a86c092cf96781e
Not enough to meaningfully test selinux, though.
Tetsuo Handa
2017-12-04 14:07:56 UTC
Permalink
Post by Stephen Smalley
Post by Tetsuo Handa
Thus, I guess the simplest fix is to use strncmp() instead of strcmp().
Already fixed by
https://www.spinics.net/lists/selinux/msg23589.html
OK, I thought everyone was too busy.
I would appreciate if you responded to all.
Loading...