* [PATCH 4.19 011/115] afs: Fix SELinux setting security label on /afs
From: Greg Kroah-Hartman @ 2020-01-07 20:53 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, David Howells, Marc Dionne, selinux,
linux-security-module, Sasha Levin
In-Reply-To: <20200107205240.283674026@linuxfoundation.org>
From: David Howells <dhowells@redhat.com>
[ Upstream commit bcbccaf2edcf1b76f73f890e968babef446151a4 ]
Make the AFS dynamic root superblock R/W so that SELinux can set the
security label on it. Without this, upgrades to, say, the Fedora
filesystem-afs RPM fail if afs is mounted on it because the SELinux label
can't be (re-)applied.
It might be better to make it possible to bypass the R/O check for LSM
label application through setxattr.
Fixes: 4d673da14533 ("afs: Support the AFS dynamic root")
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Marc Dionne <marc.dionne@auristor.com>
cc: selinux@vger.kernel.org
cc: linux-security-module@vger.kernel.org
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/afs/super.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/afs/super.c b/fs/afs/super.c
index 4d3e274207fb..bd2608297473 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -404,7 +404,6 @@ static int afs_fill_super(struct super_block *sb,
/* allocate the root inode and dentry */
if (as->dyn_root) {
inode = afs_iget_pseudo_dir(sb, true);
- sb->s_flags |= SB_RDONLY;
} else {
sprintf(sb->s_id, "%u", as->volume->vid);
afs_activate_volume(as->volume);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH bpf-next v1 02/13] bpf: lsm: Add a skeleton and config options
From: James Morris @ 2020-01-07 21:13 UTC (permalink / raw)
To: KP Singh
Cc: linux-kernel, bpf, linux-security-module, Alexei Starovoitov,
Daniel Borkmann, Kees Cook, Thomas Garnier, Michael Halcrow,
Paul Turner, Brendan Gregg, Jann Horn, Matthew Garrett,
Christian Brauner, Mickaël Salaün, Florent Revest,
Brendan Jackman, Martin KaFai Lau, Song Liu, Yonghong Song,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20191220154208.15895-3-kpsingh@chromium.org>
On Fri, 20 Dec 2019, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> The LSM can be enabled by CONFIG_SECURITY_BPF.
> Without CONFIG_SECURITY_BPF_ENFORCE, the LSM will run the
> attached eBPF programs but not enforce MAC policy based
> on the return value of the attached programs.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH bpf-next v1 06/13] bpf: lsm: Init Hooks and create files in securityfs
From: James Morris @ 2020-01-07 21:22 UTC (permalink / raw)
To: KP Singh
Cc: linux-kernel, bpf, linux-security-module, Alexei Starovoitov,
Daniel Borkmann, Kees Cook, Thomas Garnier, Michael Halcrow,
Paul Turner, Brendan Gregg, Jann Horn, Matthew Garrett,
Christian Brauner, Mickaël Salaün, Florent Revest,
Brendan Jackman, Martin KaFai Lau, Song Liu, Yonghong Song,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20191220154208.15895-7-kpsingh@chromium.org>
On Fri, 20 Dec 2019, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> The LSM creates files in securityfs for each hook registered with the
> LSM.
>
> /sys/kernel/security/bpf/<h_name>
>
> The list of LSM hooks are maintained in an internal header "hooks.h"
> Eventually, this list should either be defined collectively in
> include/linux/lsm_hooks.h or auto-generated from it.
>
> * Creation of a file for the hook in the securityfs.
> * Allocation of a bpf_lsm_hook data structure which stores
> a pointer to the dentry of the newly created file in securityfs.
> * Creation of a typedef for the hook so that BTF information
> can be generated for the LSM hooks to:
>
> - Make them "Compile Once, Run Everywhere".
> - Pass the right arguments when the attached programs are run.
> - Verify the accesses made by the program by using the BTF
> information.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH bpf-next v1 07/13] bpf: lsm: Implement attach, detach and execution.
From: James Morris @ 2020-01-07 21:27 UTC (permalink / raw)
To: KP Singh
Cc: linux-kernel, bpf, linux-security-module, Alexei Starovoitov,
Daniel Borkmann, Kees Cook, Thomas Garnier, Michael Halcrow,
Paul Turner, Brendan Gregg, Jann Horn, Matthew Garrett,
Christian Brauner, Mickaël Salaün, Florent Revest,
Brendan Jackman, Martin KaFai Lau, Song Liu, Yonghong Song,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20191220154208.15895-8-kpsingh@chromium.org>
On Fri, 20 Dec 2019, KP Singh wrote:
>
> - h_dentry = securityfs_create_file(h->name, 0600, parent,
> - NULL, &hook_ops);
> + h_dentry = securityfs_create_file(h->name, 0600,
> + parent, NULL, &hook_ops);
Minor thing to fix for the next version.
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH bpf-next v1 08/13] bpf: lsm: Show attached program names in hook read handler.
From: James Morris @ 2020-01-07 21:28 UTC (permalink / raw)
To: KP Singh
Cc: linux-kernel, bpf, linux-security-module, Alexei Starovoitov,
Daniel Borkmann, Kees Cook, Thomas Garnier, Michael Halcrow,
Paul Turner, Brendan Gregg, Jann Horn, Matthew Garrett,
Christian Brauner, Mickaël Salaün, Florent Revest,
Brendan Jackman, Martin KaFai Lau, Song Liu, Yonghong Song,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20191220154208.15895-9-kpsingh@chromium.org>
On Fri, 20 Dec 2019, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> For inspectability the system administrator should be able to view the
> list of active KRSI programs:
>
> bash # cat /sys/kernel/security/bpf/bprm_check_security
> bpf_prog1
>
> Signed-off-by: KP Singh <kpsingh@google.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v13 19/25] NET: Store LSM netlabel data in a lsmblob
From: Casey Schaufler @ 2020-01-07 21:58 UTC (permalink / raw)
To: Stephen Smalley, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, john.johansen, penguin-kernel, paul, Casey Schaufler
In-Reply-To: <9e114536-36be-9395-ef00-cd1059d710ac@tycho.nsa.gov>
On 1/7/2020 11:25 AM, Stephen Smalley wrote:
> On 12/24/19 6:59 PM, Casey Schaufler wrote:
>> Netlabel uses LSM interfaces requiring an lsmblob and
>> the internal storage is used to pass information between
>> these interfaces, so change the internal data from a secid
>> to a lsmblob. Update the netlabel interfaces and their
>> callers to accommodate the change. This requires that the
>> modules using netlabel use the lsm_id.slot to access the
>> correct secid when using netlabel.
>>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>> Reviewed-by: John Johansen <john.johansen@canonical.com>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>
> Why is this needed for stacking AppArmor? AA doesn't use NetLabel, at least not upstream AFAICS.
Netlabel uses LSM interfaces that provide/require blobs,
security_secid_to_secctx() and security_secctx_to_secid()
in particular. Either the data maintained needs to be converted
to blobs or it needs extensive scaffolding. The scaffolding would
require a mechanism to identify the lsmblob slot to be used in
netlabel. You can't always use slot 0 because it would be possible
to put AppArmor on the module list ahead of SELinux or Smack. That
would be the only case where the slot number is needed outside the
security sub-system. Since converting the netlabel data to blobs
will be necessary eventually anyway, I want to avoid having to
provide a mechanism whereby netlabel can identify which slot to
use. This is especially true since Paul has nixed the idea of
assigning netlabel to a particular security module.
^ permalink raw reply
* Re: [PATCH 2/2] security, selinux: get rid of security_delete_hooks()
From: Paul Moore @ 2020-01-08 5:31 UTC (permalink / raw)
To: Stephen Smalley
Cc: Ondrej Mosnacek, linux-security-module, James Morris,
Serge E. Hallyn, Casey Schaufler, selinux, John Johansen,
Kees Cook, Micah Morton, Tetsuo Handa
In-Reply-To: <bad81aeb-c21f-c5be-12a1-61912d04573a@tycho.nsa.gov>
On Tue, Jan 7, 2020 at 9:46 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 1/7/20 8:31 AM, Ondrej Mosnacek wrote:
> > The only user is SELinux, which is hereby converted to check the
> > disabled flag in each hook instead of removing the hooks from the list.
> >
> > The __lsm_ro_after_init macro is now removed and replaced with
> > __ro_after_init directly.
> >
> > This fixes a race condition in SELinux runtime disable, which was
> > introduced with the switch to hook lists in b1d9e6b0646d ("LSM: Switch
> > to lists of hooks").
>
> Not opposed (naturally, since I suggested it) but my impression from the
> earlier thread was that Paul preferred the less invasive approach of
> your original patch (just reordering the hooks) as a short term fix with
> an eye toward full removal of disable support in the not-too-distant future.
Unless we are seeing wide spread breakages (I don't think we are), or
we decide we can never remove the runtime disable, I still prefer the
hook-shuffle over the changes proposed in this patchset.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: INFO: rcu detected stall in sys_kill
From: Dmitry Vyukov @ 2020-01-08 6:20 UTC (permalink / raw)
To: Daniel Axtens
Cc: Casey Schaufler, syzbot, linux-security-module, Andrey Ryabinin,
kasan-dev, Andrea Arcangeli, Andrew Morton, Christian Brauner,
christian, cyphar, Reshetova, Elena, Jason Gunthorpe, Kees Cook,
ldv, LKML, Andy Lutomirski, Ingo Molnar, Peter Zijlstra,
syzkaller-bugs, Thomas Gleixner, Al Viro, Will Drewry
In-Reply-To: <87h81zax74.fsf@dja-thinkpad.axtens.net>
On Tue, Dec 17, 2019 at 2:39 PM Daniel Axtens <dja@axtens.net> wrote:
>
> Daniel Axtens <dja@axtens.net> writes:
>
> > Hi Casey,
> >
> >> There haven't been Smack changes recently, so this is
> >> going to have been introduced elsewhere. I'm perfectly
> >> willing to accept that Smack is doing something horribly
> >> wrong WRT rcu, and that it needs repair, but its going to
> >> be tough for me to track down. I hope someone else is looking
> >> into this, as my chances of finding the problem are pretty
> >> slim.
> >
> > Yeah, I'm having a look, it's probably related to my kasan-vmalloc
> > stuff. It's currently in a bit of flux as syzkaller finds a bunch of
> > other bugs with it, once that stablises a bit I'll come back to Smack.
>
> I have had a brief and wildly unsuccessful look at this. I'm happy to
> come back to it and go over it with a finer toothed comb, but it will
> almost certainly have to wait until next year.
>
> I don't think it's related to RCU, we also have a plain lockup:
> https://syzkaller.appspot.com/bug?id=be03729d17bb3b2df1754a7486a8f8628f6ff1ec
>
> Dmitry, I've been really struggling to repro this locally, even with
> your config. Is there an easy way to see the kernel command line you
> booted with and anything else that makes this image special? I have zero
> experience with smack so this is a steep learning curve.
I temporarily re-enabled smack instance and it produced another 50
stalls all over the kernel, and now keeps spewing a dozen every hour.
I've mailed 3 new samples, you can see them here:
https://syzkaller.appspot.com/bug?extid=de8d933e7d153aa0c1bb
The config is provided, command line args are here:
https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-smack.cmdline
Some non-default sysctls that syzbot sets are here:
https://github.com/google/syzkaller/blob/master/dashboard/config/upstream.sysctl
Image can be downloaded from here:
https://github.com/google/syzkaller/blob/master/docs/syzbot.md#crash-does-not-reproduce
syzbot uses GCE VMs with 2 CPUs and 7.5GB memory, but this does not
look to be virtualization-related (?) so probably should reproduce in
qemu too.
> Regards,
> Daniel
>
> >
> > Regards,
> > Daniel
> >
> >>
> >>>>
> >>>> I see 2 common this across all stalls:
> >>>> 1. They all happen on the instance that uses smack (which is now
> >>>> effectively dead), see smack instance here:
> >>>> https://syzkaller.appspot.com/upstream
> >>>> 2. They all contain this frame in the stack trace:
> >>>> free_thread_stack+0x168/0x590 kernel/fork.c:280
> >>>> The last commit that touches this file is "fork: support VMAP_STACK
> >>>> with KASAN_VMALLOC".
> >>>> That may be very likely the root cause. +Daniel
> >>> I've stopped smack syzbot instance b/c it produces infinite stream of
> >>> assorted crashes due to this.
> >>> Please ping syzkaller@googlegroups.com when this is fixed, I will
> >>> re-enable the instance.
> >>>
> >>>>> rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
> >>>>> (detected by 1, t=10502 jiffies, g=6629, q=331)
> >>>>> rcu: All QSes seen, last rcu_preempt kthread activity 10503
> >>>>> (4294953794-4294943291), jiffies_till_next_fqs=1, root ->qsmask 0x0
> >>>>> syz-executor.0 R running task 24648 8293 8292 0x0000400a
> >>>>> Call Trace:
> >>>>> <IRQ>
> >>>>> sched_show_task+0x40f/0x560 kernel/sched/core.c:5954
> >>>>> print_other_cpu_stall kernel/rcu/tree_stall.h:410 [inline]
> >>>>> check_cpu_stall kernel/rcu/tree_stall.h:538 [inline]
> >>>>> rcu_pending kernel/rcu/tree.c:2827 [inline]
> >>>>> rcu_sched_clock_irq+0x1861/0x1ad0 kernel/rcu/tree.c:2271
> >>>>> update_process_times+0x12d/0x180 kernel/time/timer.c:1726
> >>>>> tick_sched_handle kernel/time/tick-sched.c:167 [inline]
> >>>>> tick_sched_timer+0x263/0x420 kernel/time/tick-sched.c:1310
> >>>>> __run_hrtimer kernel/time/hrtimer.c:1514 [inline]
> >>>>> __hrtimer_run_queues+0x403/0x840 kernel/time/hrtimer.c:1576
> >>>>> hrtimer_interrupt+0x38c/0xda0 kernel/time/hrtimer.c:1638
> >>>>> local_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1110 [inline]
> >>>>> smp_apic_timer_interrupt+0x109/0x280 arch/x86/kernel/apic/apic.c:1135
> >>>>> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:829
> >>>>> </IRQ>
> >>>>> RIP: 0010:__read_once_size include/linux/compiler.h:199 [inline]
> >>>>> RIP: 0010:check_kcov_mode kernel/kcov.c:70 [inline]
> >>>>> RIP: 0010:__sanitizer_cov_trace_pc+0x1c/0x50 kernel/kcov.c:102
> >>>>> Code: cc 07 48 89 de e8 64 02 3b 00 5b 5d c3 cc 48 8b 04 24 65 48 8b 0c 25
> >>>>> c0 1d 02 00 65 8b 15 b8 81 8b 7e f7 c2 00 01 1f 00 75 2c <8b> 91 80 13 00
> >>>>> 00 83 fa 02 75 21 48 8b 91 88 13 00 00 48 8b 32 48
> >>>>> RSP: 0018:ffffc900021c7c28 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
> >>>>> RAX: ffffffff81487433 RBX: 0000000000000000 RCX: ffff88809428a100
> >>>>> RDX: 0000000000000001 RSI: 00000000fffffffc RDI: ffffea0002479240
> >>>>> RBP: ffffc900021c7c50 R08: dffffc0000000000 R09: fffffbfff1287025
> >>>>> R10: fffffbfff1287025 R11: 0000000000000000 R12: dffffc0000000000
> >>>>> R13: dffffc0000000000 R14: 00000000fffffffc R15: ffff888091c57428
> >>>>> free_thread_stack+0x168/0x590 kernel/fork.c:280
> >>>>> release_task_stack kernel/fork.c:440 [inline]
> >>>>> put_task_stack+0xa3/0x130 kernel/fork.c:451
> >>>>> finish_task_switch+0x3f1/0x550 kernel/sched/core.c:3256
> >>>>> context_switch kernel/sched/core.c:3388 [inline]
> >>>>> __schedule+0x9a8/0xcc0 kernel/sched/core.c:4081
> >>>>> preempt_schedule_common kernel/sched/core.c:4236 [inline]
> >>>>> preempt_schedule+0xdb/0x120 kernel/sched/core.c:4261
> >>>>> ___preempt_schedule+0x16/0x18 arch/x86/entry/thunk_64.S:50
> >>>>> __raw_read_unlock include/linux/rwlock_api_smp.h:227 [inline]
> >>>>> _raw_read_unlock+0x3a/0x40 kernel/locking/spinlock.c:255
> >>>>> kill_something_info kernel/signal.c:1586 [inline]
> >>>>> __do_sys_kill kernel/signal.c:3640 [inline]
> >>>>> __se_sys_kill+0x5e9/0x6c0 kernel/signal.c:3634
> >>>>> __x64_sys_kill+0x5b/0x70 kernel/signal.c:3634
> >>>>> do_syscall_64+0xf7/0x1c0 arch/x86/entry/common.c:294
> >>>>> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> >>>>> RIP: 0033:0x422a17
> >>>>> Code: 44 00 00 48 c7 c2 d4 ff ff ff f7 d8 64 89 02 b8 ff ff ff ff c3 66 2e
> >>>>> 0f 1f 84 00 00 00 00 00 0f 1f 40 00 b8 3e 00 00 00 0f 05 <48> 3d 01 f0 ff
> >>>>> ff 0f 83 dd 32 ff ff c3 66 2e 0f 1f 84 00 00 00 00
> >>>>> RSP: 002b:00007fff38dca538 EFLAGS: 00000293 ORIG_RAX: 000000000000003e
> >>>>> RAX: ffffffffffffffda RBX: 0000000000000064 RCX: 0000000000422a17
> >>>>> RDX: 0000000000000bb8 RSI: 0000000000000009 RDI: 00000000fffffffe
> >>>>> RBP: 0000000000000002 R08: 0000000000000001 R09: 0000000001c62940
> >>>>> R10: 0000000000000000 R11: 0000000000000293 R12: 0000000000000008
> >>>>> R13: 00007fff38dca570 R14: 000000000000f0b6 R15: 00007fff38dca580
> >>>>> rcu: rcu_preempt kthread starved for 10533 jiffies! g6629 f0x2
> >>>>> RCU_GP_WAIT_FQS(5) ->state=0x0 ->cpu=0
> >>>>> rcu: RCU grace-period kthread stack dump:
> >>>>> rcu_preempt R running task 29032 10 2 0x80004008
> >>>>> Call Trace:
> >>>>> context_switch kernel/sched/core.c:3388 [inline]
> >>>>> __schedule+0x9a8/0xcc0 kernel/sched/core.c:4081
> >>>>> schedule+0x181/0x210 kernel/sched/core.c:4155
> >>>>> schedule_timeout+0x14f/0x240 kernel/time/timer.c:1895
> >>>>> rcu_gp_fqs_loop kernel/rcu/tree.c:1661 [inline]
> >>>>> rcu_gp_kthread+0xed8/0x1770 kernel/rcu/tree.c:1821
> >>>>> kthread+0x332/0x350 kernel/kthread.c:255
> >>>>> ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
> >>>>>
> >>>>>
> >>>>> ---
> >>>>> This bug is generated by a bot. It may contain errors.
> >>>>> See https://goo.gl/tpsmEJ for more information about syzbot.
> >>>>> syzbot engineers can be reached at syzkaller@googlegroups.com.
> >>>>>
> >>>>> syzbot will keep track of this bug report. See:
> >>>>> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> >>>>>
> >>>>> --
> >>>>> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> >>>>> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> >>>>> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/00000000000036decf0598c8762e%40google.com.
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/87h81zax74.fsf%40dja-thinkpad.axtens.net.
^ permalink raw reply
* Re: [PATCH 2/2] security, selinux: get rid of security_delete_hooks()
From: Ondrej Mosnacek @ 2020-01-08 8:15 UTC (permalink / raw)
To: Paul Moore
Cc: Stephen Smalley, Linux Security Module list, James Morris,
Serge E. Hallyn, Casey Schaufler, SElinux list, John Johansen,
Kees Cook, Micah Morton, Tetsuo Handa
In-Reply-To: <CAHC9VhRMLJLSUY5VfdDRv=OuyLkvzsyqfpNhf_SfC9V8OALJ4g@mail.gmail.com>
On Wed, Jan 8, 2020 at 6:32 AM Paul Moore <paul@paul-moore.com> wrote:
> On Tue, Jan 7, 2020 at 9:46 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > On 1/7/20 8:31 AM, Ondrej Mosnacek wrote:
> > > The only user is SELinux, which is hereby converted to check the
> > > disabled flag in each hook instead of removing the hooks from the list.
> > >
> > > The __lsm_ro_after_init macro is now removed and replaced with
> > > __ro_after_init directly.
> > >
> > > This fixes a race condition in SELinux runtime disable, which was
> > > introduced with the switch to hook lists in b1d9e6b0646d ("LSM: Switch
> > > to lists of hooks").
> >
> > Not opposed (naturally, since I suggested it) but my impression from the
> > earlier thread was that Paul preferred the less invasive approach of
> > your original patch (just reordering the hooks) as a short term fix with
> > an eye toward full removal of disable support in the not-too-distant future.
>
> Unless we are seeing wide spread breakages (I don't think we are), or
> we decide we can never remove the runtime disable, I still prefer the
> hook-shuffle over the changes proposed in this patchset.
OK, I'm fine with either solution. Do you want me to rebase and resend
the reorder patch? There are some minor conflicts with Stephen's
recently merged patches.
--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.
^ permalink raw reply
* Re: [PATCH 2/2] security,selinux: get rid of security_delete_hooks()
From: Ondrej Mosnacek @ 2020-01-08 8:21 UTC (permalink / raw)
To: James Morris
Cc: Linux Security Module list, Serge E. Hallyn, Casey Schaufler,
SElinux list, Paul Moore, Stephen Smalley, John Johansen,
Kees Cook, Micah Morton, Tetsuo Handa
In-Reply-To: <alpine.LRH.2.21.2001080653220.575@namei.org>
On Tue, Jan 7, 2020 at 9:00 PM James Morris <jmorris@namei.org> wrote:
> On Tue, 7 Jan 2020, Ondrej Mosnacek wrote:
>
> > The only user is SELinux, which is hereby converted to check the
> > disabled flag in each hook instead of removing the hooks from the list.
> >
> > The __lsm_ro_after_init macro is now removed and replaced with
> > __ro_after_init directly.
> >
> > This fixes a race condition in SELinux runtime disable, which was
> > introduced with the switch to hook lists in b1d9e6b0646d ("LSM: Switch
> > to lists of hooks").
> >
> > Suggested-by: Stephen Smalley <sds@tycho.nsa.gov>
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > ---
> > include/linux/lsm_hooks.h | 31 --
> > security/Kconfig | 5 -
> > security/apparmor/lsm.c | 6 +-
> > security/commoncap.c | 2 +-
> > security/loadpin/loadpin.c | 2 +-
> > security/lockdown/lockdown.c | 2 +-
> > security/security.c | 5 +-
> > security/selinux/Kconfig | 6 -
> > security/selinux/hooks.c | 742 ++++++++++++++++++++++++++++++-----
> > security/smack/smack_lsm.c | 4 +-
> > security/tomoyo/tomoyo.c | 6 +-
> > security/yama/yama_lsm.c | 2 +-
> > 12 files changed, 654 insertions(+), 159 deletions(-)
>
> Please separate the changes for each LSM into separate patches (the
> __lsm_ro_after_init removal patch can be last).
>
> > config SECURITY_SELINUX_DEVELOP
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 47ad4db925cf..9ac2b6b69ff9 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -650,13 +650,15 @@ static int selinux_set_mnt_opts(struct super_block *sb,
> > {
> > const struct cred *cred = current_cred();
> > struct superblock_security_struct *sbsec = sb->s_security;
> > - struct dentry *root = sbsec->sb->s_root;
> > struct selinux_mnt_opts *opts = mnt_opts;
>
> Seems like there are a bunch of unrelated cleanups mixed in here.
These are not unrelated - we need to avoid dereferencing the security
structs before checking selinux_disabled(), because they may be NULL
or uninitialized when SELinux has been diabled.
--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: Make trampolines W^X
From: Andy Lutomirski @ 2020-01-08 8:41 UTC (permalink / raw)
To: Edgecombe, Rick P, Dave Hansen, Nadav Amit, Peter Zijlstra
Cc: songliubraving@fb.com, linux-kernel@vger.kernel.org,
bpf@vger.kernel.org, keescook@chromium.org, jeyu@kernel.org,
ast@kernel.org, kuznet@ms2.inr.ac.ru, daniel@iogearbox.net,
mjg59@google.com, thgarnie@chromium.org, kpsingh@chromium.org,
linux-security-module@vger.kernel.org, x86@kernel.org,
revest@chromium.org, jannh@google.com, namit@vmware.com,
jackmanb@chromium.org, kafai@fb.com, yhs@fb.com,
davem@davemloft.net, yoshfuji@linux-ipv6.org, mhalcrow@google.com,
andriin@fb.com
In-Reply-To: <cdd157ef011efda92c9434f76141fc3aef174d85.camel@intel.com>
> On Jan 7, 2020, at 9:01 AM, Edgecombe, Rick P <rick.p.edgecombe@intel.com> wrote:
>
> CC Nadav and Jessica.
>
> On Mon, 2020-01-06 at 15:36 -1000, Andy Lutomirski wrote:
>>> On Jan 6, 2020, at 12:25 PM, Edgecombe, Rick P <rick.p.edgecombe@intel.com>
>>> wrote:
>>>
>>> On Sat, 2020-01-04 at 09:49 +0900, Andy Lutomirski wrote:
>>>>>>> On Jan 4, 2020, at 8:47 AM, KP Singh <kpsingh@chromium.org> wrote:
>>>>>>
>>>>>> From: KP Singh <kpsingh@google.com>
>>>>>>
>>>>>> The image for the BPF trampolines is allocated with
>>>>>> bpf_jit_alloc_exe_page which marks this allocated page executable. This
>>>>>> means that the allocated memory is W and X at the same time making it
>>>>>> susceptible to WX based attacks.
>>>>>>
>>>>>> Since the allocated memory is shared between two trampolines (the
>>>>>> current and the next), 2 pages must be allocated to adhere to W^X and
>>>>>> the following sequence is obeyed where trampolines are modified:
>>>>>
>>>>> Can we please do better rather than piling garbage on top of garbage?
>>>>>
>>>>>>
>>>>>> - Mark memory as non executable (set_memory_nx). While module_alloc for
>>>>>> x86 allocates the memory as PAGE_KERNEL and not PAGE_KERNEL_EXEC, not
>>>>>> all implementations of module_alloc do so
>>>>>
>>>>> How about fixing this instead?
>>>>>
>>>>>> - Mark the memory as read/write (set_memory_rw)
>>>>>
>>>>> Probably harmless, but see above about fixing it.
>>>>>
>>>>>> - Modify the trampoline
>>>>>
>>>>> Seems reasonable. It’s worth noting that this whole approach is
>>>>> suboptimal:
>>>>> the “module” allocator should really be returning a list of pages to be
>>>>> written (not at the final address!) with the actual executable mapping to
>>>>> be
>>>>> materialized later, but that’s a bigger project that you’re welcome to
>>>>> ignore
>>>>> for now. (Concretely, it should produce a vmap address with backing pages
>>>>> but
>>>>> with the vmap alias either entirely unmapped or read-only. A subsequent
>>>>> healer
>>>>> would, all at once, make the direct map pages RO or not-present and make
>>>>> the
>>>>> vmap alias RX.)
>>>>>> - Mark the memory as read-only (set_memory_ro)
>>>>>> - Mark the memory as executable (set_memory_x)
>>>>>
>>>>> No, thanks. There’s very little excuse for doing two IPI flushes when one
>>>>> would suffice.
>>>>>
>>>>> As far as I know, all architectures can do this with a single flush
>>>>> without
>>>>> races x86 certainly can. The module freeing code gets this sequence
>>>>> right.
>>>>> Please reuse its mechanism or, if needed, export the relevant interfaces.
>>>
>>> So if I understand this right, some trampolines have been added that are
>>> currently set as RWX at modification time AND left that way during runtime?
>>> The
>>> discussion on the order of set_memory_() calls in the commit message made me
>>> think that this was just a modification time thing at first.
>>
>> I’m not sure what the status quo is.
>>
>> We really ought to have a genuinely good API for allocation and initialization
>> of text. We can do so much better than set_memory_blahblah.
>>
>> FWIW, I have some ideas about making kernel flushes cheaper. It’s currently
>> blocked on finding some time and on tglx’s irqtrace work.
>>
>
> Makes sense to me. I guess there are 6 types of text allocations now:
> - These two BPF trampolines
> - BPF JITs
> - Modules
> - Kprobes
> - Ftrace
>
> All doing (or should be doing) pretty much the same thing. I believe Jessica had
> said at one point that she didn't like all the other features using
> module_alloc() as it was supposed to be just for real modules. Where would the
> API live?
New header? This shouldn’t matter that much.
Here are two strawman proposals. All of this is very rough -- the
actual data structures and signatures are likely problematic for
multiple reasons.
--- First proposal ---
struct text_allocation {
void *final_addr;
struct page *pages;
int npages;
};
int text_alloc(struct text_allocation *out, size_t size);
/* now final_addr is not accessible and pages is writable. */
int text_freeze(struct text_allocation *alloc);
/* now pages are not accessible and final_addr is RO. Alternatively,
pages are RO and final_addr is unmapped. */
int text_finish(struct text_allocation *alloc);
/* now final_addr is RX. All done. */
This gets it with just one flush and gives a chance to double-check in
case of race attacks from other CPUs. Double-checking is annoying,
though.
--- Second proposal ---
struct text_allocation {
void *final_addr;
/* lots of opaque stuff including an mm_struct */
/* optional: list of struct page, but this isn't obviously useful */
};
int text_alloc(struct text_allocation *out, size_t size);
/* Memory is allocated. There is no way to access it at all right
now. The memory is RO or not present in the direct map. */
void __user *text_activate_mapping(struct text_allocation *out);
/* Now the text is RW at *user* address given by return value.
Preemption is off if required by use_temporary_mm(). Real user memory
cannot be accessed. */
void text_deactivate_mapping(struct text_allocation *alloc);
/* Now the memory is inaccessible again. */
void text_finalize(struct text_allocation *alloc);
/* Now it's RX or XO at the final address. */
Pros of second approach:
- Inherently immune to cross-CPU attack. No double-check.
- If we ever implement a cache of non-direct-mapped, unaliased pages,
then it works with no flushes at all. We could even relax it a bit to
allow non-direct-mapped pages that may have RX / XO aliases but no W
aliases.
- Can easily access without worrying about page boundaries.
Cons:
- The use of a temporary mm is annoying -- you can't copy from user
memory, for example.
^ permalink raw reply
* Re: [PATCH v6 00/10] proc: modernize proc to support multiple private instances
From: Alexey Gladkov @ 2020-01-08 10:37 UTC (permalink / raw)
To: Alexey Dobriyan
Cc: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module, Akinobu Mita, Alexander Viro,
Andrew Morton, Andy Lutomirski, Daniel Micay, Djalal Harouni,
Dmitry V . Levin, Eric W . Biederman, Greg Kroah-Hartman,
Ingo Molnar, J . Bruce Fields, Jeff Layton, Jonathan Corbet,
Kees Cook, Linus Torvalds, Oleg Nesterov, Solar Designer
In-Reply-To: <20200106151514.GA382@avx2>
On Mon, Jan 06, 2020 at 06:15:14PM +0300, Alexey Dobriyan wrote:
> > hidepid= Set /proc/<pid>/ access mode.
> > gid= Set the group authorized to learn processes information.
> > + pidonly= Show only task related subset of procfs.
>
> I'd rather have
>
> mount -t proc -o set=pid
This is a great idea.
> so that is can be naturally extended to
>
> mount -t proc -o set=pid,sysctl,misc
>
> > +static int proc_dir_open(struct inode *inode, struct file *file)
> > +{
> > + struct proc_fs_info *fs_info = proc_sb_info(inode->i_sb);
> > +
> > + if (proc_fs_pidonly(fs_info) == PROC_PIDONLY_ON)
> > + return -ENOENT;
> > +
> > + return 0;
> > +}
> > +
> > /*
> > * These are the generic /proc directory operations. They
> > * use the in-memory "struct proc_dir_entry" tree to parse
> > @@ -338,6 +357,7 @@ static const struct file_operations proc_dir_operations = {
> > .llseek = generic_file_llseek,
> > .read = generic_read_dir,
> > .iterate_shared = proc_readdir,
> > + .open = proc_dir_open,
>
> This should not be necessary: if lookup and readdir filters work
> then ->open can't happen.
Yes you are right.
> > --- a/include/linux/proc_fs.h
> > +++ b/include/linux/proc_fs.h
> > +/* definitions for hide_pid field */
> > +enum {
> > + HIDEPID_OFF = 0,
> > + HIDEPID_NO_ACCESS = 1,
> > + HIDEPID_INVISIBLE = 2,
> > + HIDEPID_NOT_PTRACABLE = 3, /* Limit pids to only ptracable pids */
> > +};
>
> These should live in uapi/ as they _are_ user interface to mount().
OK.
What do you think, maybe it's better to make these values a mask ?
I mean:
#define HIDEPID_OFF 0
#define HIDEPID_NO_ACCESS 1
#define HIDEPID_INVISIBLE 2
#define HIDEPID_NOT_PTRACABLE 4
In this case, if in the future there appear values that can be combined,
then there will be no need to make additional parameters.
--
Rgrds, legion
^ permalink raw reply
* [PATCH v2] ima: export the measurement list when needed
From: Janne Karhunen @ 2020-01-08 11:17 UTC (permalink / raw)
To: linux-integrity, linux-security-module, zohar
Cc: kgold, david.safford, monty.wiseman, Janne Karhunen
Some systems can end up carrying lots of entries in the ima
measurement list. Since every entry is using a bit of kernel
memory, allow the sysadmin to export the measurement list to
the filesystem to free up some memory.
Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
---
security/integrity/ima/ima.h | 7 +-
security/integrity/ima/ima_fs.c | 171 +++++++++++++++++++++++++++++
security/integrity/ima/ima_queue.c | 2 +-
3 files changed, 175 insertions(+), 5 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 19769bf5f6ab..78f0b706848d 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -151,20 +151,19 @@ int template_desc_init_fields(const char *template_fmt,
struct ima_template_desc *ima_template_desc_current(void);
struct ima_template_desc *lookup_template_desc(const char *name);
bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
+void ima_free_template_entry(struct ima_template_entry *entry);
int ima_restore_measurement_entry(struct ima_template_entry *entry);
int ima_restore_measurement_list(loff_t bufsize, void *buf);
int ima_measurements_show(struct seq_file *m, void *v);
unsigned long ima_get_binary_runtime_size(void);
int ima_init_template(void);
void ima_init_template_list(void);
+int ima_export_list(const char *from);
int __init ima_init_digests(void);
int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
void *lsm_data);
-/*
- * used to protect h_table and sha_table
- */
-extern spinlock_t ima_queue_lock;
+extern struct mutex ima_extend_list_mutex;
struct ima_h_table {
atomic_long_t len; /* number of stored measurements in the list */
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 2000e8df0301..b60a241b0d8b 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -22,10 +22,17 @@
#include <linux/rcupdate.h>
#include <linux/parser.h>
#include <linux/vmalloc.h>
+#include <linux/fs_struct.h>
+#include <linux/syscalls.h>
#include "ima.h"
+#define secfs_mnt "/sys/kernel/security"
+#define am_filename "/integrity/ima/ascii_runtime_measurements"
+
static DEFINE_MUTEX(ima_write_mutex);
+static DEFINE_MUTEX(ima_list_mutex);
+static char *ima_msmt_list_name;
bool ima_canonical_fmt;
static int __init default_canonical_fmt_setup(char *str)
@@ -362,6 +369,7 @@ static struct dentry *ascii_runtime_measurements;
static struct dentry *runtime_measurements_count;
static struct dentry *violations;
static struct dentry *ima_policy;
+static struct dentry *ima_list_name;
enum ima_fs_flags {
IMA_FS_BUSY,
@@ -449,6 +457,162 @@ static const struct file_operations ima_measure_policy_ops = {
.llseek = generic_file_llseek,
};
+static void ima_free_list(void)
+{
+ struct ima_queue_entry *qe, *e;
+
+ list_for_each_entry_safe(qe, e, &ima_measurements, later) {
+ hlist_del_rcu(&qe->hnext);
+ atomic_long_dec(&ima_htable.len);
+
+ list_del_rcu(&qe->later);
+ ima_free_template_entry(qe->entry);
+ kfree(qe);
+ }
+}
+
+static int ima_unlink_file(const char *filename)
+{
+ struct filename *file;
+
+ file = getname_kernel(filename);
+ if (IS_ERR(file))
+ return -EINVAL;
+
+ return do_unlinkat(AT_FDCWD, file);
+}
+
+int ima_export_list(const char *from)
+{
+ static bool init_export = true;
+
+ struct file *file_out = NULL;
+ struct file *file_in = NULL;
+ const char *to = ima_msmt_list_name;
+ ssize_t bytesin, bytesout;
+ mm_segment_t fs;
+ struct path root;
+ loff_t offin = 0, offout = 0;
+ char data[512];
+ int err = 0;
+
+ if (to == NULL)
+ goto out_err;
+ if (from == NULL)
+ from = secfs_mnt am_filename;
+
+ pr_info("exporting msmt list to %s\n", to);
+ fs = get_fs();
+ set_fs(KERNEL_DS);
+
+ if (init_export) {
+ ima_unlink_file(to);
+ init_export = false;
+ }
+ /*
+ * Use the root of the init task..
+ */
+ task_lock(&init_task);
+ get_fs_root(init_task.fs, &root);
+ task_unlock(&init_task);
+
+ file_out = file_open_root(root.dentry, root.mnt, to,
+ O_CREAT|O_WRONLY|O_APPEND|O_NOFOLLOW,
+ 0600);
+ if (IS_ERR(file_out)) {
+ err = PTR_ERR(file_out);
+ pr_err("failed to open %s, err %d\n", to, err);
+ file_out = NULL;
+ goto out_close;
+ }
+ file_in = file_open_root(root.dentry, root.mnt, from, O_RDONLY, 0);
+ if (IS_ERR(file_in)) {
+ err = PTR_ERR(file_in);
+ pr_err("failed to open %s, err %d\n", from, err);
+ file_in = NULL;
+ goto out_close;
+ }
+ mutex_lock(&ima_extend_list_mutex);
+ do {
+ bytesin = vfs_read(file_in, data, 512, &offin);
+ if (bytesin < 0) {
+ pr_err("read error at %lld\n", offin);
+ err = -EIO;
+ goto out_unlock;
+ }
+ bytesout = vfs_write(file_out, data, bytesin, &offout);
+ if (bytesin != bytesout) {
+ /*
+ * If we fail writing, don't free the list and allow
+ * a retry later on.
+ */
+ pr_err("write error at %lld\n", offout);
+ err = -EIO;
+ goto out_unlock;
+ }
+ } while (bytesin == 512);
+ ima_free_list();
+
+out_unlock:
+ mutex_unlock(&ima_extend_list_mutex);
+out_close:
+ if (file_in)
+ filp_close(file_in, NULL);
+ if (file_out)
+ filp_close(file_out, NULL);
+
+ path_put(&root);
+ set_fs(fs);
+out_err:
+ return err;
+}
+
+static ssize_t ima_write_list_name(struct file *filp,
+ const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ int err;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ if ((count <= 1) || (count >= 255))
+ return -EINVAL;
+
+ if (*buf != '/')
+ return -EINVAL;
+
+ mutex_lock(&ima_list_mutex);
+ kfree(ima_msmt_list_name);
+
+ ima_msmt_list_name = kzalloc(count, GFP_KERNEL);
+ if (!ima_msmt_list_name) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
+ err = copy_from_user(ima_msmt_list_name, buf, count);
+ if (err) {
+ kfree(ima_msmt_list_name);
+ ima_msmt_list_name = NULL;
+ goto out_unlock;
+ }
+ if (ima_msmt_list_name[count-1] == '\n')
+ ima_msmt_list_name[count-1] = 0;
+
+ err = ima_export_list(NULL);
+out_unlock:
+ mutex_unlock(&ima_list_mutex);
+ if (err) {
+ pr_err("list export failed with %d\n", err);
+ return err;
+ }
+ return count;
+}
+
+static const struct file_operations ima_list_export_ops = {
+ .write = ima_write_list_name,
+};
+
int __init ima_fs_init(void)
{
ima_dir = securityfs_create_dir("ima", integrity_dir);
@@ -493,6 +657,11 @@ int __init ima_fs_init(void)
if (IS_ERR(ima_policy))
goto out;
+ ima_list_name = securityfs_create_file("list_name", 0200, ima_dir,
+ NULL, &ima_list_export_ops);
+ if (IS_ERR(ima_list_name))
+ goto out;
+
return 0;
out:
securityfs_remove(violations);
@@ -502,5 +671,7 @@ int __init ima_fs_init(void)
securityfs_remove(ima_symlink);
securityfs_remove(ima_dir);
securityfs_remove(ima_policy);
+ securityfs_remove(ima_list_name);
+
return -1;
}
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 1ce8b1701566..77c538ec8474 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -44,7 +44,7 @@ struct ima_h_table ima_htable = {
* and extending the TPM PCR aggregate. Since tpm_extend can take
* long (and the tpm driver uses a mutex), we can't use the spinlock.
*/
-static DEFINE_MUTEX(ima_extend_list_mutex);
+DEFINE_MUTEX(ima_extend_list_mutex);
/* lookup up the digest value in the hash table, and return the entry */
static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] LSM: Delete hooks in reverse order for avoiding race
From: Ondrej Mosnacek @ 2020-01-08 11:42 UTC (permalink / raw)
To: Huaisheng Ye
Cc: Kees Cook, Casey Schaufler, James Morris, efremov, Paul Moore,
David Howells, joel, tyu1, Linux kernel mailing list,
Huaisheng Ye, SElinux list, Linux Security Module list
In-Reply-To: <20200108083430.57412-1-yehs2007@zoho.com>
Hi,
On Wed, Jan 8, 2020 at 9:51 AM Huaisheng Ye <yehs2007@zoho.com> wrote:
> From: Huaisheng Ye <yehs1@lenovo.com>
>
> There is small possibility as race condition when selinux_disable
> has been triggered. security_delete_hooks deletes all selinux hooks
> from security_hook_heads, but there are some selinux functions which
> are being called at the same time.
>
> Here is a panic accident scene from 4.18 based kernel,
>
> [ 26.654494] SELinux: Disabled at runtime.
> [ 26.654507] BUG: unable to handle kernel NULL pointer dereference
> at 0000000000000020
> [ 26.654508] PGD 0 P4D 0
> [ 26.654510] Oops: 0002 [#1] SMP NOPTI
> [ 26.654512] CPU: 53 PID: 2614 Comm: systemd-cgroups Tainted: G
> OE --------- - - 4.18.0-80.el8.x86_64 #1
> [ 26.654512] Hardware name: Lenovo ThinkSystem SR850P
> -[7D2H]-/-[7D2H]-, BIOS -[TEE145P-1.10]- 12/06/2019
> [ 26.654519] RIP: 0010:selinux_socket_post_create+0x80/0x390
> [ 26.654520] Code: e9 95 6a 89 00 bd 16 00 00 00 c7 44 24 04 01
> 00 00 00 45 85 c0 0f 85 f6 00 00 00 8b 56 14 85 d2 0f 84 26 01 00
> 00 89 54 24 04 <66> 41 89 6c 24 20 31 c0 41 89 54 24 1c 41 c6 44
> 24 22 01 49 8b 4d
> [ 26.654521] RSP: 0018:ffffbf515cc63e48 EFLAGS: 00010246
> [ 26.654522] RAX: 0000000000000000 RBX: 0000000000000001 RCX: 0000000000000019
> [ 26.654522] RDX: 0000000000000001 RSI: 0000000000000001 RDI: ffffffffab46f680
> [ 26.654523] RBP: 0000000000000019 R08: 0000000000000000 R09: ffffbf515cc63e4c
> [ 26.654523] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> [ 26.654524] R13: ffff97d7bb6cbc80 R14: 0000000000000001 R15: ffff97d7bb6cbc80
> [ 26.654525] FS: 00007f5c608ea380(0000) GS:ffff97d7bf140000(0000) knlGS:0000000000000000
> [ 26.654525] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 26.654526] CR2: 0000000000000020 CR3: 0000011ebc934004 CR4: 00000000007606e0
> [ 26.654527] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 26.654528] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 26.654528] PKRU: 55555554
> [ 26.654528] Call Trace:
> [ 26.654535] security_socket_post_create+0x42/0x60
> [ 26.654537] SELinux: Unregistering netfilter hooks
> [ 26.654542] __sock_create+0x106/0x1a0
> [ 26.654545] __sys_socket+0x57/0xe0
> [ 26.654547] __x64_sys_socket+0x16/0x20
> [ 26.654551] do_syscall_64+0x5b/0x1b0
> [ 26.654554] entry_SYSCALL_64_after_hwframe+0x65/0xca
>
> The root cause is that, selinux_inode_alloc_security has been deleted
> firstly from security_hook_heads, so security_inode_alloc directly
> return 0, that means the value of pointer inode->i_security equalling
> to NULL.
>
> But selinux_socket_post_create hasn't been deleted at that moment, so
> which would involked by mistake. Inside the function, pointer isec
> needs to point to inode->i_security, then a NULL pointer defect happens.
>
> For current upstream kernel, because of commit
> afb1cbe37440c7f38b9cf46fc331cc9dfd5cce21
> the inode security has been moved out to LSM infrastructure from
> individual security modules like selinux.
>
> But this patch still can be applied for solving similar issue when
> security_delete_hooks has been used. Also for stable branch v4.19,
> the inode security still need to be created in individual modules.
Thank you for the patch, however there are already existing proposed
patches to fix this issue, see [1], [2], and [3]. At the moment it
looks like the SELinux hooks reorder approach ([1]) will be accepted
as a temporary solution (the SELinux runtime disable is being
deprecated [4] in favor of properly disabling SELinux by setting
selinux=0 on the kernel command line).
Your approach unfortunately isn't robust (depends on assumptions about
how hooks are ordered by LSMs) nor complete (even the inverse order
still has some race conditions that may lead to a crash - e.g.
selinux_bpf_map() vs. selinux_bpf_map_alloc()).
Also, please, don't forget to Cc the LSM/SELinux mailing lists
(linux-security-module@vger.kernel.org/selinux@vger.kernel.org,
respectively) for patches related to the LSM framework/SELinux.
[1] https://lore.kernel.org/selinux/20191209075756.123157-1-omosnace@redhat.com/T/
[2] https://lore.kernel.org/selinux/20191211140833.939845-1-omosnace@redhat.com/T/
[3] https://lore.kernel.org/selinux/20200107133154.588958-1-omosnace@redhat.com/T/
[4] https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git/commit/?h=next&id=89b223bfb8a89731bea4c84982b5d2ad7ba460e3
>
> The patch has been verified by Lenovo SR850P server through overnight
> reboot cycles.
>
> Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>
> ---
> include/linux/lsm_hooks.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 20d8cf1..57cb2ac 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2164,7 +2164,7 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
> int i;
>
> for (i = 0; i < count; i++)
> - hlist_del_rcu(&hooks[i].list);
> + hlist_del_rcu(&hooks[count - 1 - i].list);
> }
> #endif /* CONFIG_SECURITY_SELINUX_DISABLE */
>
> --
> 1.8.3.1
>
>
--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.
^ permalink raw reply
* Re: [PATCH 2/2] security, selinux: get rid of security_delete_hooks()
From: Paul Moore @ 2020-01-08 13:45 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Stephen Smalley, Linux Security Module list, James Morris,
Serge E. Hallyn, Casey Schaufler, SElinux list, John Johansen,
Kees Cook, Micah Morton, Tetsuo Handa
In-Reply-To: <CAFqZXNuv6OV_w_qneo-vK2Z4SBOqKRnnwNr-tgK__uSoX=76Ww@mail.gmail.com>
On Wed, Jan 8, 2020 at 3:15 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> On Wed, Jan 8, 2020 at 6:32 AM Paul Moore <paul@paul-moore.com> wrote:
> > On Tue, Jan 7, 2020 at 9:46 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > > On 1/7/20 8:31 AM, Ondrej Mosnacek wrote:
> > > > The only user is SELinux, which is hereby converted to check the
> > > > disabled flag in each hook instead of removing the hooks from the list.
> > > >
> > > > The __lsm_ro_after_init macro is now removed and replaced with
> > > > __ro_after_init directly.
> > > >
> > > > This fixes a race condition in SELinux runtime disable, which was
> > > > introduced with the switch to hook lists in b1d9e6b0646d ("LSM: Switch
> > > > to lists of hooks").
> > >
> > > Not opposed (naturally, since I suggested it) but my impression from the
> > > earlier thread was that Paul preferred the less invasive approach of
> > > your original patch (just reordering the hooks) as a short term fix with
> > > an eye toward full removal of disable support in the not-too-distant future.
> >
> > Unless we are seeing wide spread breakages (I don't think we are), or
> > we decide we can never remove the runtime disable, I still prefer the
> > hook-shuffle over the changes proposed in this patchset.
>
> OK, I'm fine with either solution. Do you want me to rebase and resend
> the reorder patch? There are some minor conflicts with Stephen's
> recently merged patches.
Yes please.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH 2/2] security, selinux: get rid of security_delete_hooks()
From: Stephen Smalley @ 2020-01-08 14:49 UTC (permalink / raw)
To: Paul Moore
Cc: Ondrej Mosnacek, linux-security-module, James Morris,
Serge E. Hallyn, Casey Schaufler, selinux, John Johansen,
Kees Cook, Micah Morton, Tetsuo Handa
In-Reply-To: <CAHC9VhRMLJLSUY5VfdDRv=OuyLkvzsyqfpNhf_SfC9V8OALJ4g@mail.gmail.com>
On 1/8/20 12:31 AM, Paul Moore wrote:
> On Tue, Jan 7, 2020 at 9:46 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>> On 1/7/20 8:31 AM, Ondrej Mosnacek wrote:
>>> The only user is SELinux, which is hereby converted to check the
>>> disabled flag in each hook instead of removing the hooks from the list.
>>>
>>> The __lsm_ro_after_init macro is now removed and replaced with
>>> __ro_after_init directly.
>>>
>>> This fixes a race condition in SELinux runtime disable, which was
>>> introduced with the switch to hook lists in b1d9e6b0646d ("LSM: Switch
>>> to lists of hooks").
>>
>> Not opposed (naturally, since I suggested it) but my impression from the
>> earlier thread was that Paul preferred the less invasive approach of
>> your original patch (just reordering the hooks) as a short term fix with
>> an eye toward full removal of disable support in the not-too-distant future.
>
> Unless we are seeing wide spread breakages (I don't think we are), or
> we decide we can never remove the runtime disable, I still prefer the
> hook-shuffle over the changes proposed in this patchset.
Note that the first patch is a necessary and correct cleanup regardless
of this one.
^ permalink raw reply
* Re: [PATCH bpf-next v1 00/13] MAC and Audit policy using eBPF (KRSI)
From: Stephen Smalley @ 2020-01-08 15:25 UTC (permalink / raw)
To: Kees Cook, KP Singh
Cc: Casey Schaufler, open list, bpf, linux-security-module,
Alexei Starovoitov, Daniel Borkmann, James Morris, Thomas Garnier,
Michael Halcrow, Paul Turner, Brendan Gregg, Jann Horn,
Matthew Garrett, Christian Brauner, Mickaël Salaün,
Florent Revest, Brendan Jackman, Martin KaFai Lau, Song Liu,
Yonghong Song, Serge E. Hallyn, Mauro Carvalho Chehab,
David S. Miller, Greg Kroah-Hartman, Nicolas Ferre,
Stanislav Fomichev, Quentin Monnet, Andrey Ignatov, Joe Stringer,
Paul Moore
In-Reply-To: <201912301112.A1A63A4@keescook>
On 12/30/19 2:15 PM, Kees Cook wrote:
> On Fri, Dec 20, 2019 at 06:38:45PM +0100, KP Singh wrote:
>> Hi Casey,
>>
>> Thanks for taking a look!
>>
>> On Fri, Dec 20, 2019 at 6:17 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>
>>> On 12/20/2019 7:41 AM, KP Singh wrote:
>>>> From: KP Singh <kpsingh@google.com>
>>>>
>>>> This patch series is a continuation of the KRSI RFC
>>>> (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/)
>>>>
>>>> # Motivation
>>>>
>>>> Google does rich analysis of runtime security data collected from
>>>> internal Linux deployments (corporate devices and servers) to detect and
>>>> thwart threats in real-time. Currently, this is done in custom kernel
>>>> modules but we would like to replace this with something that's upstream
>>>> and useful to others.
>>>>
>>>> The current kernel infrastructure for providing telemetry (Audit, Perf
>>>> etc.) is disjoint from access enforcement (i.e. LSMs). Augmenting the
>>>> information provided by audit requires kernel changes to audit, its
>>>> policy language and user-space components. Furthermore, building a MAC
>>>> policy based on the newly added telemetry data requires changes to
>>>> various LSMs and their respective policy languages.
>>>>
>>>> This patchset proposes a new stackable and privileged LSM which allows
>>>> the LSM hooks to be implemented using eBPF. This facilitates a unified
>>>> and dynamic (not requiring re-compilation of the kernel) audit and MAC
>>>> policy.
>>>>
>>>> # Why an LSM?
>>>>
>>>> Linux Security Modules target security behaviours rather than the
>>>> kernel's API. For example, it's easy to miss out a newly added system
>>>> call for executing processes (eg. execve, execveat etc.) but the LSM
>>>> framework ensures that all process executions trigger the relevant hooks
>>>> irrespective of how the process was executed.
>>>>
>>>> Allowing users to implement LSM hooks at runtime also benefits the LSM
>>>> eco-system by enabling a quick feedback loop from the security community
>>>> about the kind of behaviours that the LSM Framework should be targeting.
>>>>
>>>> # How does it work?
>>>>
>>>> The LSM introduces a new eBPF (https://docs.cilium.io/en/v1.6/bpf/)
>>>> program type, BPF_PROG_TYPE_LSM, which can only be attached to a LSM
>>>> hook. All LSM hooks are exposed as files in securityfs. Attachment
>>>> requires CAP_SYS_ADMIN for loading eBPF programs and CAP_MAC_ADMIN for
>>>> modifying MAC policies.
>>>>
>>>> The eBPF programs are passed the same arguments as the LSM hooks and
>>>> executed in the body of the hook.
>>>
>>> This effectively exposes the LSM hooks as external APIs.
>>> It would mean that we can't change or delete them. That
>>> would be bad.
>>
>> Perhaps this should have been clearer, we *do not* want to make LSM hooks
>> a stable API and expect the eBPF programs to adapt when such changes occur.
>>
>> Based on our comparison with the previous approach, this still ends up
>> being a better trade-off (w.r.t. maintenance) when compared to adding
>> specific helpers or verifier logic for each new hook or field that
>> needs to be exposed.
>
> Given the discussion around tracing and stable ABI at the last kernel
> summit, Linus's mandate is mainly around "every day users" and not
> around these system-builder-sensitive cases where everyone has a strong
> expectation to rebuild their policy when the kernel changes. i.e. it's
> not "powertop", which was Linus's example of "and then everyone running
> Fedora breaks".
>
> So, while I know we've tried in the past to follow the letter of the
> law, it seems Linus really expects this only to be followed when it will
> have "real world" impact on unsuspecting end users.
>
> Obviously James Morris has the final say here, but as I understand it,
> it is fine to expose these here for the same reasons it's fine to expose
> the (ever changing) tracepoints and BPF hooks.
This appears to impose a very different standard to this eBPF-based LSM
than has been applied to the existing LSMs, e.g. we are required to
preserve SELinux policy compatibility all the way back to Linux 2.6.0
such that new kernel with old policy does not break userspace. If that
standard isn't being applied to the eBPF-based LSM, it seems to inhibit
its use in major Linux distros, since otherwise users will in fact start
experiencing breakage on the first such incompatible change. Not
arguing for or against, just trying to make sure I understand correctly...
^ permalink raw reply
* Re: [PATCH v2] ima: add the ability to query the hash of a given file.
From: Mimi Zohar @ 2020-01-08 16:05 UTC (permalink / raw)
To: Florent Revest, linux-integrity
Cc: kpsingh, mjg59, nramas, linux-kernel, linux-security-module,
Florent Revest
In-Reply-To: <20200106162524.164650-1-revest@chromium.org>
On Mon, 2020-01-06 at 17:25 +0100, Florent Revest wrote:
> From: Florent Revest <revest@google.com>
>
> This allows other parts of the kernel (perhaps a stacked LSM allowing
> system monitoring, eg. the proposed KRSI LSM [1]) to retrieve the hash
> of a given file from IMA if it's present in the iint cache.
>
> It's true that the existence of the hash means that it's also in the
> audit logs or in /sys/kernel/security/ima/ascii_runtime_measurements,
> but it can be difficult to pull that information out for every
> subsequent exec. This is especially true if a given host has been up
> for a long time and the file was first measured a long time ago.
>
> This is based on Peter Moody's patch:
> https://sourceforge.net/p/linux-ima/mailman/message/33036180/
FYI, but unlike the audit log/IMA measurement list, the iint cache
entries can be removed. Refer to security_inode_free(). Perhaps
mention of this difference should be included, here, in the patch
description.
>
> [1] https://lkml.org/lkml/2019/9/10/393
>
> Signed-off-by: Florent Revest <revest@google.com>
Assuming, with the above difference, you're still interested in having
this feature upstreamed and addressing the comments above and below:
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
> include/linux/ima.h | 6 ++++
> security/integrity/ima/ima_main.c | 46 +++++++++++++++++++++++++++++++
> 2 files changed, 52 insertions(+)
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 6d904754d858..d621c65ba9a5 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -23,6 +23,7 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
> extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
> enum kernel_read_file_id id);
> extern void ima_post_path_mknod(struct dentry *dentry);
> +extern int ima_file_hash(struct file *file, char *buf, size_t buf_size);
> extern void ima_kexec_cmdline(const void *buf, int size);
>
> #ifdef CONFIG_IMA_KEXEC
> @@ -91,6 +92,11 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
> return;
> }
>
> +static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> static inline void ima_kexec_cmdline(const void *buf, int size) {}
> #endif /* CONFIG_IMA */
>
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index d7e987baf127..3799b6c6c3b8 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -445,6 +445,52 @@ int ima_file_check(struct file *file, int mask)
> }
> EXPORT_SYMBOL_GPL(ima_file_check);
>
> +/**
> + * ima_file_hash - return the stored measurement if a file has been hashed.
> + * @file: pointer to the file
> + * @buf: buffer in which to store the hash
> + * @buf_size: length of the buffer
> + *
> + * On success, return the hash algorithm (as defined in the enum hash_algo).
> + * If buf is not NULL, this function also outputs the hash into buf.
As of Linux 5.4.y, IMA support for appended file signatures was added.
Should we indicate that the file hash returned is based on the entire
file, including the appended signature?
Mimi
> + * If the hash is larger than buf_size, then only buf_size bytes will be copied.
> + * It generally just makes sense to pass a buffer capable of holding the largest
> + * possible hash: IMA_MAX_DIGEST_SIZE
> + *
> + * If IMA is disabled or if no measurement is available, return -EOPNOTSUPP.
> + * If the parameters are incorrect, return -EINVAL.
> + */
> +int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> +{
> + struct inode *inode;
> + struct integrity_iint_cache *iint;
> + int hash_algo;
> +
> + if (!file)
> + return -EINVAL;
> +
> + if (!ima_policy_flag)
> + return -EOPNOTSUPP;
> +
> + inode = file_inode(file);
> + iint = integrity_iint_find(inode);
> + if (!iint)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&iint->mutex);
> + if (buf) {
> + size_t copied_size;
> +
> + copied_size = min_t(size_t, iint->ima_hash->length, buf_size);
> + memcpy(buf, iint->ima_hash->digest, copied_size);
> + }
> + hash_algo = iint->ima_hash->algo;
> + mutex_unlock(&iint->mutex);
> +
> + return hash_algo;
> +}
> +EXPORT_SYMBOL_GPL(ima_file_hash);
> +
> /**
> * ima_post_create_tmpfile - mark newly created tmpfile as new
> * @file : newly created tmpfile
^ permalink raw reply
* Re: [PATCH v4 2/9] perf/core: open access for CAP_SYS_PERFMON privileged process
From: Peter Zijlstra @ 2020-01-08 16:07 UTC (permalink / raw)
To: Alexey Budankov
Cc: Arnaldo Carvalho de Melo, Ingo Molnar,
jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com,
rodrigo.vivi@intel.com, Alexei Starovoitov,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, James Morris,
Will Deacon, Mark Rutland, Casey Schaufler, Robert Richter,
Jiri Olsa, Andi Kleen, Stephane Eranian, Igor Lubashev,
Alexander Shishkin, Namhyung Kim, Kees Cook, Jann Horn,
Thomas Gleixner, Tvrtko Ursulin, Lionel Landwerlin, Song Liu,
linux-kernel, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
bpf@vger.kernel.org, linux-parisc@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, linux-perf-users@vger.kernel.org,
linux-arm-kernel, oprofile-list
In-Reply-To: <c93309dc-b920-f5fa-f997-e8b2faf47b88@linux.intel.com>
On Wed, Dec 18, 2019 at 12:25:35PM +0300, Alexey Budankov wrote:
>
> Open access to perf_events monitoring for CAP_SYS_PERFMON privileged
> processes. For backward compatibility reasons access to perf_events
> subsystem remains open for CAP_SYS_ADMIN privileged processes but
> CAP_SYS_ADMIN usage for secure perf_events monitoring is discouraged
> with respect to CAP_SYS_PERFMON capability.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> include/linux/perf_event.h | 6 +++---
> kernel/events/core.c | 6 +++---
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 34c7c6910026..f46acd69425f 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1285,7 +1285,7 @@ static inline int perf_is_paranoid(void)
>
> static inline int perf_allow_kernel(struct perf_event_attr *attr)
> {
> - if (sysctl_perf_event_paranoid > 1 && !capable(CAP_SYS_ADMIN))
> + if (sysctl_perf_event_paranoid > 1 && !perfmon_capable())
> return -EACCES;
>
> return security_perf_event_open(attr, PERF_SECURITY_KERNEL);
> @@ -1293,7 +1293,7 @@ static inline int perf_allow_kernel(struct perf_event_attr *attr)
>
> static inline int perf_allow_cpu(struct perf_event_attr *attr)
> {
> - if (sysctl_perf_event_paranoid > 0 && !capable(CAP_SYS_ADMIN))
> + if (sysctl_perf_event_paranoid > 0 && !perfmon_capable())
> return -EACCES;
>
> return security_perf_event_open(attr, PERF_SECURITY_CPU);
> @@ -1301,7 +1301,7 @@ static inline int perf_allow_cpu(struct perf_event_attr *attr)
>
> static inline int perf_allow_tracepoint(struct perf_event_attr *attr)
> {
> - if (sysctl_perf_event_paranoid > -1 && !capable(CAP_SYS_ADMIN))
> + if (sysctl_perf_event_paranoid > -1 && !perfmon_capable())
> return -EPERM;
>
> return security_perf_event_open(attr, PERF_SECURITY_TRACEPOINT);
These are OK I suppose.
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 059ee7116008..d9db414f2197 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -9056,7 +9056,7 @@ static int perf_kprobe_event_init(struct perf_event *event)
> if (event->attr.type != perf_kprobe.type)
> return -ENOENT;
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (!perfmon_capable())
> return -EACCES;
>
> /*
This one only allows attaching to already extant kprobes, right? It does
not allow creation of kprobes.
> @@ -9116,7 +9116,7 @@ static int perf_uprobe_event_init(struct perf_event *event)
> if (event->attr.type != perf_uprobe.type)
> return -ENOENT;
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (!perfmon_capable())
> return -EACCES;
>
> /*
Idem, I presume.
> @@ -11157,7 +11157,7 @@ SYSCALL_DEFINE5(perf_event_open,
> }
>
> if (attr.namespaces) {
> - if (!capable(CAP_SYS_ADMIN))
> + if (!perfmon_capable())
> return -EACCES;
> }
And given we basically make the entire kernel observable with this CAP,
busting namespaces shoulnd't be a problem either.
So yeah, I suppose that works.
^ permalink raw reply
* Re: [PATCH bpf-next v1 10/13] bpf: lsm: Handle attachment of the same program
From: James Morris @ 2020-01-08 18:21 UTC (permalink / raw)
To: KP Singh
Cc: linux-kernel, bpf, linux-security-module, Alexei Starovoitov,
Daniel Borkmann, Kees Cook, Thomas Garnier, Michael Halcrow,
Paul Turner, Brendan Gregg, Jann Horn, Matthew Garrett,
Christian Brauner, Mickaël Salaün, Florent Revest,
Brendan Jackman, Martin KaFai Lau, Song Liu, Yonghong Song,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20191220154208.15895-11-kpsingh@chromium.org>
On Fri, 20 Dec 2019, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> Allow userspace to attach a newer version of a program without having
> duplicates of the same program.
>
> If BPF_F_ALLOW_OVERRIDE is passed, the attachment logic compares the
> name of the new program to the names of existing attached programs. The
> names are only compared till a "__" (or '\0', if there is no "__"). If
> a successful match is found, the existing program is replaced with the
> newer attachment.
>
> ./loader Attaches "env_dumper__v1" followed by "env_dumper__v2"
> to the bprm_check_security hook..
>
> ./loader
> ./loader
>
> Before:
>
> cat /sys/kernel/security/bpf/process_execution
> env_dumper__v1
> env_dumper__v2
>
> After:
>
> cat /sys/kernel/security/bpf/process_execution
> env_dumper__v2
>
> Signed-off-by: KP Singh <kpsingh@google.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH bpf-next v1 11/13] tools/libbpf: Add bpf_program__attach_lsm
From: James Morris @ 2020-01-08 18:24 UTC (permalink / raw)
To: KP Singh
Cc: linux-kernel, bpf, linux-security-module, Alexei Starovoitov,
Daniel Borkmann, Kees Cook, Thomas Garnier, Michael Halcrow,
Paul Turner, Brendan Gregg, Jann Horn, Matthew Garrett,
Christian Brauner, Mickaël Salaün, Florent Revest,
Brendan Jackman, Martin KaFai Lau, Song Liu, Yonghong Song,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20191220154208.15895-12-kpsingh@chromium.org>
On Fri, 20 Dec 2019, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> Add functionality in libbpf to attach eBPF program to LSM hooks.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH bpf-next v1 12/13] bpf: lsm: Add selftests for BPF_PROG_TYPE_LSM
From: James Morris @ 2020-01-08 18:25 UTC (permalink / raw)
To: KP Singh
Cc: linux-kernel, bpf, linux-security-module, Alexei Starovoitov,
Daniel Borkmann, Kees Cook, Thomas Garnier, Michael Halcrow,
Paul Turner, Brendan Gregg, Jann Horn, Matthew Garrett,
Christian Brauner, Mickaël Salaün, Florent Revest,
Brendan Jackman, Martin KaFai Lau, Song Liu, Yonghong Song,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20191220154208.15895-13-kpsingh@chromium.org>
On Fri, 20 Dec 2019, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> * Load a BPF program that audits mprotect calls
> * Attach the program to the "file_mprotect" LSM hook
> * Verify if the program is actually loading by reading
> securityfs
> * Initialize the perf events buffer and poll for audit events
> * Do an mprotect on some memory allocated on the heap
> * Verify if the audit event was received
>
> Signed-off-by: KP Singh <kpsingh@google.com>
Good to see!
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH bpf-next v1 00/13] MAC and Audit policy using eBPF (KRSI)
From: James Morris @ 2020-01-08 18:27 UTC (permalink / raw)
To: Kees Cook
Cc: KP Singh, Casey Schaufler, open list, bpf, linux-security-module,
Alexei Starovoitov, Daniel Borkmann, Thomas Garnier,
Michael Halcrow, Paul Turner, Brendan Gregg, Jann Horn,
Matthew Garrett, Christian Brauner, Mickaël Salaün,
Florent Revest, Brendan Jackman, Martin KaFai Lau, Song Liu,
Yonghong Song, Serge E. Hallyn, Mauro Carvalho Chehab,
David S. Miller, Greg Kroah-Hartman, Nicolas Ferre,
Stanislav Fomichev, Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <201912301112.A1A63A4@keescook>
On Mon, 30 Dec 2019, Kees Cook wrote:
>
> Given the discussion around tracing and stable ABI at the last kernel
> summit, Linus's mandate is mainly around "every day users" and not
> around these system-builder-sensitive cases where everyone has a strong
> expectation to rebuild their policy when the kernel changes. i.e. it's
> not "powertop", which was Linus's example of "and then everyone running
> Fedora breaks".
>
> So, while I know we've tried in the past to follow the letter of the
> law, it seems Linus really expects this only to be followed when it will
> have "real world" impact on unsuspecting end users.
>
> Obviously James Morris has the final say here, but as I understand it,
> it is fine to expose these here for the same reasons it's fine to expose
> the (ever changing) tracepoints and BPF hooks.
Agreed. This API should be seen in the same light as tracing / debugging,
and it should not be exposed by users directly to general purpose
applications.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH 2/2] security,selinux: get rid of security_delete_hooks()
From: James Morris @ 2020-01-08 18:47 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Linux Security Module list, Serge E. Hallyn, Casey Schaufler,
SElinux list, Paul Moore, Stephen Smalley, John Johansen,
Kees Cook, Micah Morton, Tetsuo Handa
In-Reply-To: <CAFqZXNvbNqrTJWxdQU4P-7O-kLRcGW+QcL7LY5Ca8rULLm2ScA@mail.gmail.com>
On Wed, 8 Jan 2020, Ondrej Mosnacek wrote:
> > > {
> > > const struct cred *cred = current_cred();
> > > struct superblock_security_struct *sbsec = sb->s_security;
> > > - struct dentry *root = sbsec->sb->s_root;
> > > struct selinux_mnt_opts *opts = mnt_opts;
> >
> > Seems like there are a bunch of unrelated cleanups mixed in here.
>
> These are not unrelated - we need to avoid dereferencing the security
> structs before checking selinux_disabled(), because they may be NULL
> or uninitialized when SELinux has been diabled.
Ok.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH bpf-next v1 00/13] MAC and Audit policy using eBPF (KRSI)
From: James Morris @ 2020-01-08 18:58 UTC (permalink / raw)
To: Stephen Smalley
Cc: Kees Cook, KP Singh, Casey Schaufler, open list, bpf,
linux-security-module, Alexei Starovoitov, Daniel Borkmann,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Brendan Jackman,
Martin KaFai Lau, Song Liu, Yonghong Song, Serge E. Hallyn,
Mauro Carvalho Chehab, David S. Miller, Greg Kroah-Hartman,
Nicolas Ferre, Stanislav Fomichev, Quentin Monnet, Andrey Ignatov,
Joe Stringer, Paul Moore
In-Reply-To: <c4e6cdf2-1233-fc82-ca01-ba84d218f5aa@tycho.nsa.gov>
On Wed, 8 Jan 2020, Stephen Smalley wrote:
> This appears to impose a very different standard to this eBPF-based LSM than
> has been applied to the existing LSMs, e.g. we are required to preserve
> SELinux policy compatibility all the way back to Linux 2.6.0 such that new
> kernel with old policy does not break userspace. If that standard isn't being
> applied to the eBPF-based LSM, it seems to inhibit its use in major Linux
> distros, since otherwise users will in fact start experiencing breakage on the
> first such incompatible change. Not arguing for or against, just trying to
> make sure I understand correctly...
A different standard would be applied here vs. a standard LSM like
SELinux, which are retrofitted access control systems.
I see KRSI as being more of a debugging / analytical API, rather than an
access control system. You could of course build such a system with KRSI
but it would need to provide a layer of abstraction for general purpose
users.
So yes this would be a special case, as its real value is in being a
special case, i.e. dynamic security telemetry.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
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