KASAN: slab-out-of-bounds Read in strcmp

KASAN: slab-out-of-bounds Read in strcmp

[Tetsuo Handa]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Tetsuo Handa]

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 <penguin-kernel@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-kernel@I-love.SAKURA.ne.jp>
Reported-by: syzbot <syzkaller@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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info@ http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[James Morris]

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
<james.l.morris@oracle.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Tetsuo Handa]

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
----------
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Tetsuo Handa]

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Dmitry Vyukov]

On Sun, Dec 3, 2017 at 2:27 PM, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> 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 <penguin-kernel@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-kernel@I-love.SAKURA.ne.jp>
> Reported-by: syzbot <syzkaller@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
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe at googlegroups.com.
> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Stephen Smalley]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Dmitry Vyukov]

On Mon, Dec 4, 2017 at 2:43 PM, Stephen Smalley <sds@tycho.nsa.gov> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Paul Moore]

On Mon, Dec 4, 2017 at 8:47 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Mon, Dec 4, 2017 at 2:43 PM, Stephen Smalley <sds@tycho.nsa.gov> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Dmitry Vyukov]

On Mon, Dec 4, 2017 at 2:59 PM, Paul Moore <pmoore@redhat.com> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Paul Moore]

On Mon, Dec 4, 2017 at 11:29 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Mon, Dec 4, 2017 at 2:59 PM, Paul Moore <pmoore@redhat.com> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Dmitry Vyukov]

On Mon, Dec 4, 2017 at 10:10 PM, Paul Moore <pmoore@redhat.com> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Dmitry Vyukov]

On Mon, Dec 4, 2017 at 10:10 PM, Paul Moore <pmoore@redhat.com> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Dmitry Vyukov]

On Mon, Dec 4, 2017 at 2:59 PM, Paul Moore <pmoore@redhat.com> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Stephen Smalley]

On Mon, 2017-12-04 at 17:39 +0100, Dmitry Vyukov wrote:
> On Mon, Dec 4, 2017 at 2:59 PM, Paul Moore <pmoore@redhat.com> 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.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Dmitry Vyukov]

On Mon, Dec 4, 2017 at 6:33 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Mon, 2017-12-04 at 17:39 +0100, Dmitry Vyukov wrote:
>> On Mon, Dec 4, 2017 at 2:59 PM, Paul Moore <pmoore@redhat.com> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Dmitry Vyukov]

On Tue, Dec 5, 2017 at 11:00 AM, Dmitry Vyukov <dvyukov@google.com> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

KASAN: slab-out-of-bounds Read in strcmp

[Tetsuo Handa]

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html