* Re: [PATCH v2 1/3] security: Create "kernel hardening" config area
From: Alexander Popov @ 2019-04-15 16:44 UTC (permalink / raw)
To: Kees Cook, Alexander Potapenko, Masahiro Yamada, James Morris
Cc: Nick Desaulniers, Kostya Serebryany, Dmitry Vyukov, Sandeep Patil,
Laura Abbott, Randy Dunlap, Michal Marek, Emese Revfy,
Serge E. Hallyn, kernel-hardening, linux-security-module,
linux-kbuild, linux-kernel
In-Reply-To: <20190411180117.27704-2-keescook@chromium.org>
On 11.04.2019 21:01, Kees Cook wrote:
> Right now kernel hardening options are scattered around various Kconfig
> files. This can be a central place to collect these kinds of options
> going forward. This is initially populated with the memory initialization
> options from the gcc-plugins.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Hello Kees, hello everyone!
After applying this series the kernel config looks like that:
...
...
CONFIG_LSM="yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
#
# Kernel hardening options
#
#
# Memory initialization
#
CONFIG_INIT_STACK_NONE=y
# CONFIG_GCC_PLUGIN_STRUCTLEAK_USER is not set
# CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF is not set
# CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL is not set
# CONFIG_GCC_PLUGIN_STACKLEAK is not set
CONFIG_CRYPTO=y
#
# Crypto core or helper
#
CONFIG_CRYPTO_ALGAPI=y
...
...
What do you think about some separator between memory initialization options and
CONFIG_CRYPTO?
Best regards,
Alexander
^ permalink raw reply
* [PULL] Smack: Changes for 5.2
From: Casey Schaufler @ 2019-04-15 16:27 UTC (permalink / raw)
To: James Morris, Linux Security Module list; +Cc: casey
James, please take these updates for 5.2. There's one bug fix for
IPv6 handling and two memory use improvements.
The following changes since commit 9d7b7bfbafba5e6cad609f1188243a7f0cd0d293:
Merge tag 'v5.1-rc2' into next-general (2019-03-26 16:28:01 -0700)
are available in the Git repository at:
https://github.com/cschaufler/next-smack smack-for-5.2
for you to fetch changes up to f7450bc6e76860564f3842a41892f9b74313cc23:
Smack: Fix IPv6 handling of 0 secmark (2019-04-03 14:28:38 -0700)
----------------------------------------------------------------
Casey Schaufler (2):
Smack: Create smack_rule cache to optimize memory usage
Smack: Fix IPv6 handling of 0 secmark
Vishal Goel (1):
smack: removal of global rule list
security/smack/smack.h | 1 +
security/smack/smack_lsm.c | 13 +++++++++--
security/smack/smackfs.c | 55 ++++++++++++++--------------------------------
3 files changed, 28 insertions(+), 41 deletions(-)
^ permalink raw reply
* Re: kernel BUG at kernel/cred.c:434!
From: Paul Moore @ 2019-04-15 16:20 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Casey Schaufler, chengjian (D), neilb, Anna.Schumaker, keescook,
linux-kernel@vger.kernel.org, viro, Xiexiuqi (Xie XiuQi), Li Bin,
yanaijie, peterz, mingo, Linux Security Module list, selinux
In-Reply-To: <20190415150520.GA13257@redhat.com>
On Mon, Apr 15, 2019 at 11:05 AM Oleg Nesterov <oleg@redhat.com> wrote:
> On 04/15, Paul Moore wrote:
> >
> > On Mon, Apr 15, 2019 at 9:43 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > > Well, acct("/proc/self/attr/current") doesn't look like a good idea, but I do
> > > not know where should we put the additional check... And probably
> > > "echo /proc/self/attr/current > /proc/sys/kernel/core_pattern" can hit the
> > > same problem, do_coredump() does override_creds() too.
> > >
> > > May be just add
> > >
> > > if (current->cred != current->real_cred)
> > > return -EACCES;
> > >
> > > into proc_pid_attr_write(), I dunno.
> >
> > Is the problem that do_acct_process() is calling override_creds() and
> > the returned/old credentials are being freed before do_acct_process()
> > can reinstall the creds via revert_creds()? Presumably because the
> > process accounting is causing the credentials to be replaced?
>
> Afaics, the problem is that do_acct_process() does override_creds() and
> then __kernel_write(). Which calls proc_pid_attr_write(), which in turn calls
> selinux_setprocattr(), which does another prepare_creds() + commit_creds();
> and commit_creds() hits
>
> BUG_ON(task->cred != old);
Gotcha. In the process of looking at the backtrace I forgot about the
BUG_ON() at the top of the oops message.
I wonder what terrible things would happen if we changed the BUG_ON()
in commit_creds to simple returning an error an error code to the
caller. There is a warning/requirement in commit_creds() function
header comment that it should always return 0.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: kernel BUG at kernel/cred.c:434!
From: Oleg Nesterov @ 2019-04-15 15:05 UTC (permalink / raw)
To: Paul Moore
Cc: Casey Schaufler, chengjian (D), neilb, Anna.Schumaker, keescook,
linux-kernel@vger.kernel.org, viro, Xiexiuqi (Xie XiuQi), Li Bin,
yanaijie, peterz, mingo, Linux Security Module list, selinux
In-Reply-To: <CAHC9VhQ7ihqU0K57ctwzoHq0t0QVJPKiQQZn5TXB1FUqsvOSEQ@mail.gmail.com>
On 04/15, Paul Moore wrote:
>
> On Mon, Apr 15, 2019 at 9:43 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > Well, acct("/proc/self/attr/current") doesn't look like a good idea, but I do
> > not know where should we put the additional check... And probably
> > "echo /proc/self/attr/current > /proc/sys/kernel/core_pattern" can hit the
> > same problem, do_coredump() does override_creds() too.
> >
> > May be just add
> >
> > if (current->cred != current->real_cred)
> > return -EACCES;
> >
> > into proc_pid_attr_write(), I dunno.
>
> Is the problem that do_acct_process() is calling override_creds() and
> the returned/old credentials are being freed before do_acct_process()
> can reinstall the creds via revert_creds()? Presumably because the
> process accounting is causing the credentials to be replaced?
Afaics, the problem is that do_acct_process() does override_creds() and
then __kernel_write(). Which calls proc_pid_attr_write(), which in turn calls
selinux_setprocattr(), which does another prepare_creds() + commit_creds();
and commit_creds() hits
BUG_ON(task->cred != old);
Oleg.
^ permalink raw reply
* Re: kernel BUG at kernel/cred.c:434!
From: Paul Moore @ 2019-04-15 14:48 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Casey Schaufler, chengjian (D), neilb, Anna.Schumaker, keescook,
linux-kernel@vger.kernel.org, viro, Xiexiuqi (Xie XiuQi), Li Bin,
yanaijie, peterz, mingo, Linux Security Module list, selinux
In-Reply-To: <20190415134331.GC22204@redhat.com>
On Mon, Apr 15, 2019 at 9:43 AM Oleg Nesterov <oleg@redhat.com> wrote:
> Well, acct("/proc/self/attr/current") doesn't look like a good idea, but I do
> not know where should we put the additional check... And probably
> "echo /proc/self/attr/current > /proc/sys/kernel/core_pattern" can hit the
> same problem, do_coredump() does override_creds() too.
>
> May be just add
>
> if (current->cred != current->real_cred)
> return -EACCES;
>
> into proc_pid_attr_write(), I dunno.
Is the problem that do_acct_process() is calling override_creds() and
the returned/old credentials are being freed before do_acct_process()
can reinstall the creds via revert_creds()? Presumably because the
process accounting is causing the credentials to be replaced?
> On 04/12, Casey Schaufler wrote:
> >
> > On 4/11/2019 11:21 PM, chengjian (D) wrote:
> >
> > Added LSM and SELinux lists.
> >
> >
> > >Hi.
> > >
> > >
> > >syzkaller reported the following BUG:
> > >
> > >[ 73.146973] kernel BUG at kernel/cred.c:434!
> > >[ 73.150231] invalid opcode: 0000 [#1] SMP KASAN PTI
> > >[ 73.151928] CPU: 2 PID: 4058 Comm: syz-executor.6 Not tainted
> > >5.1.0-rc4-00062-g2d06b235815e-dirty #2
> > >[ 73.155174] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > >rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
> > >[ 73.159798] RIP: 0010:commit_creds+0xadb/0xe50
> > >[ 73.161426] Code: 8b 5b 20 48 c1 ea 03 0f b6 04 02 84 c0 74 08 3c 03 0f
> > >8e 06 03 00 00 39 5d 20 0f 85 ff fa ff ff e9 0c fb ff ff e8 05 a2 25 00
> > ><0f> 0b 48 c7 c7 80 56 c0 83 e8 95 22 b1 00 e8 f2 a1 25 00 0f 0b 48
> > >[ 73.167852] RSP: 0000:ffff88836e65f5d0 EFLAGS: 00010293
> > >[ 73.169636] RAX: ffff8883767b0000 RBX: ffff88837f111300 RCX:
> > >ffffffff8124b5db
> > >[ 73.171962] RDX: 0000000000000000 RSI: ffffffff83c9b140 RDI:
> > >ffff88837f111300
> > >[ 73.174310] RBP: ffff888376610400 R08: 0000000000000000 R09:
> > >0000000000000004
> > >[ 73.176646] R10: 0000000000000001 R11: ffffed107c655acf R12:
> > >ffff8883767b0000
> > >[ 73.178527] Process accounting resumed
> > >[ 73.179021] R13: ffff88837f111900 R14: ffff88837f111300 R15:
> > >ffff8883767b0ac0
> > >[ 73.179029] FS: 00007f2d207f9700(0000) GS:ffff8883e3280000(0000)
> > >knlGS:0000000000000000
> > >[ 73.179034] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > >[ 73.179039] CR2: 00007f1500bd36c0 CR3: 00000003df304003 CR4:
> > >00000000000206e0
> > >[ 73.179047] Call Trace:
> > >[ 73.190461] selinux_setprocattr+0x2ea/0x8f0
> > >[ 73.191925] ? ptrace_parent_sid+0x530/0x530
> > >[ 73.193436] ? proc_pid_attr_write+0x185/0x5a0
> > >[ 73.194967] security_setprocattr+0xa1/0x100
> > >[ 73.196408] proc_pid_attr_write+0x307/0x5a0
> > >[ 73.197869] ? mem_read+0x40/0x40
> > >[ 73.199013] __vfs_write+0x81/0x100
> > >[ 73.200222] __kernel_write+0xf8/0x330
> > >[ 73.201562] do_acct_process+0xca5/0x1340
> > >[ 73.202969] ? __ia32_sys_acct+0x1e0/0x1e0
> > >[ 73.204498] ? find_held_lock+0x2f/0x1e0
> > >[ 73.205857] ? rcu_irq_exit+0xec/0x2c0
> > >[ 73.207160] ? lock_downgrade+0x630/0x630
> > >[ 73.208541] acct_pin_kill+0x63/0x150
> > >[ 73.209816] pin_kill+0x16d/0x7c0
> > >[ 73.210934] ? lockdep_hardirqs_on+0x5e0/0x5e0
> > >[ 73.212452] ? xas_start+0x155/0x510
> > >[ 73.213705] ? pin_insert+0x50/0x50
> > >[ 73.214903] ? finish_wait+0x270/0x270
> > >[ 73.216213] ? cpumask_next+0x57/0x90
> > >[ 73.217442] ? mnt_pin_kill+0x68/0x1d0
> > >[ 73.218851] mnt_pin_kill+0x68/0x1d0
> > >[ 73.220398] cleanup_mnt+0x11b/0x150
> > >[ 73.221970] task_work_run+0x136/0x1b0
> > >[ 73.223427] do_exit+0x830/0x2ca0
> > >[ 73.224586] ? trace_hardirqs_off+0x3b/0x180
> > >[ 73.226088] ? mm_update_next_owner+0x6a0/0x6a0
> > >[ 73.227622] ? find_held_lock+0x2f/0x1e0
> > >[ 73.228954] ? get_signal+0x2cf/0x1c00
> > >[ 73.230236] ? lock_downgrade+0x630/0x630
> > >[ 73.231628] ? rwlock_bug.part.0+0x90/0x90
> > >[ 73.233020] do_group_exit+0x106/0x2f0
> > >[ 73.234330] get_signal+0x325/0x1c00
> > >[ 73.235571] do_signal+0x97/0x1670
> > >[ 73.236739] ? do_send_specific+0x12d/0x220
> > >[ 73.238213] ? lock_downgrade+0x630/0x630
> > >[ 73.239566] ? setup_sigcontext+0x820/0x820
> > >[ 73.240982] ? check_kill_permission+0x4a/0x510
> > >[ 73.242509] ? do_send_specific+0x156/0x220
> > >[ 73.243905] ? do_tkill+0x1c4/0x260
> > >[ 73.245081] ? do_send_specific+0x220/0x220
> > >[ 73.246514] ? trace_hardirqs_on_thunk+0x1a/0x1c
> > >[ 73.248061] ? exit_to_usermode_loop+0x97/0x1d0
> > >[ 73.249619] exit_to_usermode_loop+0x108/0x1d0
> > >[ 73.251129] do_syscall_64+0x461/0x580
> > >[ 73.252454] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > >[ 73.254219] RIP: 0033:0x462eb9
> > >[ 73.255327] Code: Bad RIP value.
> > >[ 73.256539] RSP: 002b:00007f2d207f8c58 EFLAGS: 00000246 ORIG_RAX:
> > >00000000000000c8
> > >[ 73.259454] RAX: 0000000000000000 RBX: 000000000073bf00 RCX:
> > >0000000000462eb9
> > >[ 73.262309] RDX: 0000000000000000 RSI: 000000000000001e RDI:
> > >0000000000000005
> > >[ 73.265064] RBP: 0000000000000002 R08: 0000000000000000 R09:
> > >0000000000000000
> > >[ 73.267774] R10: 0000000000000000 R11: 0000000000000246 R12:
> > >00007f2d207f96bc
> > >[ 73.270546] R13: 00000000004c5509 R14: 00000000007042f0 R15:
> > >00000000ffffffff
> > >[ 73.273542] Modules linked in:
> > >[ 73.274670] Dumping ftrace buffer:
> > >[ 73.275852] (ftrace buffer empty)
> > >[ 73.277187] ---[ end trace dde36a95f458175d ]---
> > >[ 73.278834] RIP: 0010:commit_creds+0xadb/0xe50
> > >[ 73.280549] Code: 8b 5b 20 48 c1 ea 03 0f b6 04 02 84 c0 74 08 3c 03 0f
> > >8e 06 03 00 00 39 5d 20 0f 85 ff fa ff ff e9 0c fb ff ff e8 05 a2 25 00
> > ><0f> 0b 48 c7 c7 80 56 c0 83 e8 95 22 b1 00 e8 f2 a1 25 00 0f 0b 48
> > >[ 73.287090] RSP: 0000:ffff88836e65f5d0 EFLAGS: 00010293
> > >[ 73.288917] RAX: ffff8883767b0000 RBX: ffff88837f111300 RCX:
> > >ffffffff8124b5db
> > >[ 73.291390] RDX: 0000000000000000 RSI: ffffffff83c9b140 RDI:
> > >ffff88837f111300
> > >[ 73.293864] RBP: ffff888376610400 R08: 0000000000000000 R09:
> > >0000000000000004
> > >[ 73.296370] R10: 0000000000000001 R11: ffffed107c655acf R12:
> > >ffff8883767b0000
> > >[ 73.299275] R13: ffff88837f111900 R14: ffff88837f111300 R15:
> > >ffff8883767b0ac0
> > >[ 73.301351] Process accounting resumed
> > >[ 73.301822] FS: 00007f2d207f9700(0000) GS:ffff8883e3280000(0000)
> > >knlGS:0000000000000000
> > >[ 73.301827] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > >[ 73.301832] CR2: 0000000000462e8f CR3: 0000000003a18002 CR4:
> > >00000000000206e0
> > >[ 73.301850] Kernel panic - not syncing: Fatal exception
> > >[ 73.310719] Process accounting resumed
> > >[ 73.311515] Process accounting resumed
> > >[ 73.318916] Dumping ftrace buffer:
> > >[ 73.318921] (ftrace buffer empty)
> > >[ 73.318945] Kernel Offset: disabled
> > >[ 73.328061] Rebooting in 10 seconds..
> > >
> > >
> > >425 int commit_creds(struct cred *new)
> > >426 {
> > >427 struct task_struct *task = current;
> > >428 const struct cred *old = task->real_cred;
> > >429
> > >430 kdebug("commit_creds(%p{%d,%d})", new,
> > >431 atomic_read(&new->usage),
> > >432 read_cred_subscribers(new));
> > >433
> > >434 BUG_ON(task->cred != old); // BUG here
> > >
> > >
> > >I find that the call chain which triggered the BUG is :
> > >
> > >do_exit
> > > |-=> acct_process
> > > | -=> do_acct_process
> > > | -=> orig_cred = override_creds(file->f_cred); // cred =
> > >ffff8883c1878900/real_cred = ffff8883c1878900
> > > | -=> if (file_start_write_trylock(file))
> > >{__kernel_write(file, &ac, sizeof(acct_t), &pos);}
> > > | -=> __kernel_write+0xf8/0x330
> > > | -=> __vfs_write+0x81/0x100
> > > | -=> proc_pid_attr_write+0x307/0x5a0
> > > | -=> security_setprocattr+0xa1/0x100
> > > | -=>selinux_setprocattr+0x2ea/0x8f0
> > > | -=>commit_creds+0xd97/0x1080 // cred = ffff888379a79c00/real_cred =
> > >ffff888379a79c00
> > > | -=> revert_creds(orig_cred); // cred =
> > >ffff8883c1878900/real_cred = ffff888379a79c00
> > > |-=> task_work_run
> > > -=> cleanup_mnt+0x11b/0x150
> > > -=> mnt_pin_kill+0x68/0x1d0
> > > -=> pin_kill+0x16d/0x7c0
> > > -=> acct_pin_kill+0x2e/0x100
> > > -=> do_acct_process+0x1a0/0x1340
> > > -=> override_creds+0x18a/0x1c0 // cred =
> > >ffff8883c1878900/real_cred = ffff888379a79c00
> > > -=> if (file_start_write_trylock(file))
> > >{__kernel_write(file, &ac, sizeof(acct_t), &pos);}
> > > -=> ......
> > >-=>commit_creds+0xd97/0x1080 // new = ffff888379a6bb00, cred =
> > >ffff8883c1878900, real_ceed = ffff888379a79c00 BUG here
> > > -=> revert_creds(orig_cred);
> > >
> > >
> > >Syzkaller Report Testcase:
> > >
> > >cat crash.log
> > >
> > >04:05:28 executing program 3:
> > >clone(0x24100, 0x0, 0x0, 0x0, 0x0)
> > >r0 = gettid()
> > >perf_event_open(&(0x7f00000000c0)={0x2, 0x70, 0x4, 0x2, 0x0, 0x0, 0x0,
> > >0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> > >0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> > >0x0, 0x0, 0x0, 0x0, @perf_bp={0x0}}, 0x0, 0xffffffffffffffff,
> > >0xffffffffffffffff, 0x0)
> > >acct(&(0x7f0000000000)='./file0\x00')
> > >r1 = openat$smack_thread_current(0xffffffffffffff9c,
> > >&(0x7f0000000000)='/proc/thread-self/attr/current\x00', 0x2, 0x0)
> > >fcntl$setown(r1, 0x8, r0)
> > >rt_tgsigqueueinfo(r0, r0, 0x1e, &(0x7f00000002c0))
> > >tkill(r0, 0x1e)
> > >
> > >
> > >Reproduce this BUG:
> > >./syz-execprog -executor=./syz-executor -repeat=0 -procs=16 -cover=0
> > >./crash.log
> > >
> > >
> > >Can anyone help me ?
> > >
> > >
> > >Thanks,
> > >
> > > Cheng Jian.
> > >
> > >
>
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: kernel BUG at kernel/cred.c:434!
From: Oleg Nesterov @ 2019-04-15 13:43 UTC (permalink / raw)
To: Casey Schaufler
Cc: chengjian (D), neilb, Anna.Schumaker, keescook,
linux-kernel@vger.kernel.org, viro, Xiexiuqi (Xie XiuQi), Li Bin,
yanaijie, peterz, mingo, Linux Security Module list, selinux
In-Reply-To: <b1575d02-fd88-65e6-2f16-70a96985087e@schaufler-ca.com>
Well, acct("/proc/self/attr/current") doesn't look like a good idea, but I do
not know where should we put the additional check... And probably
"echo /proc/self/attr/current > /proc/sys/kernel/core_pattern" can hit the
same problem, do_coredump() does override_creds() too.
May be just add
if (current->cred != current->real_cred)
return -EACCES;
into proc_pid_attr_write(), I dunno.
On 04/12, Casey Schaufler wrote:
>
> On 4/11/2019 11:21 PM, chengjian (D) wrote:
>
> Added LSM and SELinux lists.
>
>
> >Hi.
> >
> >
> >syzkaller reported the following BUG:
> >
> >[ 73.146973] kernel BUG at kernel/cred.c:434!
> >[ 73.150231] invalid opcode: 0000 [#1] SMP KASAN PTI
> >[ 73.151928] CPU: 2 PID: 4058 Comm: syz-executor.6 Not tainted
> >5.1.0-rc4-00062-g2d06b235815e-dirty #2
> >[ 73.155174] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> >rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
> >[ 73.159798] RIP: 0010:commit_creds+0xadb/0xe50
> >[ 73.161426] Code: 8b 5b 20 48 c1 ea 03 0f b6 04 02 84 c0 74 08 3c 03 0f
> >8e 06 03 00 00 39 5d 20 0f 85 ff fa ff ff e9 0c fb ff ff e8 05 a2 25 00
> ><0f> 0b 48 c7 c7 80 56 c0 83 e8 95 22 b1 00 e8 f2 a1 25 00 0f 0b 48
> >[ 73.167852] RSP: 0000:ffff88836e65f5d0 EFLAGS: 00010293
> >[ 73.169636] RAX: ffff8883767b0000 RBX: ffff88837f111300 RCX:
> >ffffffff8124b5db
> >[ 73.171962] RDX: 0000000000000000 RSI: ffffffff83c9b140 RDI:
> >ffff88837f111300
> >[ 73.174310] RBP: ffff888376610400 R08: 0000000000000000 R09:
> >0000000000000004
> >[ 73.176646] R10: 0000000000000001 R11: ffffed107c655acf R12:
> >ffff8883767b0000
> >[ 73.178527] Process accounting resumed
> >[ 73.179021] R13: ffff88837f111900 R14: ffff88837f111300 R15:
> >ffff8883767b0ac0
> >[ 73.179029] FS: 00007f2d207f9700(0000) GS:ffff8883e3280000(0000)
> >knlGS:0000000000000000
> >[ 73.179034] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >[ 73.179039] CR2: 00007f1500bd36c0 CR3: 00000003df304003 CR4:
> >00000000000206e0
> >[ 73.179047] Call Trace:
> >[ 73.190461] selinux_setprocattr+0x2ea/0x8f0
> >[ 73.191925] ? ptrace_parent_sid+0x530/0x530
> >[ 73.193436] ? proc_pid_attr_write+0x185/0x5a0
> >[ 73.194967] security_setprocattr+0xa1/0x100
> >[ 73.196408] proc_pid_attr_write+0x307/0x5a0
> >[ 73.197869] ? mem_read+0x40/0x40
> >[ 73.199013] __vfs_write+0x81/0x100
> >[ 73.200222] __kernel_write+0xf8/0x330
> >[ 73.201562] do_acct_process+0xca5/0x1340
> >[ 73.202969] ? __ia32_sys_acct+0x1e0/0x1e0
> >[ 73.204498] ? find_held_lock+0x2f/0x1e0
> >[ 73.205857] ? rcu_irq_exit+0xec/0x2c0
> >[ 73.207160] ? lock_downgrade+0x630/0x630
> >[ 73.208541] acct_pin_kill+0x63/0x150
> >[ 73.209816] pin_kill+0x16d/0x7c0
> >[ 73.210934] ? lockdep_hardirqs_on+0x5e0/0x5e0
> >[ 73.212452] ? xas_start+0x155/0x510
> >[ 73.213705] ? pin_insert+0x50/0x50
> >[ 73.214903] ? finish_wait+0x270/0x270
> >[ 73.216213] ? cpumask_next+0x57/0x90
> >[ 73.217442] ? mnt_pin_kill+0x68/0x1d0
> >[ 73.218851] mnt_pin_kill+0x68/0x1d0
> >[ 73.220398] cleanup_mnt+0x11b/0x150
> >[ 73.221970] task_work_run+0x136/0x1b0
> >[ 73.223427] do_exit+0x830/0x2ca0
> >[ 73.224586] ? trace_hardirqs_off+0x3b/0x180
> >[ 73.226088] ? mm_update_next_owner+0x6a0/0x6a0
> >[ 73.227622] ? find_held_lock+0x2f/0x1e0
> >[ 73.228954] ? get_signal+0x2cf/0x1c00
> >[ 73.230236] ? lock_downgrade+0x630/0x630
> >[ 73.231628] ? rwlock_bug.part.0+0x90/0x90
> >[ 73.233020] do_group_exit+0x106/0x2f0
> >[ 73.234330] get_signal+0x325/0x1c00
> >[ 73.235571] do_signal+0x97/0x1670
> >[ 73.236739] ? do_send_specific+0x12d/0x220
> >[ 73.238213] ? lock_downgrade+0x630/0x630
> >[ 73.239566] ? setup_sigcontext+0x820/0x820
> >[ 73.240982] ? check_kill_permission+0x4a/0x510
> >[ 73.242509] ? do_send_specific+0x156/0x220
> >[ 73.243905] ? do_tkill+0x1c4/0x260
> >[ 73.245081] ? do_send_specific+0x220/0x220
> >[ 73.246514] ? trace_hardirqs_on_thunk+0x1a/0x1c
> >[ 73.248061] ? exit_to_usermode_loop+0x97/0x1d0
> >[ 73.249619] exit_to_usermode_loop+0x108/0x1d0
> >[ 73.251129] do_syscall_64+0x461/0x580
> >[ 73.252454] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> >[ 73.254219] RIP: 0033:0x462eb9
> >[ 73.255327] Code: Bad RIP value.
> >[ 73.256539] RSP: 002b:00007f2d207f8c58 EFLAGS: 00000246 ORIG_RAX:
> >00000000000000c8
> >[ 73.259454] RAX: 0000000000000000 RBX: 000000000073bf00 RCX:
> >0000000000462eb9
> >[ 73.262309] RDX: 0000000000000000 RSI: 000000000000001e RDI:
> >0000000000000005
> >[ 73.265064] RBP: 0000000000000002 R08: 0000000000000000 R09:
> >0000000000000000
> >[ 73.267774] R10: 0000000000000000 R11: 0000000000000246 R12:
> >00007f2d207f96bc
> >[ 73.270546] R13: 00000000004c5509 R14: 00000000007042f0 R15:
> >00000000ffffffff
> >[ 73.273542] Modules linked in:
> >[ 73.274670] Dumping ftrace buffer:
> >[ 73.275852] (ftrace buffer empty)
> >[ 73.277187] ---[ end trace dde36a95f458175d ]---
> >[ 73.278834] RIP: 0010:commit_creds+0xadb/0xe50
> >[ 73.280549] Code: 8b 5b 20 48 c1 ea 03 0f b6 04 02 84 c0 74 08 3c 03 0f
> >8e 06 03 00 00 39 5d 20 0f 85 ff fa ff ff e9 0c fb ff ff e8 05 a2 25 00
> ><0f> 0b 48 c7 c7 80 56 c0 83 e8 95 22 b1 00 e8 f2 a1 25 00 0f 0b 48
> >[ 73.287090] RSP: 0000:ffff88836e65f5d0 EFLAGS: 00010293
> >[ 73.288917] RAX: ffff8883767b0000 RBX: ffff88837f111300 RCX:
> >ffffffff8124b5db
> >[ 73.291390] RDX: 0000000000000000 RSI: ffffffff83c9b140 RDI:
> >ffff88837f111300
> >[ 73.293864] RBP: ffff888376610400 R08: 0000000000000000 R09:
> >0000000000000004
> >[ 73.296370] R10: 0000000000000001 R11: ffffed107c655acf R12:
> >ffff8883767b0000
> >[ 73.299275] R13: ffff88837f111900 R14: ffff88837f111300 R15:
> >ffff8883767b0ac0
> >[ 73.301351] Process accounting resumed
> >[ 73.301822] FS: 00007f2d207f9700(0000) GS:ffff8883e3280000(0000)
> >knlGS:0000000000000000
> >[ 73.301827] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >[ 73.301832] CR2: 0000000000462e8f CR3: 0000000003a18002 CR4:
> >00000000000206e0
> >[ 73.301850] Kernel panic - not syncing: Fatal exception
> >[ 73.310719] Process accounting resumed
> >[ 73.311515] Process accounting resumed
> >[ 73.318916] Dumping ftrace buffer:
> >[ 73.318921] (ftrace buffer empty)
> >[ 73.318945] Kernel Offset: disabled
> >[ 73.328061] Rebooting in 10 seconds..
> >
> >
> >425 int commit_creds(struct cred *new)
> >426 {
> >427 struct task_struct *task = current;
> >428 const struct cred *old = task->real_cred;
> >429
> >430 kdebug("commit_creds(%p{%d,%d})", new,
> >431 atomic_read(&new->usage),
> >432 read_cred_subscribers(new));
> >433
> >434 BUG_ON(task->cred != old); // BUG here
> >
> >
> >I find that the call chain which triggered the BUG is :
> >
> >do_exit
> > |-=> acct_process
> > | -=> do_acct_process
> > | -=> orig_cred = override_creds(file->f_cred); // cred =
> >ffff8883c1878900/real_cred = ffff8883c1878900
> > | -=> if (file_start_write_trylock(file))
> >{__kernel_write(file, &ac, sizeof(acct_t), &pos);}
> > | -=> __kernel_write+0xf8/0x330
> > | -=> __vfs_write+0x81/0x100
> > | -=> proc_pid_attr_write+0x307/0x5a0
> > | -=> security_setprocattr+0xa1/0x100
> > | -=>selinux_setprocattr+0x2ea/0x8f0
> > | -=>commit_creds+0xd97/0x1080 // cred = ffff888379a79c00/real_cred =
> >ffff888379a79c00
> > | -=> revert_creds(orig_cred); // cred =
> >ffff8883c1878900/real_cred = ffff888379a79c00
> > |-=> task_work_run
> > -=> cleanup_mnt+0x11b/0x150
> > -=> mnt_pin_kill+0x68/0x1d0
> > -=> pin_kill+0x16d/0x7c0
> > -=> acct_pin_kill+0x2e/0x100
> > -=> do_acct_process+0x1a0/0x1340
> > -=> override_creds+0x18a/0x1c0 // cred =
> >ffff8883c1878900/real_cred = ffff888379a79c00
> > -=> if (file_start_write_trylock(file))
> >{__kernel_write(file, &ac, sizeof(acct_t), &pos);}
> > -=> ......
> >-=>commit_creds+0xd97/0x1080 // new = ffff888379a6bb00, cred =
> >ffff8883c1878900, real_ceed = ffff888379a79c00 BUG here
> > -=> revert_creds(orig_cred);
> >
> >
> >Syzkaller Report Testcase:
> >
> >cat crash.log
> >
> >04:05:28 executing program 3:
> >clone(0x24100, 0x0, 0x0, 0x0, 0x0)
> >r0 = gettid()
> >perf_event_open(&(0x7f00000000c0)={0x2, 0x70, 0x4, 0x2, 0x0, 0x0, 0x0,
> >0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> >0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> >0x0, 0x0, 0x0, 0x0, @perf_bp={0x0}}, 0x0, 0xffffffffffffffff,
> >0xffffffffffffffff, 0x0)
> >acct(&(0x7f0000000000)='./file0\x00')
> >r1 = openat$smack_thread_current(0xffffffffffffff9c,
> >&(0x7f0000000000)='/proc/thread-self/attr/current\x00', 0x2, 0x0)
> >fcntl$setown(r1, 0x8, r0)
> >rt_tgsigqueueinfo(r0, r0, 0x1e, &(0x7f00000002c0))
> >tkill(r0, 0x1e)
> >
> >
> >Reproduce this BUG:
> >./syz-execprog -executor=./syz-executor -repeat=0 -procs=16 -cover=0
> >./crash.log
> >
> >
> >Can anyone help me ?
> >
> >
> >Thanks,
> >
> > Cheng Jian.
> >
> >
^ permalink raw reply
* Re: [PATCH 1/2] apparmor: Use a memory pool instead per-CPU caches
From: Sebastian Andrzej Siewior @ 2019-04-15 10:50 UTC (permalink / raw)
To: linux-security-module, John Johansen, James Morris,
Serge E. Hallyn; +Cc: tglx
In-Reply-To: <20190405133458.4809-1-bigeasy@linutronix.de>
On 2019-04-05 15:34:57 [+0200], To linux-security-module@vger.kernel.org wrote:
> The get_buffers() macro may provide one or two buffers to the caller.
> Those buffers are preallocated on init for each CPU. By default it
> allocates
> 2* 2 * MAX_PATH * POSSIBLE_CPU
>
> which equals 64KiB on a system with 4 CPUs or 1MiB with 64 CPUs and so
> on.
>
> Replace the per-CPU buffers with a common memory pool which is shared
> across all CPUs. The pool grows on demand and never shrinks.
> By using this pool it is possible to request a buffer and keeping
> preemption enabled which avoids the hack in profile_transition().
>
> During light testing I didn't get more than two buffers in total with
> this patch. So it seems to make sense to allocate the buffers on demand
> and keep them for further use for a quick access.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
a gentle ping.
Sebastian
^ permalink raw reply
* Re: [PATCH] TCG2 log support build fixes for non-x86_64
From: Jarkko Sakkinen @ 2019-04-15 8:47 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-integrity, peterhuewe, jgg, roberto.sassu, linux-efi,
linux-security-module, linux-kernel, tweek
In-Reply-To: <20190402215556.257406-1-matthewgarrett@google.com>
On Tue, Apr 02, 2019 at 02:55:54PM -0700, Matthew Garrett wrote:
> Couple of patches to fix ktest reported issues with the crypto-agile log
> format support.
Applied and squashed. Should be soon in linux-next.
/Jarkko
^ permalink raw reply
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
From: Herbert Xu @ 2019-04-15 2:46 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Kees Cook, Eric Biggers, Rik van Riel, linux-crypto,
Dmitry Vyukov, Geert Uytterhoeven, linux-security-module,
Linux ARM, Linux Kernel Mailing List, Laura Abbott, linux-mm
In-Reply-To: <20190415022412.GA29714@bombadil.infradead.org>
On Sun, Apr 14, 2019 at 07:24:12PM -0700, Matthew Wilcox wrote:
> On Thu, Apr 11, 2019 at 01:32:32PM -0700, Kees Cook wrote:
> > > @@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
> > > int i;
> > >
> > > for (i = 0; i < XBUFSIZE; i++) {
> > > - buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
> > > + buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
> > > + order);
> >
> > Is there a reason __GFP_COMP isn't automatically included in all page
> > allocations? (Or rather, it seems like the exception is when things
> > should NOT be considered part of the same allocation, so something
> > like __GFP_SINGLE should exist?.)
>
> The question is not whether or not things should be considered part of the
> same allocation. The question is whether the allocation is of a compound
> page or of N consecutive pages. Now you're asking what the difference is,
> and it's whether you need to be able to be able to call compound_head(),
> compound_order(), PageTail() or use a compound_dtor. If you don't, then
> you can save some time at allocation & free by not specifying __GFP_COMP.
Thanks for clarifying Matthew.
Eric, this means that we should not use __GFP_COMP here just to
silent what is clearly a broken warning.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
From: Matthew Wilcox @ 2019-04-15 2:24 UTC (permalink / raw)
To: Kees Cook
Cc: Eric Biggers, Rik van Riel, linux-crypto, Herbert Xu,
Dmitry Vyukov, Geert Uytterhoeven, linux-security-module,
Linux ARM, Linux Kernel Mailing List, Laura Abbott, linux-mm
In-Reply-To: <CAGXu5jJ8k7fP5Vb=ygmQ0B45GfrK2PeaV04bPWmcZ6Vb+swgyA@mail.gmail.com>
On Thu, Apr 11, 2019 at 01:32:32PM -0700, Kees Cook wrote:
> > @@ -156,7 +156,8 @@ static int __testmgr_alloc_buf(char *buf[XBUFSIZE], int order)
> > int i;
> >
> > for (i = 0; i < XBUFSIZE; i++) {
> > - buf[i] = (char *)__get_free_pages(GFP_KERNEL, order);
> > + buf[i] = (char *)__get_free_pages(GFP_KERNEL | __GFP_COMP,
> > + order);
>
> Is there a reason __GFP_COMP isn't automatically included in all page
> allocations? (Or rather, it seems like the exception is when things
> should NOT be considered part of the same allocation, so something
> like __GFP_SINGLE should exist?.)
The question is not whether or not things should be considered part of the
same allocation. The question is whether the allocation is of a compound
page or of N consecutive pages. Now you're asking what the difference is,
and it's whether you need to be able to be able to call compound_head(),
compound_order(), PageTail() or use a compound_dtor. If you don't, then
you can save some time at allocation & free by not specifying __GFP_COMP.
I'll agree this is not documented well, and maybe most multi-page
allocations do want __GFP_COMP and we should invert that bit, but
__GFP_SINGLE doesn't seem like the right antonym to __GFP_COMP to me.
^ permalink raw reply
* Re: fanotify and LSM path hooks
From: Amir Goldstein @ 2019-04-14 20:28 UTC (permalink / raw)
To: Al Viro
Cc: Jan Kara, linux-fsdevel, LSM List, Serge E. Hallyn, James Morris,
Miklos Szeredi, Matthew Bobrowski, Kentaro Takeda, Tetsuo Handa,
John Johansen
In-Reply-To: <20190414192601.GR2217@ZenIV.linux.org.uk>
On Sun, Apr 14, 2019 at 10:26 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Sun, Apr 14, 2019 at 09:51:38PM +0300, Amir Goldstein wrote:
>
> > But the truth is I would much rather that users have a way to mark
> > a subtree root and ask fanotify for events under that subtree.
> > As a matter of fact, I have some private POC patches that allow users to
> > setup a mark on a "subtree root" dentry, which really marks the super block
> > and keep a reference to the dentry. Than every event on that super block
> > is filtered with is_subdir() against the marked dentry.
>
> And that is_subdir() is protected by what, exactly? And what happens
> if you have many such dentries?
>
> Or, for that matter, what happens if that dentry gets invalidated?
Well, as I said, its just a POC, it only supports filtering by a single dentry
and I didn't think that was the way to go forward. I am looking for the best way
to go forward.
That's why I was looking for an API to "fence" renaming objects in/out of
of a subtree root. It sounds useful to have something like this for
containers and chroots.
Let's look at the options users have today.
Users can use recursive inotify that is racy and pins too many inodes in cache.
Users can use the new fanotify sb mark to get all events on filesystem and
filter them by path is userspace (also racy w.r.t ancestry).
I donno, maybe filtering by projid or another inherited persistent
inode property
is good enough for the existing use cases out there - this seems to be the way
ext4 is going with encrypted subtrees and case insensitive subtrees.
Thanks,
Amir.
^ permalink raw reply
* Re: fanotify and LSM path hooks
From: Al Viro @ 2019-04-14 19:26 UTC (permalink / raw)
To: Amir Goldstein
Cc: Jan Kara, linux-fsdevel, LSM List, Serge E. Hallyn, James Morris,
Miklos Szeredi, Matthew Bobrowski, Kentaro Takeda, Tetsuo Handa,
John Johansen
In-Reply-To: <CAOQ4uxg-NsccKfhzJbZnCnXhKA90VfVdV0_qLRt+8iVa53655g@mail.gmail.com>
On Sun, Apr 14, 2019 at 09:51:38PM +0300, Amir Goldstein wrote:
> But the truth is I would much rather that users have a way to mark
> a subtree root and ask fanotify for events under that subtree.
> As a matter of fact, I have some private POC patches that allow users to
> setup a mark on a "subtree root" dentry, which really marks the super block
> and keep a reference to the dentry. Than every event on that super block
> is filtered with is_subdir() against the marked dentry.
And that is_subdir() is protected by what, exactly? And what happens
if you have many such dentries?
Or, for that matter, what happens if that dentry gets invalidated?
^ permalink raw reply
* Re: fanotify and LSM path hooks
From: Amir Goldstein @ 2019-04-14 18:51 UTC (permalink / raw)
To: Al Viro
Cc: Jan Kara, linux-fsdevel, LSM List, Serge E. Hallyn, James Morris,
Miklos Szeredi, Matthew Bobrowski, Kentaro Takeda, Tetsuo Handa,
John Johansen
In-Reply-To: <20190414163950.GQ2217@ZenIV.linux.org.uk>
On Sun, Apr 14, 2019 at 7:40 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Sun, Apr 14, 2019 at 07:04:14PM +0300, Amir Goldstein wrote:
>
> > Another problem is that they seem to be bypassed by several subsystems.
> > cachefiles, ecryptfs, overlayfs and nfsd all call the vfs_rename() helper, but
> > only cachefiles bothers to call the security_path_rename() hook.
> > This is of course true for all other security_path_ hooks.
> > I think that is something that requires fixing regardless of the fanotify pre
> > modification hooks. I wonder if tomoyo and apparmor developers
> > (LSM that implement security_path_ hooks) are aware of those missing
> > hooks?
>
> First of all, _what_ path? You do realize that there is no such thing
> as *the* pathname of dentry, right?
Yap.
> The same filesystem may be mounted
> in any number of places, some of which might be visible in a given
> namespace (including "none of them" - and you are not even guaranteed
> that they are visible in any namespace at all).
>
> It's not "bypassed", it's "inapplicable and deeply flawed in general".
I am assuming that you are referring to the security_path_ hooks in
general. I cannot speak on behalf of Tomoyo and Apparmor, but I think
I understand why you view path based security policy as flawed.
From fanotify POV, passing the path that was used to operate on
dentries to fanotify lets the users choose if they want to get events
for all operations on an inode, all operations on a specific filesystem or
all operations where the inode was accessed via a specific mount
(FAN_MARK_MOUNT).
When looking at userspace applications of kernel filesystem change
notifications, like https://facebook.github.io/watchman/
I believe what users really want is a subtree watch.
So for example, you may want to monitor your git workspace(s) for changes
on a large source tree(s) to save time on git index updates.
If you bind mount your git work tree and watch the mount for changes,
chances are nobody is messing with your source files from another
mount and that you do not have hardlinks pointing outside the git
workspace root.
But the truth is I would much rather that users have a way to mark
a subtree root and ask fanotify for events under that subtree.
As a matter of fact, I have some private POC patches that allow users to
setup a mark on a "subtree root" dentry, which really marks the super block
and keep a reference to the dentry. Than every event on that super block
is filtered with is_subdir() against the marked dentry.
I am just not convinced that is the most efficient way to implement a
subtree filter... another though I had was to filter not by subtree, but by
project id and let users worry about populating their subtrees of interest
with appropriate projids.
More comments about ideas that are deeply flawed are welcome.
Those comments would hopefully help me navigate towards the
mildly flawed ideas ;-)
Thanks,
Amir.
^ permalink raw reply
* Re: fanotify and LSM path hooks
From: Al Viro @ 2019-04-14 16:39 UTC (permalink / raw)
To: Amir Goldstein
Cc: Jan Kara, linux-fsdevel, LSM List, Serge E. Hallyn, James Morris,
Miklos Szeredi, Matthew Bobrowski, Kentaro Takeda, Tetsuo Handa,
John Johansen
In-Reply-To: <CAOQ4uxgn=YNj8cJuccx2KqxEVGZy1z3DBVYXrD=Mc7Dc=Je+-w@mail.gmail.com>
On Sun, Apr 14, 2019 at 07:04:14PM +0300, Amir Goldstein wrote:
> Another problem is that they seem to be bypassed by several subsystems.
> cachefiles, ecryptfs, overlayfs and nfsd all call the vfs_rename() helper, but
> only cachefiles bothers to call the security_path_rename() hook.
> This is of course true for all other security_path_ hooks.
> I think that is something that requires fixing regardless of the fanotify pre
> modification hooks. I wonder if tomoyo and apparmor developers
> (LSM that implement security_path_ hooks) are aware of those missing
> hooks?
First of all, _what_ path? You do realize that there is no such thing
as *the* pathname of dentry, right? The same filesystem may be mounted
in any number of places, some of which might be visible in a given
namespace (including "none of them" - and you are not even guaranteed
that they are visible in any namespace at all).
It's not "bypassed", it's "inapplicable and deeply flawed in general".
^ permalink raw reply
* fanotify and LSM path hooks
From: Amir Goldstein @ 2019-04-14 16:04 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, LSM List, Serge E. Hallyn, James Morris, Al Viro,
Miklos Szeredi, Matthew Bobrowski, Kentaro Takeda, Tetsuo Handa,
John Johansen
Hi Jan,
I started to look at directory pre-modification "permission" hooks
that were discussed on last year's LSFMM:
https://lwn.net/Articles/755277/
The two existing fanotify_perm() hooks are called
from security_file_permission() and security_file_open()
and depend on build time CONFIG_SECURITY.
If you look at how the fsnotify_perm() hooks are planted inside the
generic security hooks, one might wonder, why are fanotify permission
hooks getting a special treatment and are not registering as LSM hooks?
One benefit from an fanotify LSM, besides more generic code, would be
that fanotify permission hooks could be disabled with boot parameters.
I only bring this up because security hooks seems like the most natural
place to add pre-modify fanotify events for the purpose of maintaining
a filesystem change journal. It would be ugly to spray more fsnotify hooks
inside security hooks instead of registering an fanotify LSM, but maybe
there are downsides of registering fanotify as LSM that I am not aware of?
Another observation relates to the security_path_ hooks.
Let's take rename as an example.
LSM could implement security_path_rename() and/or security_inode_rename()
hooks and rename syscalls will end up calling both hooks.
The security_path_ hooks are more attractive for fanotify, because the path
information could be used to setup pre-modification permission mask on
mount marks and not only on filesystem/inode marks.
One problem with security_path_ hooks is that they require an extra
build time CONFIG_SECURITY_PATH.
Another problem is that they seem to be bypassed by several subsystems.
cachefiles, ecryptfs, overlayfs and nfsd all call the vfs_rename() helper, but
only cachefiles bothers to call the security_path_rename() hook.
This is of course true for all other security_path_ hooks.
I think that is something that requires fixing regardless of the fanotify pre
modification hooks. I wonder if tomoyo and apparmor developers
(LSM that implement security_path_ hooks) are aware of those missing
hooks?
Would love to get feedback about whether or not fanotify LSM sounds
like a good or bad idea and about the security_path_ hooks questions.
Thanks,
Amir.
^ permalink raw reply
* Re: [GIT PULL] linux-integrity patches for Linux 5.2
From: James Morris @ 2019-04-12 22:22 UTC (permalink / raw)
To: Mimi Zohar; +Cc: linux-integrity, linux-security-module
In-Reply-To: <1555032089.4914.4.camel@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 592 bytes --]
On Thu, 11 Apr 2019, Mimi Zohar wrote:
> Hi James,
>
> This pull request contains just three patches, the remainder are
> either included in other pull requests (eg. audit, lockdown) or will
> be upstreamed via other subsystems (eg. kselftests, Power). Included
> in this pull request is one bug fix, one documentation update, and
> extending the x86 IMA arch policy rules to coordinate the different
> kernel module signature verification methods.
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-integrity
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH 2/3] smack: Check address length before reading address family
From: Casey Schaufler @ 2019-04-12 15:32 UTC (permalink / raw)
To: Tetsuo Handa, linux-security-module
In-Reply-To: <1555066776-9758-2-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp>
On 4/12/2019 3:59 AM, Tetsuo Handa wrote:
> KMSAN will complain if valid address length passed to bind()/connect()/
> sendmsg() is shorter than sizeof("struct sockaddr"->sa_family) bytes.
>
> Also, since smk_ipv6_port_label()/smack_netlabel_send()/
> smack_ipv6host_label()/smk_ipv6_check()/smk_ipv6_port_check() are not
> checking valid address length and/or address family, make sure we check
> both. The minimal valid length in smack_socket_connect() is changed from
> sizeof(struct sockaddr_in6) bytes to SIN6_LEN_RFC2133 bytes, for it seems
> that Smack is not using "struct sockaddr_in6"->sin6_scope_id field.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> security/smack/smack_lsm.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 5c1613519d5a..4b59e4519e0c 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2805,13 +2805,17 @@ static int smack_socket_socketpair(struct socket *socka,
> *
> * Records the label bound to a port.
> *
> - * Returns 0
> + * Returns 0 on success, and error code otherwise
> */
> static int smack_socket_bind(struct socket *sock, struct sockaddr *address,
> int addrlen)
> {
> - if (sock->sk != NULL && sock->sk->sk_family == PF_INET6)
> + if (sock->sk != NULL && sock->sk->sk_family == PF_INET6) {
> + if (addrlen < SIN6_LEN_RFC2133 ||
> + address->sa_family != AF_INET6)
> + return -EINVAL;
> smk_ipv6_port_label(sock, address);
> + }
> return 0;
> }
> #endif /* SMACK_IPV6_PORT_LABELING */
> @@ -2847,12 +2851,13 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap,
>
> switch (sock->sk->sk_family) {
> case PF_INET:
> - if (addrlen < sizeof(struct sockaddr_in))
> + if (addrlen < sizeof(struct sockaddr_in) ||
> + sap->sa_family != AF_INET)
> return -EINVAL;
> rc = smack_netlabel_send(sock->sk, (struct sockaddr_in *)sap);
> break;
> case PF_INET6:
> - if (addrlen < sizeof(struct sockaddr_in6))
> + if (addrlen < SIN6_LEN_RFC2133 || sap->sa_family != AF_INET6)
> return -EINVAL;
> #ifdef SMACK_IPV6_SECMARK_LABELING
> rsp = smack_ipv6host_label(sip);
> @@ -3682,9 +3687,15 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg,
>
> switch (sock->sk->sk_family) {
> case AF_INET:
> + if (msg->msg_namelen < sizeof(struct sockaddr_in) ||
> + sip->sin_family != AF_INET)
> + return -EINVAL;
> rc = smack_netlabel_send(sock->sk, sip);
> break;
> case AF_INET6:
> + if (msg->msg_namelen < SIN6_LEN_RFC2133 ||
> + sap->sin6_family != AF_INET6)
> + return -EINVAL;
> #ifdef SMACK_IPV6_SECMARK_LABELING
> rsp = smack_ipv6host_label(sap);
> if (rsp != NULL)
^ permalink raw reply
* Re: kernel BUG at kernel/cred.c:434!
From: Casey Schaufler @ 2019-04-12 15:28 UTC (permalink / raw)
To: chengjian (D), neilb, Anna.Schumaker, keescook,
linux-kernel@vger.kernel.org, oleg, viro, Xiexiuqi (Xie XiuQi),
Li Bin, yanaijie, peterz, mingo, Linux Security Module list,
selinux
In-Reply-To: <6e4428ca-3da1-a033-08f7-a51e57503989@huawei.com>
On 4/11/2019 11:21 PM, chengjian (D) wrote:
Added LSM and SELinux lists.
> Hi.
>
>
> syzkaller reported the following BUG:
>
> [ 73.146973] kernel BUG at kernel/cred.c:434!
> [ 73.150231] invalid opcode: 0000 [#1] SMP KASAN PTI
> [ 73.151928] CPU: 2 PID: 4058 Comm: syz-executor.6 Not tainted
> 5.1.0-rc4-00062-g2d06b235815e-dirty #2
> [ 73.155174] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
> [ 73.159798] RIP: 0010:commit_creds+0xadb/0xe50
> [ 73.161426] Code: 8b 5b 20 48 c1 ea 03 0f b6 04 02 84 c0 74 08 3c
> 03 0f 8e 06 03 00 00 39 5d 20 0f 85 ff fa ff ff e9 0c fb ff ff e8 05
> a2 25 00 <0f> 0b 48 c7 c7 80 56 c0 83 e8 95 22 b1 00 e8 f2 a1 25 00 0f
> 0b 48
> [ 73.167852] RSP: 0000:ffff88836e65f5d0 EFLAGS: 00010293
> [ 73.169636] RAX: ffff8883767b0000 RBX: ffff88837f111300 RCX:
> ffffffff8124b5db
> [ 73.171962] RDX: 0000000000000000 RSI: ffffffff83c9b140 RDI:
> ffff88837f111300
> [ 73.174310] RBP: ffff888376610400 R08: 0000000000000000 R09:
> 0000000000000004
> [ 73.176646] R10: 0000000000000001 R11: ffffed107c655acf R12:
> ffff8883767b0000
> [ 73.178527] Process accounting resumed
> [ 73.179021] R13: ffff88837f111900 R14: ffff88837f111300 R15:
> ffff8883767b0ac0
> [ 73.179029] FS: 00007f2d207f9700(0000) GS:ffff8883e3280000(0000)
> knlGS:0000000000000000
> [ 73.179034] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 73.179039] CR2: 00007f1500bd36c0 CR3: 00000003df304003 CR4:
> 00000000000206e0
> [ 73.179047] Call Trace:
> [ 73.190461] selinux_setprocattr+0x2ea/0x8f0
> [ 73.191925] ? ptrace_parent_sid+0x530/0x530
> [ 73.193436] ? proc_pid_attr_write+0x185/0x5a0
> [ 73.194967] security_setprocattr+0xa1/0x100
> [ 73.196408] proc_pid_attr_write+0x307/0x5a0
> [ 73.197869] ? mem_read+0x40/0x40
> [ 73.199013] __vfs_write+0x81/0x100
> [ 73.200222] __kernel_write+0xf8/0x330
> [ 73.201562] do_acct_process+0xca5/0x1340
> [ 73.202969] ? __ia32_sys_acct+0x1e0/0x1e0
> [ 73.204498] ? find_held_lock+0x2f/0x1e0
> [ 73.205857] ? rcu_irq_exit+0xec/0x2c0
> [ 73.207160] ? lock_downgrade+0x630/0x630
> [ 73.208541] acct_pin_kill+0x63/0x150
> [ 73.209816] pin_kill+0x16d/0x7c0
> [ 73.210934] ? lockdep_hardirqs_on+0x5e0/0x5e0
> [ 73.212452] ? xas_start+0x155/0x510
> [ 73.213705] ? pin_insert+0x50/0x50
> [ 73.214903] ? finish_wait+0x270/0x270
> [ 73.216213] ? cpumask_next+0x57/0x90
> [ 73.217442] ? mnt_pin_kill+0x68/0x1d0
> [ 73.218851] mnt_pin_kill+0x68/0x1d0
> [ 73.220398] cleanup_mnt+0x11b/0x150
> [ 73.221970] task_work_run+0x136/0x1b0
> [ 73.223427] do_exit+0x830/0x2ca0
> [ 73.224586] ? trace_hardirqs_off+0x3b/0x180
> [ 73.226088] ? mm_update_next_owner+0x6a0/0x6a0
> [ 73.227622] ? find_held_lock+0x2f/0x1e0
> [ 73.228954] ? get_signal+0x2cf/0x1c00
> [ 73.230236] ? lock_downgrade+0x630/0x630
> [ 73.231628] ? rwlock_bug.part.0+0x90/0x90
> [ 73.233020] do_group_exit+0x106/0x2f0
> [ 73.234330] get_signal+0x325/0x1c00
> [ 73.235571] do_signal+0x97/0x1670
> [ 73.236739] ? do_send_specific+0x12d/0x220
> [ 73.238213] ? lock_downgrade+0x630/0x630
> [ 73.239566] ? setup_sigcontext+0x820/0x820
> [ 73.240982] ? check_kill_permission+0x4a/0x510
> [ 73.242509] ? do_send_specific+0x156/0x220
> [ 73.243905] ? do_tkill+0x1c4/0x260
> [ 73.245081] ? do_send_specific+0x220/0x220
> [ 73.246514] ? trace_hardirqs_on_thunk+0x1a/0x1c
> [ 73.248061] ? exit_to_usermode_loop+0x97/0x1d0
> [ 73.249619] exit_to_usermode_loop+0x108/0x1d0
> [ 73.251129] do_syscall_64+0x461/0x580
> [ 73.252454] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [ 73.254219] RIP: 0033:0x462eb9
> [ 73.255327] Code: Bad RIP value.
> [ 73.256539] RSP: 002b:00007f2d207f8c58 EFLAGS: 00000246 ORIG_RAX:
> 00000000000000c8
> [ 73.259454] RAX: 0000000000000000 RBX: 000000000073bf00 RCX:
> 0000000000462eb9
> [ 73.262309] RDX: 0000000000000000 RSI: 000000000000001e RDI:
> 0000000000000005
> [ 73.265064] RBP: 0000000000000002 R08: 0000000000000000 R09:
> 0000000000000000
> [ 73.267774] R10: 0000000000000000 R11: 0000000000000246 R12:
> 00007f2d207f96bc
> [ 73.270546] R13: 00000000004c5509 R14: 00000000007042f0 R15:
> 00000000ffffffff
> [ 73.273542] Modules linked in:
> [ 73.274670] Dumping ftrace buffer:
> [ 73.275852] (ftrace buffer empty)
> [ 73.277187] ---[ end trace dde36a95f458175d ]---
> [ 73.278834] RIP: 0010:commit_creds+0xadb/0xe50
> [ 73.280549] Code: 8b 5b 20 48 c1 ea 03 0f b6 04 02 84 c0 74 08 3c
> 03 0f 8e 06 03 00 00 39 5d 20 0f 85 ff fa ff ff e9 0c fb ff ff e8 05
> a2 25 00 <0f> 0b 48 c7 c7 80 56 c0 83 e8 95 22 b1 00 e8 f2 a1 25 00 0f
> 0b 48
> [ 73.287090] RSP: 0000:ffff88836e65f5d0 EFLAGS: 00010293
> [ 73.288917] RAX: ffff8883767b0000 RBX: ffff88837f111300 RCX:
> ffffffff8124b5db
> [ 73.291390] RDX: 0000000000000000 RSI: ffffffff83c9b140 RDI:
> ffff88837f111300
> [ 73.293864] RBP: ffff888376610400 R08: 0000000000000000 R09:
> 0000000000000004
> [ 73.296370] R10: 0000000000000001 R11: ffffed107c655acf R12:
> ffff8883767b0000
> [ 73.299275] R13: ffff88837f111900 R14: ffff88837f111300 R15:
> ffff8883767b0ac0
> [ 73.301351] Process accounting resumed
> [ 73.301822] FS: 00007f2d207f9700(0000) GS:ffff8883e3280000(0000)
> knlGS:0000000000000000
> [ 73.301827] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 73.301832] CR2: 0000000000462e8f CR3: 0000000003a18002 CR4:
> 00000000000206e0
> [ 73.301850] Kernel panic - not syncing: Fatal exception
> [ 73.310719] Process accounting resumed
> [ 73.311515] Process accounting resumed
> [ 73.318916] Dumping ftrace buffer:
> [ 73.318921] (ftrace buffer empty)
> [ 73.318945] Kernel Offset: disabled
> [ 73.328061] Rebooting in 10 seconds..
>
>
> 425 int commit_creds(struct cred *new)
> 426 {
> 427 struct task_struct *task = current;
> 428 const struct cred *old = task->real_cred;
> 429
> 430 kdebug("commit_creds(%p{%d,%d})", new,
> 431 atomic_read(&new->usage),
> 432 read_cred_subscribers(new));
> 433
> 434 BUG_ON(task->cred != old); // BUG here
>
>
> I find that the call chain which triggered the BUG is :
>
> do_exit
> |-=> acct_process
> | -=> do_acct_process
> | -=> orig_cred = override_creds(file->f_cred); // cred =
> ffff8883c1878900/real_cred = ffff8883c1878900
> | -=> if (file_start_write_trylock(file))
> {__kernel_write(file, &ac, sizeof(acct_t), &pos);}
> | -=> __kernel_write+0xf8/0x330
> | -=> __vfs_write+0x81/0x100
> | -=> proc_pid_attr_write+0x307/0x5a0
> | -=> security_setprocattr+0xa1/0x100
> | -=>selinux_setprocattr+0x2ea/0x8f0
> | -=>commit_creds+0xd97/0x1080 // cred =
> ffff888379a79c00/real_cred = ffff888379a79c00
> | -=> revert_creds(orig_cred); // cred =
> ffff8883c1878900/real_cred = ffff888379a79c00
> |-=> task_work_run
> -=> cleanup_mnt+0x11b/0x150
> -=> mnt_pin_kill+0x68/0x1d0
> -=> pin_kill+0x16d/0x7c0
> -=> acct_pin_kill+0x2e/0x100
> -=> do_acct_process+0x1a0/0x1340
> -=> override_creds+0x18a/0x1c0 // cred =
> ffff8883c1878900/real_cred = ffff888379a79c00
> -=> if (file_start_write_trylock(file))
> {__kernel_write(file, &ac, sizeof(acct_t), &pos);}
> -=> ......
> -=>commit_creds+0xd97/0x1080 // new = ffff888379a6bb00, cred =
> ffff8883c1878900, real_ceed = ffff888379a79c00 BUG here
> -=> revert_creds(orig_cred);
>
>
> Syzkaller Report Testcase:
>
> cat crash.log
>
> 04:05:28 executing program 3:
> clone(0x24100, 0x0, 0x0, 0x0, 0x0)
> r0 = gettid()
> perf_event_open(&(0x7f00000000c0)={0x2, 0x70, 0x4, 0x2, 0x0, 0x0, 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, @perf_bp={0x0}}, 0x0,
> 0xffffffffffffffff, 0xffffffffffffffff, 0x0)
> acct(&(0x7f0000000000)='./file0\x00')
> r1 = openat$smack_thread_current(0xffffffffffffff9c,
> &(0x7f0000000000)='/proc/thread-self/attr/current\x00', 0x2, 0x0)
> fcntl$setown(r1, 0x8, r0)
> rt_tgsigqueueinfo(r0, r0, 0x1e, &(0x7f00000002c0))
> tkill(r0, 0x1e)
>
>
> Reproduce this BUG:
> ./syz-execprog -executor=./syz-executor -repeat=0 -procs=16 -cover=0
> ./crash.log
>
>
> Can anyone help me ?
>
>
> Thanks,
>
> Cheng Jian.
>
>
^ permalink raw reply
* Re: [PATCH] mm: security: introduce CONFIG_INIT_HEAP_ALL
From: Alexander Potapenko @ 2019-04-12 15:23 UTC (permalink / raw)
To: Qian Cai
Cc: Andrew Morton, linux-security-module,
Linux Memory Management List, Nick Desaulniers, Kostya Serebryany,
Dmitriy Vyukov, Kees Cook, Sandeep Patil, Laura Abbott,
Kernel Hardening
In-Reply-To: <1555078584.26196.50.camel@lca.pw>
On Fri, Apr 12, 2019 at 4:16 PM Qian Cai <cai@lca.pw> wrote:
>
> On Fri, 2019-04-12 at 14:45 +0200, Alexander Potapenko wrote:
> > This config option adds the possibility to initialize newly allocated
> > pages and heap objects with zeroes. This is needed to prevent possible
> > information leaks and make the control-flow bugs that depend on
> > uninitialized values more deterministic.
> >
> > Initialization is done at allocation time at the places where checks for
> > __GFP_ZERO are performed. We don't initialize slab caches with
> > constructors or SLAB_TYPESAFE_BY_RCU to preserve their semantics.
> >
> > For kernel testing purposes filling allocations with a nonzero pattern
> > would be more suitable, but may require platform-specific code. To have
> > a simple baseline we've decided to start with zero-initialization.
> >
> > No performance optimizations are done at the moment to reduce double
> > initialization of memory regions.
>
> Sounds like this has already existed in some degree, i.e.,
>
> CONFIG_PAGE_POISONING_ZERO
Note that CONFIG_PAGE_POISONING[_ZERO] initializes freed pages,
whereas the proposed patch initializes newly allocated pages.
It's debatable whether initializing pages on kmalloc()/alloc_pages()
is better or worse than doing so in kfree()/free_pages() from the
security perspective.
But the approach proposed in the patch makes it possible to use a
special GFP flag to request uninitialized memory from the underlying
allocator, so that we don't wipe it twice.
This will be harder to do in the functions that free memory, because
they don't accept GFP flags.
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply
* Re: [PATCH] mm: security: introduce CONFIG_INIT_HEAP_ALL
From: Qian Cai @ 2019-04-12 14:16 UTC (permalink / raw)
To: Alexander Potapenko, akpm
Cc: linux-security-module, linux-mm, ndesaulniers, kcc, dvyukov,
keescook, sspatil, labbott, kernel-hardening
In-Reply-To: <20190412124501.132678-1-glider@google.com>
On Fri, 2019-04-12 at 14:45 +0200, Alexander Potapenko wrote:
> This config option adds the possibility to initialize newly allocated
> pages and heap objects with zeroes. This is needed to prevent possible
> information leaks and make the control-flow bugs that depend on
> uninitialized values more deterministic.
>
> Initialization is done at allocation time at the places where checks for
> __GFP_ZERO are performed. We don't initialize slab caches with
> constructors or SLAB_TYPESAFE_BY_RCU to preserve their semantics.
>
> For kernel testing purposes filling allocations with a nonzero pattern
> would be more suitable, but may require platform-specific code. To have
> a simple baseline we've decided to start with zero-initialization.
>
> No performance optimizations are done at the moment to reduce double
> initialization of memory regions.
Sounds like this has already existed in some degree, i.e.,
CONFIG_PAGE_POISONING_ZERO
^ permalink raw reply
* [PATCH] mm: security: introduce CONFIG_INIT_HEAP_ALL
From: Alexander Potapenko @ 2019-04-12 12:45 UTC (permalink / raw)
To: akpm
Cc: linux-security-module, linux-mm, ndesaulniers, kcc, dvyukov,
keescook, sspatil, labbott, kernel-hardening
This config option adds the possibility to initialize newly allocated
pages and heap objects with zeroes. This is needed to prevent possible
information leaks and make the control-flow bugs that depend on
uninitialized values more deterministic.
Initialization is done at allocation time at the places where checks for
__GFP_ZERO are performed. We don't initialize slab caches with
constructors or SLAB_TYPESAFE_BY_RCU to preserve their semantics.
For kernel testing purposes filling allocations with a nonzero pattern
would be more suitable, but may require platform-specific code. To have
a simple baseline we've decided to start with zero-initialization.
No performance optimizations are done at the moment to reduce double
initialization of memory regions.
Signed-off-by: Alexander Potapenko <glider@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Kostya Serebryany <kcc@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Sandeep Patil <sspatil@android.com>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Jann Horn <jannh@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-mm@kvack.org
Cc: linux-security-module@vger.kernel.org
Cc: kernel-hardening@lists.openwall.com
---
This patch applies on top of the "Refactor memory initialization
hardening" patch series by Kees Cook: https://lkml.org/lkml/2019/4/10/748
---
drivers/infiniband/core/uverbs_ioctl.c | 2 +-
include/linux/gfp.h | 8 ++++++++
kernel/kexec_core.c | 2 +-
mm/dmapool.c | 2 +-
mm/page_alloc.c | 2 +-
mm/slab.c | 6 +++---
mm/slab.h | 10 ++++++++++
mm/slob.c | 2 +-
mm/slub.c | 4 ++--
net/core/sock.c | 2 +-
security/Kconfig.hardening | 10 ++++++++++
11 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
index e1379949e663..34937cecac62 100644
--- a/drivers/infiniband/core/uverbs_ioctl.c
+++ b/drivers/infiniband/core/uverbs_ioctl.c
@@ -127,7 +127,7 @@ __malloc void *_uverbs_alloc(struct uverbs_attr_bundle *bundle, size_t size,
res = (void *)pbundle->internal_buffer + pbundle->internal_used;
pbundle->internal_used =
ALIGN(new_used, sizeof(*pbundle->internal_buffer));
- if (flags & __GFP_ZERO)
+ if (GFP_WANT_INIT(flags))
memset(res, 0, size);
return res;
}
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index fdab7de7490d..4f49a6a13f6f 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -213,6 +213,14 @@ struct vm_area_struct;
#define __GFP_COMP ((__force gfp_t)___GFP_COMP)
#define __GFP_ZERO ((__force gfp_t)___GFP_ZERO)
+#ifdef CONFIG_INIT_HEAP_ALL
+#define GFP_WANT_INIT(flags) (1)
+#define GFP_INIT_ALWAYS_ON (1)
+#else
+#define GFP_WANT_INIT(flags) (unlikely((flags) & __GFP_ZERO))
+#define GFP_INIT_ALWAYS_ON (0)
+#endif
+
/* Disable lockdep for GFP context tracking */
#define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index d7140447be75..1ad0097695a1 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -315,7 +315,7 @@ static struct page *kimage_alloc_pages(gfp_t gfp_mask, unsigned int order)
arch_kexec_post_alloc_pages(page_address(pages), count,
gfp_mask);
- if (gfp_mask & __GFP_ZERO)
+ if (GFP_WANT_INIT(gfp_mask))
for (i = 0; i < count; i++)
clear_highpage(pages + i);
}
diff --git a/mm/dmapool.c b/mm/dmapool.c
index 76a160083506..d40d62145ca3 100644
--- a/mm/dmapool.c
+++ b/mm/dmapool.c
@@ -381,7 +381,7 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
#endif
spin_unlock_irqrestore(&pool->lock, flags);
- if (mem_flags & __GFP_ZERO)
+ if (GFP_WANT_INIT(mem_flags))
memset(retval, 0, pool->size);
return retval;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d96ca5bc555b..ceddc4eeaff4 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2014,7 +2014,7 @@ static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags
post_alloc_hook(page, order, gfp_flags);
- if (!free_pages_prezeroed() && (gfp_flags & __GFP_ZERO))
+ if (!free_pages_prezeroed() && GFP_WANT_INIT(gfp_flags))
for (i = 0; i < (1 << order); i++)
clear_highpage(page + i);
diff --git a/mm/slab.c b/mm/slab.c
index 47a380a486ee..848e47658667 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3331,7 +3331,7 @@ slab_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid,
local_irq_restore(save_flags);
ptr = cache_alloc_debugcheck_after(cachep, flags, ptr, caller);
- if (unlikely(flags & __GFP_ZERO) && ptr)
+ if (SLAB_WANT_INIT(cachep, flags) && ptr)
memset(ptr, 0, cachep->object_size);
slab_post_alloc_hook(cachep, flags, 1, &ptr);
@@ -3388,7 +3388,7 @@ slab_alloc(struct kmem_cache *cachep, gfp_t flags, unsigned long caller)
objp = cache_alloc_debugcheck_after(cachep, flags, objp, caller);
prefetchw(objp);
- if (unlikely(flags & __GFP_ZERO) && objp)
+ if (SLAB_WANT_INIT(cachep, flags) && objp)
memset(objp, 0, cachep->object_size);
slab_post_alloc_hook(cachep, flags, 1, &objp);
@@ -3596,7 +3596,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
cache_alloc_debugcheck_after_bulk(s, flags, size, p, _RET_IP_);
/* Clear memory outside IRQ disabled section */
- if (unlikely(flags & __GFP_ZERO))
+ if (SLAB_WANT_INIT(s, flags))
for (i = 0; i < size; i++)
memset(p[i], 0, s->object_size);
diff --git a/mm/slab.h b/mm/slab.h
index 43ac818b8592..4bb10af0031b 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -167,6 +167,16 @@ static inline slab_flags_t kmem_cache_flags(unsigned int object_size,
SLAB_TEMPORARY | \
SLAB_ACCOUNT)
+/*
+ * Do we need to initialize this allocation?
+ * Always true for __GFP_ZERO, CONFIG_INIT_HEAP_ALL enforces initialization
+ * of caches without constructors and RCU.
+ */
+#define SLAB_WANT_INIT(cache, gfp_flags) \
+ ((GFP_INIT_ALWAYS_ON && !(cache)->ctor && \
+ !((cache)->flags & SLAB_TYPESAFE_BY_RCU)) || \
+ (gfp_flags & __GFP_ZERO))
+
bool __kmem_cache_empty(struct kmem_cache *);
int __kmem_cache_shutdown(struct kmem_cache *);
void __kmem_cache_release(struct kmem_cache *);
diff --git a/mm/slob.c b/mm/slob.c
index 307c2c9feb44..0c402e819cf7 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -330,7 +330,7 @@ static void *slob_alloc(size_t size, gfp_t gfp, int align, int node)
BUG_ON(!b);
spin_unlock_irqrestore(&slob_lock, flags);
}
- if (unlikely(gfp & __GFP_ZERO))
+ if (GFP_WANT_INIT(gfp))
memset(b, 0, size);
return b;
}
diff --git a/mm/slub.c b/mm/slub.c
index d30ede89f4a6..686ab9d49ced 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2750,7 +2750,7 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,
stat(s, ALLOC_FASTPATH);
}
- if (unlikely(gfpflags & __GFP_ZERO) && object)
+ if (SLAB_WANT_INIT(s, gfpflags) && object)
memset(object, 0, s->object_size);
slab_post_alloc_hook(s, gfpflags, 1, &object);
@@ -3172,7 +3172,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
local_irq_enable();
/* Clear memory outside IRQ disabled fastpath loop */
- if (unlikely(flags & __GFP_ZERO)) {
+ if (SLAB_WANT_INIT(s, flags)) {
int j;
for (j = 0; j < i; j++)
diff --git a/net/core/sock.c b/net/core/sock.c
index 782343bb925b..51b13d7fd82f 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1601,7 +1601,7 @@ static struct sock *sk_prot_alloc(struct proto *prot, gfp_t priority,
sk = kmem_cache_alloc(slab, priority & ~__GFP_ZERO);
if (!sk)
return sk;
- if (priority & __GFP_ZERO)
+ if (GFP_WANT_INIT(priority))
sk_prot_clear_nulls(sk, prot->obj_size);
} else
sk = kmalloc(prot->obj_size, priority);
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index d744e20140b4..cb7d7dfb506f 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -93,6 +93,16 @@ choice
endchoice
+config INIT_HEAP_ALL
+ bool "Initialize kernel heap allocations"
+ default n
+ help
+ Enforce initialization of pages allocated from page allocator
+ and objects returned by kmalloc and friends.
+ Allocated memory is initialized with zeroes, preventing possible
+ information leaks and making the control-flow bugs that depend
+ on uninitialized values more deterministic.
+
config GCC_PLUGIN_STRUCTLEAK_VERBOSE
bool "Report forcefully initialized variables"
depends on GCC_PLUGIN_STRUCTLEAK
--
2.21.0.392.gf8f6787159e-goog
^ permalink raw reply related
* Re: [PATCH v2 3/3] security: Implement Clang's stack initialization
From: Alexander Potapenko @ 2019-04-12 11:36 UTC (permalink / raw)
To: Kees Cook
Cc: Masahiro Yamada, James Morris, Alexander Popov, Nick Desaulniers,
Kostya Serebryany, Dmitry Vyukov, Sandeep Patil, Laura Abbott,
Randy Dunlap, Michal Marek, Emese Revfy, Serge E. Hallyn,
Kernel Hardening, linux-security-module,
Linux Kbuild mailing list, LKML
In-Reply-To: <20190411180117.27704-4-keescook@chromium.org>
On Thu, Apr 11, 2019 at 8:01 PM Kees Cook <keescook@chromium.org> wrote:
>
> CONFIG_INIT_STACK_ALL turns on stack initialization based on
> -ftrivial-auto-var-init in Clang builds, which has greater coverage
> than CONFIG_GCC_PLUGINS_STRUCTLEAK_BYREF_ALL.
>
> -ftrivial-auto-var-init Clang option provides trivial initializers for
> uninitialized local variables, variable fields and padding.
>
> It has three possible values:
> pattern - uninitialized locals are filled with a fixed pattern
> (mostly 0xAA on 64-bit platforms, see https://reviews.llvm.org/D54604
> for more details, but 0x000000AA for 32-bit pointers) likely to cause
> crashes when uninitialized value is used;
> zero (it's still debated whether this flag makes it to the official
> Clang release) - uninitialized locals are filled with zeroes;
> uninitialized (default) - uninitialized locals are left intact.
>
> This patch uses only the "pattern" mode when CONFIG_INIT_STACK_ALL is
> enabled.
>
> Developers have the possibility to opt-out of this feature on a
> per-variable basis by using __attribute__((uninitialized)), but such
> use should be well justified in comments.
>
> Co-developed-by: Alexander Potapenko <glider@google.com>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Tested-by: Alexander Potapenko <glider@google.com>
> ---
> Makefile | 5 +++++
> security/Kconfig.hardening | 15 ++++++++++++++-
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index c0a34064c574..a7d9c6cd0267 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -745,6 +745,11 @@ KBUILD_CFLAGS += -fomit-frame-pointer
> endif
> endif
>
> +# Initialize all stack variables with a pattern, if desired.
> +ifdef CONFIG_INIT_STACK_ALL
> +KBUILD_CFLAGS += -ftrivial-auto-var-init=pattern
> +endif
> +
> DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-assignments)
>
> ifdef CONFIG_DEBUG_INFO
> diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
> index 3dd7a28c3822..5dd61770d3f0 100644
> --- a/security/Kconfig.hardening
> +++ b/security/Kconfig.hardening
> @@ -18,9 +18,12 @@ config GCC_PLUGIN_STRUCTLEAK
>
> menu "Memory initialization"
>
> +config CC_HAS_AUTO_VAR_INIT
> + def_bool $(cc-option,-ftrivial-auto-var-init=pattern)
> +
> choice
> prompt "Initialize kernel stack variables at function entry"
> - depends on GCC_PLUGINS
> + depends on CC_HAS_AUTO_VAR_INIT || GCC_PLUGINS
> default INIT_STACK_NONE
> help
> This option enables initialization of stack variables at
> @@ -76,6 +79,16 @@ choice
> of uninitialized stack variable exploits and information
> exposures.
>
> + config INIT_STACK_ALL
> + bool "0xAA-init everything on the stack (strongest)"
> + depends on CC_HAS_AUTO_VAR_INIT
> + help
> + Initializes everything on the stack with a 0xAA
> + pattern. This is intended to eliminate all classes
> + of uninitialized stack variable exploits and information
> + exposures, even variables that were warned to have been
> + left uninitialized.
> +
> endchoice
>
> config GCC_PLUGIN_STRUCTLEAK_VERBOSE
> --
> 2.17.1
>
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply
* Re: [PATCH] tomoyo: Change pathname calculation for read-only filesystems.
From: Tetsuo Handa @ 2019-04-12 11:09 UTC (permalink / raw)
To: viro; +Cc: James Morris, linux-security-module
In-Reply-To: <alpine.LRH.2.21.1902280724410.7990@namei.org>
Al, do you have any concerns with this patch?
On 2019/02/28 5:25, James Morris wrote:
> On Wed, 27 Feb 2019, Tetsuo Handa wrote:
>
>> Commit 5625f2e3266319fd ("TOMOYO: Change pathname for non-rename()able
>> filesystems.") intended to be applied to filesystems where the content is
>> not controllable from the userspace (e.g. proc, sysfs, securityfs), based
>> on an assumption that such filesystems do not support rename() operation.
>>
>> But it turned out that read-only filesystems also do not support rename()
>> operation despite the content is controllable from the userspace, and that
>> commit is annoying TOMOYO users who want to use e.g. squashfs as the root
>> filesystem due to use of local name which does not start with '/'.
>>
>> Therefore, based on an assumption that filesystems which require the
>> device argument upon mount() request is an indication that the content
>> is controllable from the userspace, do not use local name if a filesystem
>> does not support rename() operation but requires the device argument upon
>> mount() request.
>
> I'd definitely like Al's input on this.
>
>>
>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>> ---
>> security/tomoyo/realpath.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/security/tomoyo/realpath.c b/security/tomoyo/realpath.c
>> index 85e6e31..e7832448 100644
>> --- a/security/tomoyo/realpath.c
>> +++ b/security/tomoyo/realpath.c
>> @@ -295,7 +295,8 @@ char *tomoyo_realpath_from_path(const struct path *path)
>> * or dentry without vfsmount.
>> */
>> if (!path->mnt ||
>> - (!inode->i_op->rename))
>> + (!inode->i_op->rename &&
>> + !(sb->s_type->fs_flags & FS_REQUIRES_DEV)))
>> pos = tomoyo_get_local_path(path->dentry, buf,
>> buf_len - 1);
>> /* Get absolute name for the rest. */
>>
>
^ permalink raw reply
* [PATCH (resend)] tomoyo: Add a kernel config option for fuzzing testing.
From: Tetsuo Handa @ 2019-04-12 11:04 UTC (permalink / raw)
To: James Morris; +Cc: linux-security-module, Tetsuo Handa, syzbot, syzbot, syzbot
syzbot is reporting kernel panic triggered by memory allocation fault
injection before loading TOMOYO's policy [1]. To make the fuzzing tests
useful, we need to assign a profile other than "disabled" (no-op) mode.
Therefore, let's allow syzbot to load TOMOYO's built-in policy for
"learning" mode using a kernel config option. This option must not be
enabled for kernels built for production system, for this option also
disables domain/program checks when modifying policy configuration via
/sys/kernel/security/tomoyo/ interface.
[1] https://syzkaller.appspot.com/bug?extid=29569ed06425fcf67a95
Reported-by: syzbot <syzbot+e1b8084e532b6ee7afab@syzkaller.appspotmail.com>
Reported-by: syzbot <syzbot+29569ed06425fcf67a95@syzkaller.appspotmail.com>
Reported-by: syzbot <syzbot+2ee3f8974c2e7dc69feb@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
security/tomoyo/Kconfig | 10 ++++++++++
security/tomoyo/common.c | 13 ++++++++++++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/security/tomoyo/Kconfig b/security/tomoyo/Kconfig
index 404dce6..a00ab7e 100644
--- a/security/tomoyo/Kconfig
+++ b/security/tomoyo/Kconfig
@@ -74,3 +74,13 @@ config SECURITY_TOMOYO_ACTIVATION_TRIGGER
You can override this setting via TOMOYO_trigger= kernel command line
option. For example, if you pass init=/bin/systemd option, you may
want to also pass TOMOYO_trigger=/bin/systemd option.
+
+config SECURITY_TOMOYO_INSECURE_BUILTIN_SETTING
+ bool "Use insecure built-in settings for fuzzing tests."
+ default n
+ depends on SECURITY_TOMOYO
+ select SECURITY_TOMOYO_OMIT_USERSPACE_LOADER
+ help
+ Enabling this option forces minimal built-in policy and disables
+ domain/program checks for run-time policy modifications. Please enable
+ this option only if this kernel is built for doing fuzzing tests.
diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
index 57988d9..dd3d594 100644
--- a/security/tomoyo/common.c
+++ b/security/tomoyo/common.c
@@ -940,7 +940,7 @@ static bool tomoyo_manager(void)
const char *exe;
const struct task_struct *task = current;
const struct tomoyo_path_info *domainname = tomoyo_domain()->domainname;
- bool found = false;
+ bool found = IS_ENABLED(CONFIG_SECURITY_TOMOYO_INSECURE_BUILTIN_SETTING);
if (!tomoyo_policy_loaded)
return true;
@@ -2810,6 +2810,16 @@ void tomoyo_check_profile(void)
*/
void __init tomoyo_load_builtin_policy(void)
{
+#ifdef CONFIG_SECURITY_TOMOYO_INSECURE_BUILTIN_SETTING
+ static char tomoyo_builtin_profile[] __initdata =
+ "PROFILE_VERSION=20150505\n"
+ "0-CONFIG={ mode=learning grant_log=no reject_log=yes }\n";
+ static char tomoyo_builtin_exception_policy[] __initdata =
+ "aggregator proc:/self/exe /proc/self/exe\n";
+ static char tomoyo_builtin_domain_policy[] __initdata = "";
+ static char tomoyo_builtin_manager[] __initdata = "";
+ static char tomoyo_builtin_stat[] __initdata = "";
+#else
/*
* This include file is manually created and contains built-in policy
* named "tomoyo_builtin_profile", "tomoyo_builtin_exception_policy",
@@ -2817,6 +2827,7 @@ void __init tomoyo_load_builtin_policy(void)
* "tomoyo_builtin_stat" in the form of "static char [] __initdata".
*/
#include "builtin-policy.h"
+#endif
u8 i;
const int idx = tomoyo_read_lock();
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/3] tomoyo: Check address length before reading address family
From: Tetsuo Handa @ 2019-04-12 10:59 UTC (permalink / raw)
To: linux-security-module; +Cc: Tetsuo Handa
In-Reply-To: <1555066776-9758-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp>
KMSAN will complain if valid address length passed to bind()/connect()/
sendmsg() is shorter than sizeof("struct sockaddr"->sa_family) bytes.
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
security/tomoyo/network.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/security/tomoyo/network.c b/security/tomoyo/network.c
index 9094f4b3b367..f9ff121d7e1e 100644
--- a/security/tomoyo/network.c
+++ b/security/tomoyo/network.c
@@ -505,6 +505,8 @@ static int tomoyo_check_inet_address(const struct sockaddr *addr,
{
struct tomoyo_inet_addr_info *i = &address->inet;
+ if (addr_len < offsetofend(struct sockaddr, sa_family))
+ return 0;
switch (addr->sa_family) {
case AF_INET6:
if (addr_len < SIN6_LEN_RFC2133)
@@ -594,6 +596,8 @@ static int tomoyo_check_unix_address(struct sockaddr *addr,
{
struct tomoyo_unix_addr_info *u = &address->unix0;
+ if (addr_len < offsetofend(struct sockaddr, sa_family))
+ return 0;
if (addr->sa_family != AF_UNIX)
return 0;
u->addr = ((struct sockaddr_un *) addr)->sun_path;
--
2.16.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox