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
==================================================================
On 2017/12/02 3:52, syzbot wrote:
> ==================================================================
> 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
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 *".
> 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().
> 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 wrote:
> which will allow strcmp() to trigger out of bound read when "size" is
> larger than strlen(initial_sid_to_string[i]).
Oops. "smaller" than.
>
> 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?)
----------
>From 3efab617f7c22360361a2bd89a0ccaf3bcd47951 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <[email protected]>
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 <[email protected]>
Reported-by: syzbot <[email protected]>
---
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
On Sun, 3 Dec 2017, Tetsuo Handa wrote:
> Tetsuo Handa wrote:
> > which will allow strcmp() to trigger out of bound read when "size" is
> > larger than strlen(initial_sid_to_string[i]).
>
> Oops. "smaller" than.
>
> >
> > 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
<[email protected]>
On Sun, Dec 3, 2017 at 2:27 PM, Tetsuo Handa
<[email protected]> wrote:
> Tetsuo Handa wrote:
>> which will allow strcmp() to trigger out of bound read when "size" is
>> larger than strlen(initial_sid_to_string[i]).
>
> Oops. "smaller" than.
>
>>
>> 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.
> ----------
> >From 3efab617f7c22360361a2bd89a0ccaf3bcd47951 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <[email protected]>
> 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 <[email protected]>
> Reported-by: syzbot <[email protected]>
> ---
> 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 unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> 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.
James Morris wrote:
> On Sun, 3 Dec 2017, Tetsuo Handa wrote:
>
> > Tetsuo Handa wrote:
> > > which will allow strcmp() to trigger out of bound read when "size" is
> > > larger than strlen(initial_sid_to_string[i]).
> >
> > Oops. "smaller" than.
> >
> > >
> > > 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.
----------
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5973,6 +5973,7 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
PROCESS__SETCURRENT, NULL);
else
error = -EINVAL;
+ printk("setprocattr %s=%d size=%lu\n", name, error, size);
if (error)
return error;
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 33cfe5d..fbf0ade 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1417,6 +1417,7 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
int i;
for (i = 1; i < SECINITSID_NUM; i++) {
+ printk("Comparing with %s\n", initial_sid_to_string[i]);
if (!strcmp(initial_sid_to_string[i], scontext)) {
*sid = i;
return 0;
----------
----------
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/mount.h>
#include <unistd.h>
int main(int argc, char *argv[])
{
int fd;
mount("/proc", "/proc", "proc", 0, NULL);
fd = open("/proc/self/attr/current", O_WRONLY);
write(fd, "n", 1);
close(fd);
return 0;
}
----------
----------
[ 7.894061] Write protecting the kernel read-only data: 71680k
[ 7.899889] Freeing unused kernel memory: 1744K
[ 7.923592] Freeing unused kernel memory: 1832K
[ 7.926960] setprocattr current=0 size=1
[ 7.928253] Comparing with kernel
[ 7.929350] Comparing with security
[ 7.930457] Comparing with unlabeled
[ 7.931581] Comparing with fs
[ 7.932538] Comparing with file
[ 7.933537] Comparing with file_labels
[ 7.934720] Comparing with init
[ 7.935719] Comparing with any_socket
[ 7.936866] Comparing with port
[ 7.937874] Comparing with netif
[ 7.938965] ==================================================================
[ 7.941183] BUG: KASAN: slab-out-of-bounds in strcmp+0x96/0xb0
[ 7.942957] Read of size 1 at addr ffff8801145a5441 by task init/1
[ 7.944832]
[ 7.945349] CPU: 2 PID: 1 Comm: init Not tainted 4.15.0-rc2+ #323
[ 7.947177] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/02/2015
[ 7.950331] Call Trace:
[ 7.951133] dump_stack+0x12e/0x188
[ 7.952222] ? vprintk_default+0x28/0x30
[ 7.953431] ? strcmp+0x96/0xb0
[ 7.954421] print_address_description+0x73/0x260
[ 7.955860] ? strcmp+0x96/0xb0
[ 7.956855] kasan_report+0x22b/0x340
[ 7.957987] __asan_report_load1_noabort+0x14/0x20
[ 7.959460] strcmp+0x96/0xb0
[ 7.960408] security_context_to_sid_core+0x312/0x450
[ 7.961945] ? string_to_context_struct+0x940/0x940
[ 7.963434] ? vprintk_func+0x5e/0xc0
[ 7.964564] ? printk+0xaa/0xca
[ 7.965554] ? show_regs_print_info+0x65/0x65
[ 7.966876] ? proc_pid_attr_write+0x169/0x280
[ 7.968178] security_context_to_sid+0x32/0x40
[ 7.969480] selinux_setprocattr+0x2e1/0x8f0
[ 7.970734] ? ptrace_parent_sid+0x400/0x400
[ 7.972034] security_setprocattr+0x85/0xc0
[ 7.973326] proc_pid_attr_write+0x1d8/0x280
[ 7.974638] __vfs_write+0x10d/0x610
[ 7.975746] ? comm_write+0x230/0x230
[ 7.976903] ? kernel_read+0x120/0x120
[ 7.978064] ? __might_sleep+0x95/0x190
[ 7.979266] ? rcu_read_lock_sched_held+0xa3/0x120
[ 7.980722] ? rcu_sync_lockdep_assert+0x70/0xb0
[ 7.982134] ? __sb_start_write+0x211/0x2d0
[ 7.983413] vfs_write+0x18d/0x510
[ 7.984477] SyS_write+0xd4/0x1a0
[ 7.985518] ? SyS_read+0x1a0/0x1a0
[ 7.986622] ? trace_hardirqs_on_caller+0x442/0x5c0
[ 7.988107] ? trace_hardirqs_on_thunk+0x1a/0x1c
[ 7.989524] entry_SYSCALL_64_fastpath+0x1f/0x96
[ 7.990928] RIP: 0033:0x40f7a0
[ 7.991875] RSP: 002b:00007ffe20eb59c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[ 7.994144] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 000000000040f7a0
[ 7.996262] RDX: 0000000000000001 RSI: 0000000000492b75 RDI: 0000000000000003
[ 7.998372] RBP: 00007ffe20eb59a0 R08: 0000000000000000 R09: 0000000000000004
[ 8.000498] R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffe20eb5af8
[ 8.002629] R13: 00007ffe20eb5b08 R14: 0000000000000002 R15: 0000000000000000
[ 8.004774]
[ 8.005283] Allocated by task 1:
[ 8.006283] save_stack+0x46/0xd0
[ 8.007312] kasan_kmalloc+0xad/0xe0
[ 8.008417] __kmalloc_track_caller+0x192/0x760
[ 8.009804] memdup_user+0x2c/0x80
[ 8.010873] proc_pid_attr_write+0x115/0x280
[ 8.012172] __vfs_write+0x10d/0x610
[ 8.013283] vfs_write+0x18d/0x510
[ 8.014336] SyS_write+0xd4/0x1a0
[ 8.015380] entry_SYSCALL_64_fastpath+0x1f/0x96
[ 8.016793]
[ 8.017302] Freed by task 1:
[ 8.018211] save_stack+0x46/0xd0
[ 8.019241] kasan_slab_free+0x71/0xc0
[ 8.020389] kfree+0xca/0x250
[ 8.021317] acpi_ds_create_operand+0x45f/0x664
[ 8.022696] acpi_ds_evaluate_name_path+0x116/0x3b6
[ 8.024165] acpi_ds_exec_end_op+0x291/0xd61
[ 8.025469] acpi_ps_parse_loop+0x1338/0x13ee
[ 8.026815] acpi_ps_parse_aml+0x23a/0x7f4
[ 8.028064] acpi_ps_execute_method+0x4f2/0x55f
[ 8.029451] acpi_ns_evaluate+0x6ba/0x8d3
[ 8.030708] acpi_ut_evaluate_object+0x122/0x3c5
[ 8.032108] acpi_ut_execute_STA+0x84/0x15a
[ 8.033390] acpi_get_object_info+0x431/0x9bd
[ 8.034676] acpi_init_device_object+0xb65/0x15f0
[ 8.036040] acpi_add_single_object+0x119/0x1630
[ 8.037384] acpi_bus_check_add+0x1c7/0x520
[ 8.038656] acpi_ns_walk_namespace+0x1e0/0x346
[ 8.040029] acpi_walk_namespace+0xb5/0xef
[ 8.041277] acpi_bus_scan+0xe0/0xf0
[ 8.042376] acpi_scan_init+0x258/0x5e5
[ 8.043578] acpi_init+0x650/0x6d8
[ 8.044654] do_one_initcall+0x9e/0x2c9
[ 8.045843] kernel_init_freeable+0x476/0x52a
[ 8.047165] kernel_init+0x13/0x172
[ 8.048248] ret_from_fork+0x24/0x30
[ 8.049762]
[ 8.050268] The buggy address belongs to the object at ffff8801145a5440
[ 8.050268] which belongs to the cache kmalloc-32 of size 32
[ 8.053780] The buggy address is located 1 bytes inside of
[ 8.053780] 32-byte region [ffff8801145a5440, ffff8801145a5460)
[ 8.057068] The buggy address belongs to the page:
[ 8.058525] page:00000000243fc1bc count:1 mapcount:0 mapping:000000008e54372e index:0xffff8801145a5fc1
[ 8.061337] flags: 0x2fffc0000000100(slab)
[ 8.062619] raw: 02fffc0000000100 ffff8801145a5000 ffff8801145a5fc1 0000000100000020
[ 8.064922] raw: ffffea0004516ca0 ffff880119c01240 ffff880119c001c0 0000000000000000
[ 8.067232] page dumped because: kasan: bad access detected
[ 8.068902]
[ 8.069409] Memory state around the buggy address:
[ 8.070849] ffff8801145a5300: fb fb fb fb fc fc fc fc fb fb fb fb fc fc fc fc
[ 8.073016] ffff8801145a5380: fb fb fb fb fc fc fc fc fb fb fb fb fc fc fc fc
[ 8.075194] >ffff8801145a5400: fb fb fb fb fc fc fc fc 01 fc fc fc fc fc fc fc
[ 8.077367] ^
[ 8.078977] ffff8801145a5480: fb fb fb fb fc fc fc fc fb fb fb fb fc fc fc fc
[ 8.081133] ffff8801145a5500: 06 fc fc fc fc fc fc fc fb fb fb fb fc fc fc fc
[ 8.083293] ==================================================================
[ 8.085394] Disabling lock debugging due to kernel taint
[ 8.086963] Comparing with netmsg
[ 8.088029] Comparing with node
[ 8.088992] Comparing with igmp_packet
[ 8.090156] Comparing with icmp_socket
[ 8.091319] Comparing with tcp_socket
[ 8.092447] Comparing with sysctl_modprobe
[ 8.093731] Comparing with sysctl
[ 8.094805] Comparing with sysctl_fs
[ 8.095905] Comparing with sysctl_kernel
[ 8.097113] Comparing with sysctl_net
[ 8.098188] Comparing with sysctl_net_unix
[ 8.099370] Comparing with sysctl_vm
[ 8.100449] Comparing with sysctl_dev
[ 8.101571] Comparing with kmod
[ 8.102525] Comparing with policy
[ 8.103550] Comparing with scmp_packet
[ 8.105942] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000000
----------
Tetsuo Handa wrote:
> James Morris wrote:
> > On Sun, 3 Dec 2017, Tetsuo Handa wrote:
> >
> > > Tetsuo Handa wrote:
> > > > which will allow strcmp() to trigger out of bound read when "size" is
> > > > larger than strlen(initial_sid_to_string[i]).
> > >
> > > Oops. "smaller" than.
> > >
> > > >
> > > > 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.
On Mon, Dec 4, 2017 at 2:43 PM, Stephen Smalley <[email protected]> wrote:
> On Sun, 2017-12-03 at 20:33 +0900, Tetsuo Handa wrote:
>> On 2017/12/02 3:52, syzbot wrote:
>> > ==================================================================
>> > 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
>>
>> 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 *".
>>
>> > 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
Paul, please also follow this part:
> 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
> Note: all commands must start from beginning of the line in the email body.
This will greatly help to keep overall process running. Thanks.
On Sun, 2017-12-03 at 20:33 +0900, Tetsuo Handa wrote:
> On 2017/12/02 3:52, syzbot wrote:
> > ==================================================================
> > 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
>
> 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 *".
>
> > 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
>
> > 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
On Mon, Dec 4, 2017 at 8:47 AM, Dmitry Vyukov <[email protected]> wrote:
> On Mon, Dec 4, 2017 at 2:43 PM, Stephen Smalley <[email protected]> wrote:
>> On Sun, 2017-12-03 at 20:33 +0900, Tetsuo Handa wrote:
>>> On 2017/12/02 3:52, syzbot wrote:
>>> > ==================================================================
>>> > 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
>>>
>>> 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 *".
>>>
>>> > 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
>
>
> Paul, please also follow this part:
>
>> 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
>> 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?
--
paul moore
security @ redhat
Stephen Smalley wrote:
> > 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.
On Mon, Dec 4, 2017 at 2:59 PM, Paul Moore <[email protected]> wrote:
>>>> > ==================================================================
>>>> > 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
>>>>
>>>> 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 *".
>>>>
>>>> > 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
>>
>>
>> Paul, please also follow this part:
>>
>>> 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
>>> 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
On Mon, Dec 4, 2017 at 2:59 PM, Paul Moore <[email protected]> wrote:
>>>> On 2017/12/02 3:52, syzbot wrote:
>>>> > ==================================================================
>>>> > 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
>>>>
>>>> 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 *".
>>>>
>>>> > 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
>>
>>
>> Paul, please also follow this part:
>>
>>> 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
>>> 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
On Mon, 2017-12-04 at 17:39 +0100, Dmitry Vyukov wrote:
> On Mon, Dec 4, 2017 at 2:59 PM, Paul Moore <[email protected]> wrote:
> > > > > On 2017/12/02 3:52, syzbot wrote:
> > > > > > ===========================================================
> > > > > > =======
> > > > > > 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
> > > > >
> > > > > 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 *".
> > > > >
> > > > > > 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
> > >
> > >
> > > Paul, please also follow this part:
> > >
> > > > 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
> > > > 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?
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.
On Mon, Dec 4, 2017 at 11:29 AM, Dmitry Vyukov <[email protected]> wrote:
> On Mon, Dec 4, 2017 at 2:59 PM, Paul Moore <[email protected]> wrote:
> 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.
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.
>> 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.
>> 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
On Mon, Dec 4, 2017 at 10:10 PM, Paul Moore <[email protected]> wrote:
>> 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.
>
> 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.
>
>>> 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.
> 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.
>>> 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
On Mon, Dec 4, 2017 at 6:33 PM, Stephen Smalley <[email protected]> wrote:
> On Mon, 2017-12-04 at 17:39 +0100, Dmitry Vyukov wrote:
>> On Mon, Dec 4, 2017 at 2:59 PM, Paul Moore <[email protected]> wrote:
>> > > > > On 2017/12/02 3:52, syzbot wrote:
>> > > > > > ===========================================================
>> > > > > > =======
>> > > > > > 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
>> > > > >
>> > > > > 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 *".
>> > > > >
>> > > > > > 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
>> > >
>> > >
>> > > Paul, please also follow this part:
>> > >
>> > > > 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
>> > > > 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?
>
> 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.
On Tue, Dec 5, 2017 at 11:00 AM, Dmitry Vyukov <[email protected]> wrote:
>>> > > > > > ===========================================================
>>> > > > > > =======
>>> > > > > > 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
>>> > > > >
>>> > > > > 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 *".
>>> > > > >
>>> > > > > > 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
>>> > >
>>> > >
>>> > > Paul, please also follow this part:
>>> > >
>>> > > > 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
>>> > > > 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?
>>
>> 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.
On Mon, Dec 4, 2017 at 10:10 PM, Paul Moore <[email protected]> wrote:
>> 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.
>
> 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
>>> 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.
>
>>> 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