* Re: [PATCH v8 02/28] LSM: Infrastructure management of the sock security
From: John Johansen @ 2019-09-18 7:19 UTC (permalink / raw)
To: Stephen Smalley, Casey Schaufler, casey.schaufler, jmorris,
linux-security-module, selinux
Cc: keescook, penguin-kernel, paul
In-Reply-To: <5fde58fe-3925-c9d6-39bf-9adb318f7186@tycho.nsa.gov>
On 9/16/19 11:42 AM, Stephen Smalley wrote:
> On 8/29/19 7:29 PM, Casey Schaufler wrote:
>> Move management of the sock->sk_security blob out
>> of the individual security modules and into the security
>> infrastructure. Instead of allocating the blobs from within
>> the modules the modules tell the infrastructure how much
>> space is required, and the space is allocated there.
>>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>> Reviewed-by: John Johansen <john.johansen@canonical.com>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>
> One oddity noted below, but it isn't introduced by this patch so you can add my:
>
> Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov>
>
>> ---
>> include/linux/lsm_hooks.h | 1 +
>> security/apparmor/include/net.h | 6 ++-
>> security/apparmor/lsm.c | 38 ++++-----------
>> security/security.c | 36 +++++++++++++-
>> security/selinux/hooks.c | 78 +++++++++++++++----------------
>> security/selinux/include/objsec.h | 5 ++
>> security/selinux/netlabel.c | 23 ++++-----
>> security/smack/smack.h | 5 ++
>> security/smack/smack_lsm.c | 64 ++++++++++++-------------
>> security/smack/smack_netfilter.c | 8 ++--
>> 10 files changed, 144 insertions(+), 120 deletions(-)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index f9222a04968d..b353482ea348 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -2047,6 +2047,7 @@ struct lsm_blob_sizes {
>> int lbs_cred;
>> int lbs_file;
>> int lbs_inode;
>> + int lbs_sock;
>> int lbs_superblock;
>> int lbs_ipc;
>> int lbs_msg_msg;
>> diff --git a/security/apparmor/include/net.h b/security/apparmor/include/net.h
>> index 7334ac966d01..adac04e3b3cc 100644
>> --- a/security/apparmor/include/net.h
>> +++ b/security/apparmor/include/net.h
>> @@ -55,7 +55,11 @@ struct aa_sk_ctx {
>> struct aa_label *peer;
>> };
>> -#define SK_CTX(X) ((X)->sk_security)
>> +static inline struct aa_sk_ctx *aa_sock(const struct sock *sk)
>> +{
>> + return sk->sk_security + apparmor_blob_sizes.lbs_sock;
>> +}
>> +
>> #define SOCK_ctx(X) SOCK_INODE(X)->i_security
>
> This use of i_security looks suspicious, but SOCK_ctx doesn't appear to be used presently. Probably should be removed in a separate patch.
>
yes this leaked is from some in dev patches that leaked into the base socket mediation patch. I can put together a patch to remove it
^ permalink raw reply
* KASAN: use-after-free Read in key_put
From: syzbot @ 2019-09-18 8:05 UTC (permalink / raw)
To: dhowells, jmorris, keyrings, linux-kernel, linux-security-module,
serge, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: a7f89616 Merge branch 'for-5.3-fixes' of git://git.kernel...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=175eae11600000
kernel config: https://syzkaller.appspot.com/x/.config?x=861a6f31647968de
dashboard link: https://syzkaller.appspot.com/bug?extid=f3745a1dc2a5eb5040ef
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+f3745a1dc2a5eb5040ef@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in atomic_read
include/asm-generic/atomic-instrumented.h:26 [inline]
BUG: KASAN: use-after-free in refcount_sub_and_test_checked+0x87/0x200
lib/refcount.c:182
Read of size 4 at addr ffff88805b89fc80 by task kworker/1:1/23
CPU: 1 PID: 23 Comm: kworker/1:1 Not tainted 5.3.0-rc8+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: events key_garbage_collector
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
print_address_description.cold+0xd4/0x306 mm/kasan/report.c:351
__kasan_report.cold+0x1b/0x36 mm/kasan/report.c:482
kasan_report+0x12/0x17 mm/kasan/common.c:618
check_memory_region_inline mm/kasan/generic.c:185 [inline]
check_memory_region+0x134/0x1a0 mm/kasan/generic.c:192
__kasan_check_read+0x11/0x20 mm/kasan/common.c:92
atomic_read include/asm-generic/atomic-instrumented.h:26 [inline]
refcount_sub_and_test_checked+0x87/0x200 lib/refcount.c:182
refcount_dec_and_test_checked+0x1b/0x20 lib/refcount.c:220
key_put+0x20/0x90 security/keys/key.c:646
keyring_free_object+0x19/0x20 security/keys/keyring.c:389
assoc_array_destroy_subtree.part.0+0x1c5/0x4b0 lib/assoc_array.c:393
assoc_array_destroy_subtree lib/assoc_array.c:354 [inline]
assoc_array_destroy+0x43/0x90 lib/assoc_array.c:444
keyring_destroy+0x259/0x2f0 security/keys/keyring.c:431
key_gc_unused_keys.constprop.0+0x313/0x5b0 security/keys/gc.c:136
key_garbage_collector+0x3f3/0x940 security/keys/gc.c:292
process_one_work+0x9af/0x1740 kernel/workqueue.c:2269
worker_thread+0x98/0xe40 kernel/workqueue.c:2415
kthread+0x361/0x430 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Allocated by task 28788:
save_stack+0x23/0x90 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_kmalloc mm/kasan/common.c:493 [inline]
__kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:466
kasan_slab_alloc+0xf/0x20 mm/kasan/common.c:501
slab_post_alloc_hook mm/slab.h:520 [inline]
slab_alloc mm/slab.c:3319 [inline]
kmem_cache_alloc+0x121/0x710 mm/slab.c:3483
kmem_cache_zalloc include/linux/slab.h:738 [inline]
key_alloc+0x426/0x1110 security/keys/key.c:276
key_create_or_update+0x652/0xbe0 security/keys/key.c:924
__do_sys_add_key security/keys/keyctl.c:132 [inline]
__se_sys_add_key security/keys/keyctl.c:72 [inline]
__x64_sys_add_key+0x2bd/0x4f0 security/keys/keyctl.c:72
do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:296
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 10344:
save_stack+0x23/0x90 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/common.c:455
kasan_slab_free+0xe/0x10 mm/kasan/common.c:463
__cache_free mm/slab.c:3425 [inline]
kmem_cache_free+0x86/0x320 mm/slab.c:3693
key_gc_unused_keys.constprop.0+0x192/0x5b0 security/keys/gc.c:157
key_garbage_collector+0x3f3/0x940 security/keys/gc.c:292
process_one_work+0x9af/0x1740 kernel/workqueue.c:2269
worker_thread+0x98/0xe40 kernel/workqueue.c:2415
kthread+0x361/0x430 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
The buggy address belongs to the object at ffff88805b89fc80
which belongs to the cache key_jar of size 304
The buggy address is located 0 bytes inside of
304-byte region [ffff88805b89fc80, ffff88805b89fdb0)
The buggy address belongs to the page:
page:ffffea00016e27c0 refcount:1 mapcount:0 mapping:ffff88821bc461c0
index:0x0
flags: 0x1fffc0000000200(slab)
raw: 01fffc0000000200 ffffea00025c63c8 ffffea00023378c8 ffff88821bc461c0
raw: 0000000000000000 ffff88805b89f080 000000010000000a 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff88805b89fb80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88805b89fc00: fb fb fb fb fb fb fc fc fc fc fc fc fc fc fc fc
> ffff88805b89fc80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88805b89fd00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88805b89fd80: fb fb fb fb fb fb fc fc fc fc fc fc fc fc fc fc
==================================================================
---
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.
^ permalink raw reply
* Can KEY_DH_OPERATIONS become tristate? (was: Re: Kernel 5.3.0 stuck during boot on Amiga)
From: Geert Uytterhoeven @ 2019-09-18 14:27 UTC (permalink / raw)
To: John Paul Adrian Glaubitz
Cc: Michael Schmitz, linux-m68k, Mat Martineau, David Howells,
James Morris, Serge E. Hallyn, keyrings, linux-security-module,
Linux Kernel Mailing List
In-Reply-To: <CAMuHMdVeedJZE6mrGdYqRgawUtfu_ww5p-Qg1rLXNmGWiY7Nxg@mail.gmail.com>
CC crypto keys people
TL;DR: CONFIG_CRYPTO_DH=y is reported to cause boot delays of several
minutes on old and slow machines. Can KEY_DH_OPERATIONS be made tristate?
On Wed, Sep 18, 2019 at 4:08 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Wed, Sep 18, 2019 at 3:57 PM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
> > On 9/18/19 3:48 PM, Geert Uytterhoeven wrote:
> > >> Diffie-Hellman doing some heavy crypto lifting on a poor m68k CPU?
> > >>
> > >> Disable CONFIG_CRYPTO_DH?
> > >
> > > See also https://lists.debian.org/debian-68k/2019/04/msg00033.html
> > >
> > > CRYPTO_DH is selected by CRYPTO_DEV_QAT and KEY_DH_OPERATIONS.
> > > The latter is bool, forcing CRYPTO_DH builtin.
> > >
> > > If KEY_DH_OPERATIONS needs to be enabled in a Debian kernel, perhaps
> > > it can be made tristate?
> > It was enabled in [1] as it's required for certain WiFi drivers [2].
> >
> > So, should it be fixed as you suggest or should we selectively disable it on m68k?
>
> Disabling it on m68k could be a first step (any WiFi drivers supported
> on m68k yet?).
>
> Making it tristate is non-trivial, as there are some interdependencies:
>
> security/keys/Makefile:compat-obj-$(CONFIG_KEY_DH_OPERATIONS) += compat_dh.o
> security/keys/Makefile:obj-$(CONFIG_KEY_DH_OPERATIONS) += dh.o
> security/keys/internal.h:#ifdef CONFIG_KEY_DH_OPERATIONS
> security/keys/keyctl.c:
> (IS_ENABLED(CONFIG_KEY_DH_OPERATIONS) ? KEYCTL_CAPS0_DIFFIE_HELLMAN
> : 0) |
>
> > > [1] https://salsa.debian.org/kernel-team/linux/commit/88f44cb9eb34098138c79bdab5fae434492866d1
> > > [2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=911998
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: Can KEY_DH_OPERATIONS become tristate? (was: Re: Kernel 5.3.0 stuck during boot on Amiga)
From: David Howells @ 2019-09-18 15:54 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: dhowells, John Paul Adrian Glaubitz, Michael Schmitz, linux-m68k,
Mat Martineau, James Morris, Serge E. Hallyn, keyrings,
linux-security-module, Linux Kernel Mailing List
In-Reply-To: <CAMuHMdVHZ9srJcK+PY=YoP55z1NSjBAtkSr2ROA8i84C75v0zQ@mail.gmail.com>
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> CC crypto keys people
>
> TL;DR: CONFIG_CRYPTO_DH=y is reported to cause boot delays of several
> minutes on old and slow machines.
Why is it doing that? It doesn't do anything unless it is called, so
something must be calling it.
> Can KEY_DH_OPERATIONS be made tristate?
Um. It's non-trivial since it's implementing a keyctl() function for
userspace to call and there's currently no ops table to jump through.
David
^ permalink raw reply
* Re: Can KEY_DH_OPERATIONS become tristate? (was: Re: Kernel 5.3.0 stuck during boot on Amiga)
From: Geert Uytterhoeven @ 2019-09-18 16:18 UTC (permalink / raw)
To: David Howells
Cc: John Paul Adrian Glaubitz, Michael Schmitz, linux-m68k,
Mat Martineau, James Morris, Serge E. Hallyn, keyrings,
linux-security-module, Linux Kernel Mailing List
In-Reply-To: <16476.1568822057@warthog.procyon.org.uk>
Hi David,
On Wed, Sep 18, 2019 at 5:54 PM David Howells <dhowells@redhat.com> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > CC crypto keys people
> >
> > TL;DR: CONFIG_CRYPTO_DH=y is reported to cause boot delays of several
> > minutes on old and slow machines.
>
> Why is it doing that? It doesn't do anything unless it is called, so
> something must be calling it.
I don't know. Enabling initcall_debug shows that dh_init() takes a very long
time.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: Can KEY_DH_OPERATIONS become tristate? (was: Re: Kernel 5.3.0 stuck during boot on Amiga)
From: David Howells @ 2019-09-18 16:43 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: dhowells, John Paul Adrian Glaubitz, Michael Schmitz, linux-m68k,
Mat Martineau, James Morris, Serge E. Hallyn, keyrings,
linux-crypto, linux-security-module, Linux Kernel Mailing List
In-Reply-To: <CAMuHMdU_2RWFc=xs3tM38Nt_44k3dp5MMuKAT2MacyuCbO+1Hw@mail.gmail.com>
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > TL;DR: CONFIG_CRYPTO_DH=y is reported to cause boot delays of several
> > > minutes on old and slow machines.
> >
> > Why is it doing that? It doesn't do anything unless it is called, so
> > something must be calling it.
>
> I don't know. Enabling initcall_debug shows that dh_init() takes a very long
> time.
Ah... The bit that handles keyctl_dh_compute() doesn't do anything unless
asked, but the bit in the crypto layer that does dh does (ie. dh_init()). I
guess it's doing some sort of self-test, but I can't see how it effects that.
I think you need to consult the author/maintainer of crypto/dh.c.
It might be possible to make CONFIG_KEY_DH_OPERATIONS not depend on
CONFIG_CRYPTO_DH and have crypto_alloc_kpp() load the *crypto* part on
demand. Failing that, I can look into demand-loading keyctl operations.
David
^ permalink raw reply
* [GIT PULL] SafeSetID LSM changes for 5.4
From: Micah Morton @ 2019-09-18 17:41 UTC (permalink / raw)
To: Linus Torvalds, Linux Kernel Mailing List, linux-security-module
The following changes since commit 609488bc979f99f805f34e9a32c1e3b71179d10b:
Linux 5.3-rc2 (2019-07-28 12:47:02 -0700)
are available in the Git repository at:
https://github.com/micah-morton/linux.git tags/safesetid-bugfix-5.4
for you to fetch changes up to 21ab8580b383f27b7f59b84ac1699cb26d6c3d69:
LSM: SafeSetID: Stop releasing uninitialized ruleset (2019-09-17
11:27:05 -0700)
----------------------------------------------------------------
Fix for SafeSetID bug that was introduced in 5.3
Jann Horn sent some patches to fix some bugs in SafeSetID for 5.3. After
he had done his testing there were a couple small code tweaks that went
in and caused this bug. From what I can see SafeSetID is broken in 5.3
and crashes the kernel every time during initialization if you try to
use it. I came across this bug when backporting Jann's changes for 5.3
to older kernels (4.14 and 4.19). I've tested on a Chrome OS device and
verified that this change fixes things. Unless I'm missing something it
doesn't seem super useful to have this change bake in linux-next, since it is
completely broken in 5.3 and nobody noticed.
Signed-off-by: Micah Morton <mortonm@chromium.org>
----------------------------------------------------------------
Micah Morton (1):
LSM: SafeSetID: Stop releasing uninitialized ruleset
security/safesetid/securityfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
^ permalink raw reply
* Re: [PATCH v8 22/28] SELinux: Verify LSM display sanity in binder
From: Stephen Smalley @ 2019-09-18 17:43 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <20190829232935.7099-23-casey@schaufler-ca.com>
On 8/29/19 7:29 PM, Casey Schaufler wrote:
> Verify that the tasks on the ends of a binder transaction
> use LSM display values that don't cause SELinux contexts
> to be interpreted by another LSM or another LSM's context
> to be interpreted by SELinux. No judgement is made in cases
> that where SELinux contexts are not used in the binder
> transaction.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/security.h | 1 +
> security/selinux/hooks.c | 34 ++++++++++++++++++++++++++++++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index c16aea55be97..7fcc94ec8e04 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -125,6 +125,7 @@ struct lsmblob {
> u32 secid[LSMBLOB_ENTRIES];
> };
>
> +#define LSMBLOB_FIRST 0 /* First valid LSM slot number */
> #define LSMBLOB_INVALID -1 /* Not a valid LSM slot number */
> #define LSMBLOB_NEEDED -2 /* Slot requested on initialization */
> #define LSMBLOB_NOT_NEEDED -3 /* Slot not requested */
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 352be16a887d..2844f2ab7706 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2009,6 +2009,28 @@ static inline u32 open_file_to_av(struct file *file)
> return av;
> }
>
> +/*
> + * Verify that if the "display" LSM is SELinux for either task
> + * that it is for both tasks.
> + */
> +static inline bool compatible_task_displays(struct task_struct *here,
> + struct task_struct *there)
> +{
> + int h = lsm_task_display(here);
> + int t = lsm_task_display(there);
> +
> + if (h == t)
> + return true;
> +
> + /* unspecified is only ok if SELinux isn't going to be involved */
> + if (selinux_lsmid.slot == LSMBLOB_FIRST)
> + return ((h == LSMBLOB_FIRST && t == LSMBLOB_INVALID) ||
> + (t == LSMBLOB_FIRST && h == LSMBLOB_INVALID));
> +
> + /* it's ok only if neither display is SELinux */
> + return (h != selinux_lsmid.slot && t != selinux_lsmid.slot);
> +}
> +
> /* Hook functions begin here. */
>
> static int selinux_binder_set_context_mgr(struct task_struct *mgr)
> @@ -2016,6 +2038,9 @@ static int selinux_binder_set_context_mgr(struct task_struct *mgr)
> u32 mysid = current_sid();
> u32 mgrsid = task_sid(mgr);
>
> + if (!compatible_task_displays(current, mgr))
> + return -EINVAL;
> +
> return avc_has_perm(&selinux_state,
> mysid, mgrsid, SECCLASS_BINDER,
> BINDER__SET_CONTEXT_MGR, NULL);
> @@ -2029,6 +2054,9 @@ static int selinux_binder_transaction(struct task_struct *from,
> u32 tosid = task_sid(to);
> int rc;
>
> + if (!compatible_task_displays(from, to))
> + return -EINVAL;
> +
> if (mysid != fromsid) {
> rc = avc_has_perm(&selinux_state,
> mysid, fromsid, SECCLASS_BINDER,
> @@ -2048,6 +2076,9 @@ static int selinux_binder_transfer_binder(struct task_struct *from,
> u32 fromsid = task_sid(from);
> u32 tosid = task_sid(to);
>
> + if (!compatible_task_displays(from, to))
> + return -EINVAL;
> +
> return avc_has_perm(&selinux_state,
> fromsid, tosid, SECCLASS_BINDER, BINDER__TRANSFER,
> NULL);
> @@ -2064,6 +2095,9 @@ static int selinux_binder_transfer_file(struct task_struct *from,
> struct common_audit_data ad;
> int rc;
>
> + if (!compatible_task_displays(from, to))
> + return -EINVAL;
> +
> ad.type = LSM_AUDIT_DATA_PATH;
> ad.u.path = file->f_path;
>
>
The only hook that approximates where/when you want to do this check is
security_binder_transaction(), so you don't need to insert the check
into the other hooks. Further, the check is only needed if the server
has requested peer contexts, which is indicated by the flag bit
target_node->txn_security_ctx, so if you amended the hook interface to
also pass that flag value, then you could avoid the check except in that
situation.
As soon as any other LSM implements the binder hooks, it will also
likely want an equivalent check, so sooner or later you'll probably want
to take the logic to the framework and not just the security module.
^ permalink raw reply
* Re: Can KEY_DH_OPERATIONS become tristate? (was: Re: Kernel 5.3.0 stuck during boot on Amiga)
From: Geert Uytterhoeven @ 2019-09-19 19:17 UTC (permalink / raw)
To: David Howells
Cc: John Paul Adrian Glaubitz, Michael Schmitz, linux-m68k,
Mat Martineau, James Morris, Serge E. Hallyn, keyrings,
Linux Crypto Mailing List, linux-security-module,
Linux Kernel Mailing List
In-Reply-To: <13304.1568825025@warthog.procyon.org.uk>
Hi David,
On Wed, Sep 18, 2019 at 6:43 PM David Howells <dhowells@redhat.com> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > TL;DR: CONFIG_CRYPTO_DH=y is reported to cause boot delays of several
> > > > minutes on old and slow machines.
> > >
> > > Why is it doing that? It doesn't do anything unless it is called, so
> > > something must be calling it.
> >
> > I don't know. Enabling initcall_debug shows that dh_init() takes a very long
> > time.
>
> Ah... The bit that handles keyctl_dh_compute() doesn't do anything unless
> asked, but the bit in the crypto layer that does dh does (ie. dh_init()). I
> guess it's doing some sort of self-test, but I can't see how it effects that.
> I think you need to consult the author/maintainer of crypto/dh.c.
Apparently the Debian kernel config had not enabled
CONFIG_CRYPTO_MANAGER_DISABLE_TESTS, so all crypto tests
were run at boot time :-(
> It might be possible to make CONFIG_KEY_DH_OPERATIONS not depend on
> CONFIG_CRYPTO_DH and have crypto_alloc_kpp() load the *crypto* part on
> demand. Failing that, I can look into demand-loading keyctl operations.
Regardless, it may be a good idea to make KEY_DH_OPERATIONS tristate
one day, so enabling wireless as a module doesn't force CONFIG_CRYPTO_DH
builtin.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v6 01/12] tpm-buf: move from static inlines to real functions
From: Jarkko Sakkinen @ 2019-09-20 14:06 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <1568031476.6613.30.camel@HansenPartnership.com>
On Mon, Sep 09, 2019 at 01:17:56PM +0100, James Bottomley wrote:
> This separates out the old tpm_buf_... handling functions from static
> inlines in tpm.h and makes them their own tpm-buf.c file. This is a
> precursor so we can add new functions for other TPM type handling
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
What about TPM_BUF_2B that gets added in this commit?
/Jarkko
^ permalink raw reply
* Re: [PATCH v6 01/12] tpm-buf: move from static inlines to real functions
From: Jarkko Sakkinen @ 2019-09-20 14:06 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <20190920140459.GA9578@linux.intel.com>
On Fri, Sep 20, 2019 at 05:06:15PM +0300, Jarkko Sakkinen wrote:
> On Mon, Sep 09, 2019 at 01:17:56PM +0100, James Bottomley wrote:
> > This separates out the old tpm_buf_... handling functions from static
> > inlines in tpm.h and makes them their own tpm-buf.c file. This is a
> > precursor so we can add new functions for other TPM type handling
> >
> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>
> What about TPM_BUF_2B that gets added in this commit?
Probably just a glitch in rebasing/squashing?
/Jarkko
^ permalink raw reply
* Re: [PATCH v6 02/12] tpm-buf: add handling for TPM2B types
From: Jarkko Sakkinen @ 2019-09-20 14:18 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <1568031515.6613.31.camel@HansenPartnership.com>
On Mon, Sep 09, 2019 at 01:18:35PM +0100, James Bottomley wrote:
> Most complex TPM commands require appending TPM2B buffers to the
> command body. Since TPM2B types are essentially variable size arrays,
> it makes it impossible to represent these complex command arguments as
> structures and we simply have to build them up using append primitives
> like these.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
I think a better idea would be to have headerless TPM buffers and also
it makes sense to have a separate length field in the struct to keep the
code sane given that sometimes the buffer does not store the length.
E.g.
enum tpm_buf_flags {
TPM_BUF_OVERFLOW = BIT(0),
TPM_BUF_HEADERLESS = BIT(1),
};
struct tpm_buf {
unsigned int length;
struct page *data_page;
unsigned int flags;
u8 *data;
};
/Jarkko
^ permalink raw reply
* Re: [PATCH v6 05/12] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling
From: Jarkko Sakkinen @ 2019-09-20 14:34 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <1568031657.6613.34.camel@HansenPartnership.com>
On Mon, Sep 09, 2019 at 01:20:57PM +0100, James Bottomley wrote:
> This code adds true session based HMAC authentication plus parameter
> decryption and response encryption using AES.
>
> The basic design of this code is to segregate all the nasty crypto,
> hash and hmac code into tpm2-sessions.c and export a usable API.
>
> The API first of all starts off by gaining a session with
>
> tpm2_start_auth_session()
>
> Which initiates a session with the TPM and allocates an opaque
> tpm2_auth structure to handle the session parameters. Then the use is
> simply:
>
> * tpm_buf_append_name() in place of the tpm_buf_append_u32 for the
> handles
>
> * tpm_buf_append_hmac_session() where tpm2_append_auth() would go
>
> * tpm_buf_fill_hmac_session() called after the entire command buffer
> is finished but before tpm_transmit_cmd() is called which computes
> the correct HMAC and places it in the command at the correct
> location.
>
> Finally, after tpm_transmit_cmd() is called,
> tpm_buf_check_hmac_response() is called to check that the returned
> HMAC matched and collect the new state for the next use of the
> session, if any.
>
> The features of the session is controlled by the session attributes
> set in tpm_buf_append_hmac_session(). If TPM2_SA_CONTINUE_SESSION is
> not specified, the session will be flushed and the tpm2_auth structure
> freed in tpm_buf_check_hmac_response(); otherwise the session may be
> used again. Parameter encryption is specified by or'ing the flag
> TPM2_SA_DECRYPT and response encryption by or'ing the flag
> TPM2_SA_ENCRYPT. the various encryptions will be taken care of by
> tpm_buf_fill_hmac_session() and tpm_buf_check_hmac_response()
> respectively.
>
> To get all of this to work securely, the Kernel now needs a primary
> key to encrypt the session salt to, so we derive an EC key from the
> NULL seed and save its context in the tpm_chip structure. The context
> is loaded on demand into an available volatile handle when
> tpm_start_auth_session() is called, but is flushed before that
> function exits to conserve handles.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> # crypto API parts
>
> ---
>
> v2: Added docbook and improved response check API
> v3: Add readpublic, fix hmac length, add API for close on error
> allow for the hmac session not being first in the sessions
> v4: Make NULL seed template exactly match the SRK ECC template.
> Also check the NULL primary key name is what getpublic returns
> to prevent spoofing. Also parametrise the name size for reuse
> v5: Move to sync_skcipher API
> v6: eliminate kernel space and use context save for null seed and
> make feature conditional on CONFIG_TPM_BUS_SECURITY
> ---
> drivers/char/tpm/Kconfig | 11 +
> drivers/char/tpm/Makefile | 1 +
> drivers/char/tpm/tpm-buf.c | 1 +
> drivers/char/tpm/tpm.h | 20 +-
> drivers/char/tpm/tpm2-cmd.c | 22 +-
> drivers/char/tpm/tpm2-sessions.c | 1203 ++++++++++++++++++++++++++++++++++++++
> drivers/char/tpm/tpm2-sessions.h | 137 +++++
> include/linux/tpm.h | 29 +
> 8 files changed, 1411 insertions(+), 13 deletions(-)
> create mode 100644 drivers/char/tpm/tpm2-sessions.c
> create mode 100644 drivers/char/tpm/tpm2-sessions.h
>
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 88a3c06fc153..96b09adfc163 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -9,6 +9,9 @@ menuconfig TCG_TPM
> imply SECURITYFS
> select CRYPTO
> select CRYPTO_HASH_INFO
> + select CRYPTO_ECDH
> + select CRYPTO_AES
> + select CRYPTO_CFB
> ---help---
> If you have a TPM security chip in your system, which
> implements the Trusted Computing Group's specification,
> @@ -27,6 +30,14 @@ menuconfig TCG_TPM
>
> if TCG_TPM
>
> +config TPM_BUS_SECURITY
> + bool "Use secure transactions on the TPM bus"
> + default y
> + ---help---
> + Setting this causes us to deploy a tamper resistent scheme
> + for communicating with the TPM to prevent or detect bus snooping
> + attacks like TPM Genie. Saying Y here adds some encryption overhead
> + to all kernel to TPM transactions.
> config HW_RANDOM_TPM
> bool "TPM HW Random Number Generator support"
> depends on TCG_TPM && HW_RANDOM && !(TCG_TPM=y && HW_RANDOM=m)
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 78bd025b808a..8f9e58317048 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -17,6 +17,7 @@ tpm-y += eventlog/tpm1.o
> tpm-y += eventlog/tpm2.o
> tpm-y += tpm-buf.o
>
> +tpm-$(CONFIG_TPM_BUS_SECURITY) += tpm2-sessions.o
> tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o
> tpm-$(CONFIG_EFI) += eventlog/efi.o
> tpm-$(CONFIG_OF) += eventlog/of.o
> diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
> index 553adb84b0ac..f56350123a08 100644
> --- a/drivers/char/tpm/tpm-buf.c
> +++ b/drivers/char/tpm/tpm-buf.c
> @@ -31,6 +31,7 @@ void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
> head->tag = cpu_to_be16(tag);
> head->length = cpu_to_be32(sizeof(*head));
> head->ordinal = cpu_to_be32(ordinal);
> + buf->handles = 0;
> }
> EXPORT_SYMBOL_GPL(tpm_buf_reset);
>
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 85a7302ddfeb..ebead8e4c3fe 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -80,6 +80,7 @@ enum tpm2_timeouts {
> enum tpm2_structures {
> TPM2_ST_NO_SESSIONS = 0x8001,
> TPM2_ST_SESSIONS = 0x8002,
> + TPM2_ST_CREATION = 0x8021,
> };
>
> /* Indicates from what layer of the software stack the error comes from */
> @@ -116,6 +117,8 @@ enum tpm2_command_codes {
> TPM2_CC_CONTEXT_LOAD = 0x0161,
> TPM2_CC_CONTEXT_SAVE = 0x0162,
> TPM2_CC_FLUSH_CONTEXT = 0x0165,
> + TPM2_CC_READ_PUBLIC = 0x0173,
> + TPM2_CC_START_AUTH_SESS = 0x0176,
> TPM2_CC_VERIFY_SIGNATURE = 0x0177,
> TPM2_CC_GET_CAPABILITY = 0x017A,
> TPM2_CC_GET_RANDOM = 0x017B,
> @@ -128,6 +131,7 @@ enum tpm2_command_codes {
> };
>
> enum tpm2_permanent_handles {
> + TPM2_RH_NULL = 0x40000007,
> TPM2_RS_PW = 0x40000009,
> };
>
> @@ -286,7 +290,8 @@ enum tpm_buf_flags {
>
> struct tpm_buf {
> struct page *data_page;
> - unsigned int flags;
> + u8 flags;
> + u8 handles;
> u8 *data;
> };
>
> @@ -306,6 +311,9 @@ u8 tpm_get_inc_u8(const u8 **ptr);
> u16 tpm_get_inc_u16(const u8 **ptr);
> u32 tpm_get_inc_u32(const u8 **ptr);
>
> +/* opaque structure, holds auth session parameters like the session key */
> +struct tpm2_auth;
> +
> extern struct class *tpm_class;
> extern struct class *tpmrm_class;
> extern dev_t tpm_devt;
> @@ -408,4 +416,14 @@ int tpm_bios_log_setup(struct tpm_chip *chip);
> void tpm_bios_log_teardown(struct tpm_chip *chip);
> int tpm_dev_common_init(void);
> void tpm_dev_common_exit(void);
> +
> +#ifdef CONFIG_TPM_BUS_SECURITY
> +int tpm2_sessions_init(struct tpm_chip *chip);
> +#else
> +static inline int tpm2_sessions_init(struct tpm_chip *chip)
> +{
> + return 0;
> +}
> +#endif
> +
> #endif
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index ba9acae83bff..d120b0a260eb 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -12,17 +12,10 @@
> */
>
> #include "tpm.h"
> +#include "tpm2-sessions.h"
> #include <crypto/hash_info.h>
> #include <keys/trusted-type.h>
>
> -enum tpm2_object_attributes {
> - TPM2_OA_USER_WITH_AUTH = BIT(6),
> -};
> -
> -enum tpm2_session_attributes {
> - TPM2_SA_CONTINUE_SESSION = BIT(0),
> -};
> -
> struct tpm2_hash {
> unsigned int crypto_id;
> unsigned int tpm_id;
> @@ -388,10 +381,10 @@ void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
> * @hmac: the session HMAC or password, may be NULL if not used
> * @hmac_len: the session HMAC or password length, maybe 0 if not used
> */
> -static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
> - const u8 *nonce, u16 nonce_len,
> - u8 attributes,
> - const u8 *hmac, u16 hmac_len)
> +void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
> + const u8 *nonce, u16 nonce_len,
> + u8 attributes,
> + const u8 *hmac, u16 hmac_len)
> {
> tpm_buf_append_u32(buf, 9 + nonce_len + hmac_len);
> tpm_buf_append_u32(buf, session_handle);
> @@ -1042,6 +1035,11 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>
> rc = tpm2_get_cc_attrs_tbl(chip);
>
> + if (rc)
> + goto out;
> +
> + rc = tpm2_sessions_init(chip);
> +
> out:
> if (rc > 0)
> rc = -ENODEV;
> diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> new file mode 100644
> index 000000000000..7307f061e5df
> --- /dev/null
> +++ b/drivers/char/tpm/tpm2-sessions.c
> @@ -0,0 +1,1203 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright (C) 2018 James.Bottomley@HansenPartnership.com
*/
/**
* DOC: TPM2 Sessions
> + *
> + * Cryptographic helper routines for handling TPM2 sessions for
> + * authorization HMAC and request response encryption.
> + *
> + * The idea is to ensure that every TPM command is HMAC protected by a
> + * session, meaning in-flight tampering would be detected and in
> + * addition all sensitive inputs and responses should be encrypted.
> + *
> + * The basic way this works is to use a TPM feature called salted
> + * sessions where a random secret used in session construction is
> + * encrypted to the public part of a known TPM key. The problem is we
> + * have no known keys, so initially a primary Elliptic Curve key is
> + * derived from the NULL seed (we use EC because most TPMs generate
> + * these keys much faster than RSA ones). The curve used is NIST_P256
> + * because that's now mandated to be present in 'TCG TPM v2.0
> + * Provisioning Guidance'
> + *
> + * Threat problems: the initial TPM2_CreatePrimary is not (and cannot
> + * be) session protected, so a clever Man in the Middle could return a
> + * public key they control to this command and from there intercept
> + * and decode all subsequent session based transactions. The kernel
> + * cannot mitigate this threat but, after boot, userspace can get
> + * proof this has not happened by asking the TPM to certify the NULL
> + * key. This certification would chain back to the TPM Endorsement
> + * Certificate and prove the NULL seed primary had not been tampered
> + * with and thus all sessions must have been cryptographically secure.
> + * To assist with this, the initial NULL seed public key name is made
> + * available in a sysfs file.
> + *
> + * Use of these functions:
> + *
> + * The design is all the crypto, hash and hmac gunk is confined in this
> + * file and never needs to be seen even by the kernel internal user. To
> + * the user there's an init function tpm2_sessions_init() that needs to
> + * be called once per TPM which generates the NULL seed primary key.
> + *
> + * Then there are six usage functions:
> + *
> + * tpm2_start_auth_session() which allocates the opaque auth structure
> + * and gets a session from the TPM. This must be called before
> + * any of the following functions. The session is protected by a
> + * session_key which is derived from a random salt value
> + * encrypted to the NULL seed.
> + * tpm2_end_auth_session() kills the session and frees the resources.
> + * Under normal operation this function is done by
> + * tpm_buf_check_hmac_response(), so this is only to be used on
> + * error legs where the latter is not executed.
> + * tpm_buf_append_name() to add a handle to the buffer. This must be
> + * used in place of the usual tpm_buf_append_u32() for adding
> + * handles because handles have to be processed specially when
> + * calculating the HMAC. In particular, for NV, volatile and
> + * permanent objects you now need to provide the name.
> + * tpm_buf_append_hmac_session() which appends the hmac session to the
> + * buf in the same way tpm_buf_append_auth does().
> + * tpm_buf_fill_hmac_session() This calculates the correct hash and
> + * places it in the buffer. It must be called after the complete
> + * command buffer is finalized so it can fill in the correct HMAC
> + * based on the parameters.
> + * tpm_buf_check_hmac_response() which checks the session response in
> + * the buffer and calculates what it should be. If there's a
> + * mismatch it will log a warning and return an error. If
> + * tpm_buf_append_hmac_session() did not specify
> + * TPM_SA_CONTINUE_SESSION then the session will be closed (if it
> + * hasn't been consumed) and the auth structure freed.
Should be reformatted to use kdoc e.g.
1. Documentation block in the file header (the last section in [1]).
2. Proper kdoc's for the functions.
This will allow generation of the documentation from this if done right
(e.g. like DRM people do).
> + */
> +
> +#include "tpm.h"
> +#include "tpm2-sessions.h"
> +
> +#include <linux/random.h>
> +#include <linux/scatterlist.h>
> +
> +#include <asm/unaligned.h>
> +
> +#include <crypto/aes.h>
> +#include <crypto/kpp.h>
> +#include <crypto/ecdh.h>
> +#include <crypto/hash.h>
> +#include <crypto/hmac.h>
> +#include <crypto/skcipher.h>
> +
> +/* if you change to AES256, you only need change this */
> +#define AES_KEYBYTES AES_KEYSIZE_128
Ugh, this just masks things, please remove.
> +
> +#define AES_KEYBITS (AES_KEYBYTES*8)
> +#define AUTH_MAX_NAMES 3
> +
> +/*
> + * This is the structure that carries all the auth information (like
> + * session handle, nonces, session key and auth) from use to use it is
> + * designed to be opaque to anything outside.
> + */
> +struct tpm2_auth {
> + u32 handle;
> + /*
> + * This has two meanings: before tpm_buf_fill_hmac_session()
> + * it marks the offset in the buffer of the start of the
> + * sessions (i.e. after all the handles). Once the buffer has
> + * been filled it markes the session number of our auth
> + * session so we can find it again in the response buffer.
> + *
> + * The two cases are distinguished because the first offset
> + * must always be greater than TPM_HEADER_SIZE and the second
> + * must be less than or equal to 5.
> + */
> + u32 session;
> + /*
> + * the size here is variable and set by the size of our_nonce
> + * which must be between 16 and the name hash length. we set
> + * the maximum sha256 size for the greatest protection
> + */
> + u8 our_nonce[SHA256_DIGEST_SIZE];
> + u8 tpm_nonce[SHA256_DIGEST_SIZE];
> + /*
> + * the salt is only used across the session command/response
> + * after that it can be used as a scratch area
> + */
> + union {
> + u8 salt[EC_PT_SZ];
> + /* scratch for key + IV */
> + u8 scratch[AES_KEYBYTES + AES_BLOCK_SIZE];
> + };
> + /*
> + * the session key and passphrase are the same size as the
> + * name digest (sha256 again). The session key is constant
> + * for the use of the session and the passphrase can change
> + * with every invocation.
> + *
> + * Note: these fields must be adjacent and in this order
> + * because several HMAC/KDF schemes use the combination of the
> + * session_key and passphrase.
> + */
> + u8 session_key[SHA256_DIGEST_SIZE];
> + u8 passphrase[SHA256_DIGEST_SIZE];
> + int passphraselen;
> + /* saved session attributes */
> + u8 attrs;
> + __be32 ordinal;
> + struct crypto_sync_skcipher *aes;
> + struct tpm_chip *chip;
> + /* 3 names of handles: name_h is handle, name is name of handle */
> + u32 name_h[AUTH_MAX_NAMES];
> + u8 name[AUTH_MAX_NAMES][2 + SHA256_DIGEST_SIZE];
> +};
Check "kernel-doc for structs, unions, enums, and typedefs" in [1].
Look at iit this way: you've done a lot of work to explain why you
are doing what you are doing and that is great. If we can generate
documentation from it, it will be more accessible to everyone and does
not get buried inside the source tree :-)
[1] https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt
/Jarkko
^ permalink raw reply
* Re: [PATCH v6 05/12] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling
From: Jarkko Sakkinen @ 2019-09-20 14:35 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <20190920143337.GD9578@linux.intel.com>
On Fri, Sep 20, 2019 at 05:34:00PM +0300, Jarkko Sakkinen wrote:
> On Mon, Sep 09, 2019 at 01:20:57PM +0100, James Bottomley wrote:
Forgot to ask: what is the new field handles?
/Jarkko
^ permalink raw reply
* Re: [PATCH v6 01/12] tpm-buf: move from static inlines to real functions
From: James Bottomley @ 2019-09-20 15:53 UTC (permalink / raw)
To: Jarkko Sakkinen; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <20190920140642.GB9578@linux.intel.com>
On Fri, 2019-09-20 at 17:06 +0300, Jarkko Sakkinen wrote:
> On Fri, Sep 20, 2019 at 05:06:15PM +0300, Jarkko Sakkinen wrote:
> > On Mon, Sep 09, 2019 at 01:17:56PM +0100, James Bottomley wrote:
> > > This separates out the old tpm_buf_... handling functions from
> > > static
> > > inlines in tpm.h and makes them their own tpm-buf.c file. This
> > > is a
> > > precursor so we can add new functions for other TPM type handling
> > >
> > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership
> > > .com>
> >
> > What about TPM_BUF_2B that gets added in this commit?
>
> Probably just a glitch in rebasing/squashing?
Well a glitch in splitting one patch into three, yes. I'll fix it up.
James
^ permalink raw reply
* [PATCH] smack: fix an compile error in smack_post_notification
From: zhong jiang @ 2019-09-23 3:16 UTC (permalink / raw)
To: casey; +Cc: jmorris, serge, linux-security-module, zhongjiang, linux-kernel
I hit the following error when compile the kernel.
security/smack/smack_lsm.c: In function smack_post_notification:
security/smack/smack_lsm.c:4383:7: error: dereferencing pointer to incomplete type struct watch_notification
if (n->type == WATCH_TYPE_META)
^~
security/smack/smack_lsm.c:4383:17: error: WATCH_TYPE_META undeclared (first use in this function); did you mean TCA_PIE_BETA?
if (n->type == WATCH_TYPE_META)
^~~~~~~~~~~~~~~
TCA_PIE_BETA
security/smack/smack_lsm.c:4383:17: note: each undeclared identifier is reported only once for each function it appears in
Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
security/smack/smack.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/security/smack/smack.h b/security/smack/smack.h
index 62529f3..02b05a2 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -21,6 +21,7 @@
#include <linux/rculist.h>
#include <linux/lsm_audit.h>
#include <linux/msg.h>
+#include <linux/watch_queue.h>
/*
* Use IPv6 port labeling if IPv6 is enabled and secmarks
--
1.7.12.4
^ permalink raw reply related
* Re: [PATCH] smack: fix an compile error in smack_post_notification
From: kbuild test robot @ 2019-09-23 5:17 UTC (permalink / raw)
To: zhong jiang
Cc: kbuild-all, casey, jmorris, serge, linux-security-module,
zhongjiang, linux-kernel
In-Reply-To: <1569208607-23263-1-git-send-email-zhongjiang@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]
Hi zhong,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.3 next-20190920]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/zhong-jiang/smack-fix-an-compile-error-in-smack_post_notification/20190923-112403
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-13) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from security/smack/smack_lsm.c:45:0:
>> security/smack/smack.h:24:10: fatal error: linux/watch_queue.h: No such file or directory
#include <linux/watch_queue.h>
^~~~~~~~~~~~~~~~~~~~~
compilation terminated.
vim +24 security/smack/smack.h
11
12 #include <linux/capability.h>
13 #include <linux/spinlock.h>
14 #include <linux/lsm_hooks.h>
15 #include <linux/in.h>
16 #if IS_ENABLED(CONFIG_IPV6)
17 #include <linux/in6.h>
18 #endif /* CONFIG_IPV6 */
19 #include <net/netlabel.h>
20 #include <linux/list.h>
21 #include <linux/rculist.h>
22 #include <linux/lsm_audit.h>
23 #include <linux/msg.h>
> 24 #include <linux/watch_queue.h>
25
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 70053 bytes --]
^ permalink raw reply
* Re: [PATCH v8 04/28] LSM: Create and manage the lsmblob data structure.
From: Casey Schaufler @ 2019-09-23 15:56 UTC (permalink / raw)
To: Stephen Smalley, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, john.johansen, penguin-kernel, paul, casey
In-Reply-To: <a95a0c6a-1410-471a-8397-fdcc45a8c77e@tycho.nsa.gov>
On 9/16/2019 12:15 PM, Stephen Smalley wrote:
> On 8/29/19 7:29 PM, Casey Schaufler wrote:
>> When more than one security module is exporting data to
>> audit and networking sub-systems a single 32 bit integer
>> is no longer sufficient to represent the data. Add a
>> structure to be used instead.
>>
>> The lsmblob structure is currently an array of
>> u32 "secids". There is an entry for each of the
>> security modules built into the system that would
>> use secids if active. The system assigns the module
>> a "slot" when it registers hooks. If modules are
>> compiled in but not registered there will be unused
>> slots.
>
> I won't obstruct the patch on naming but I do have to wonder about the use of lsmblob here given that we use the term "security blobs" elsewhere to refer to what the ->security fields reference and this is not the same thing. Not sure why it wasn't just lsmsecids or similar, but whatever.
In earlier versions of the patch I used "struct secids" and found
it incredibly difficult to recognize the difference between uses
of a secid and a struct secids. Brains trained in the English language
tend to treat an ending 's' as a modifier, making the distinction
difficult to see. I also have not given up on the possibility of
moving the interfaces away from using secids at some point in the
future.
>
>>
>> A new lsm_id structure, which contains the name
>> of the LSM and its slot number, is created. There
>> is an instance for each LSM, which assigns the name
>> and passes it to the infrastructure to set the slot.
>
> Is it really desirable/necessary to duplicate the module name, once in the lsm_id and once in the lsm_info? Again, not a blocker for me but seems unfortunate.
Possibly not, but with one of them being __init data it's
really hard to justify putting a lot of effort into avoiding
the duplication.
>
>>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>> Reviewed-by: John Johansen <john.johansen@canonical.com>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>> include/linux/lsm_hooks.h | 12 +++++--
>> include/linux/security.h | 66 ++++++++++++++++++++++++++++++++++++++
>> security/apparmor/lsm.c | 7 +++-
>> security/commoncap.c | 7 +++-
>> security/loadpin/loadpin.c | 8 ++++-
>> security/safesetid/lsm.c | 8 ++++-
>> security/security.c | 31 ++++++++++++++----
>> security/selinux/hooks.c | 8 ++++-
>> security/smack/smack_lsm.c | 7 +++-
>> security/tomoyo/tomoyo.c | 8 ++++-
>> security/yama/yama_lsm.c | 7 +++-
>> 11 files changed, 152 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 3fe39abccc8f..fe1fb7a69ee5 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -2029,6 +2029,14 @@ struct security_hook_heads {
>> #endif /* CONFIG_BPF_SYSCALL */
>> } __randomize_layout;
>> +/*
>> + * Information that identifies a security module.
>> + */
>> +struct lsm_id {
>> + const char *lsm; /* Name of the LSM */
>> + int slot; /* Slot in lsmblob if one is allocated */
>> +};
>> +
>> /*
>> * Security module hook list structure.
>> * For use with generic list macros for common operations.
>> @@ -2037,7 +2045,7 @@ struct security_hook_list {
>> struct hlist_node list;
>> struct hlist_head *head;
>> union security_list_options hook;
>> - char *lsm;
>> + struct lsm_id *lsmid;
>> } __randomize_layout;
>> /*
>> @@ -2068,7 +2076,7 @@ extern struct security_hook_heads security_hook_heads;
>> extern char *lsm_names;
>> extern void security_add_hooks(struct security_hook_list *hooks, int count,
>> - char *lsm);
>> + struct lsm_id *lsmid);
>> #define LSM_FLAG_LEGACY_MAJOR BIT(0)
>> #define LSM_FLAG_EXCLUSIVE BIT(1)
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 49f2685324b0..5bb8b9a6fa84 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -76,6 +76,72 @@ enum lsm_event {
>> LSM_POLICY_CHANGE,
>> };
>> +/*
>> + * Data exported by the security modules
>> + *
>> + * Any LSM that provides secid or secctx based hooks must be included.
>> + */
>> +#define LSMBLOB_ENTRIES ( \
>> + (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0))
>> +
>> +struct lsmblob {
>> + u32 secid[LSMBLOB_ENTRIES];
>> +};
>> +
>> +#define LSMBLOB_INVALID -1 /* Not a valid LSM slot number */
>> +#define LSMBLOB_NEEDED -2 /* Slot requested on initialization */
>> +#define LSMBLOB_NOT_NEEDED -3 /* Slot not requested */
>> +
>> +/**
>> + * lsmblob_init - initialize an lsmblob structure.
>> + * @blob: Pointer to the data to initialize
>> + * @secid: The initial secid value
>> + *
>> + * Set all secid for all modules to the specified value.
>> + */
>> +static inline void lsmblob_init(struct lsmblob *blob, u32 secid)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < LSMBLOB_ENTRIES; i++)
>> + blob->secid[i] = secid;
>> +}
>
> This seems nonsensical for any value other than 0. Otherwise, we should only set it for the module that produced the secid originally? And if setting them all to zero, you could just do a memset.
Yes, it does, but it isn't. There are cases where setting all of the
secids to a value other than zero makes sense, at least when there's
scaffolding involved. One example is with secmarks, where we know the
secid value, but not necessarily which security module is responsible
for it. We know that it will be set and consumed by the same module,
but not which it is at that point. You can argue this should be
lsmblob_set() instead of lsmblob_init(), but that's confusing, too.
>
>> +
>> +/**
>> + * lsmblob_is_set - report if there is an value in the lsmblob
>> + * @blob: Pointer to the exported LSM data
>> + *
>> + * Returns true if there is a secid set, false otherwise
>> + */
>> +static inline bool lsmblob_is_set(struct lsmblob *blob)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < LSMBLOB_ENTRIES; i++)
>> + if (blob->secid[i] != 0)
>> + return true;
>> + return false;
>
> memcmp against the all-zeroes lsmblob?
Sure.
>
>> +}
>> +
>> +/**
>> + * lsmblob_equal - report if the two lsmblob's are equal
>> + * @bloba: Pointer to one LSM data
>> + * @blobb: Pointer to the other LSM data
>> + *
>> + * Returns true if all entries in the two are equal, false otherwise
>> + */
>> +static inline bool lsmblob_equal(struct lsmblob *bloba, struct lsmblob *blobb)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < LSMBLOB_ENTRIES; i++)
>> + if (bloba->secid[i] != blobb->secid[i])
>> + return false;
>> + return true;
>
> memcmp of the two blobs?
Sure.
>
>> +}
>> +
>> /* These functions are in security/commoncap.c */
>> extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
>> int cap, unsigned int opts);
>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>> index 2716e7731279..ec2e39aa9a84 100644
>> --- a/security/apparmor/lsm.c
>> +++ b/security/apparmor/lsm.c
>> @@ -1138,6 +1138,11 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
>> .lbs_sock = sizeof(struct aa_sk_ctx),
>> };
>> +static struct lsm_id apparmor_lsmid __lsm_ro_after_init = {
>> + .lsm = "apparmor",
>> + .slot = LSMBLOB_NEEDED
>> +};
>> +
>> static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check),
>> LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme),
>> @@ -1679,7 +1684,7 @@ static int __init apparmor_init(void)
>> goto buffers_out;
>> }
>> security_add_hooks(apparmor_hooks, ARRAY_SIZE(apparmor_hooks),
>> - "apparmor");
>> + &apparmor_lsmid);
>> /* Report that AppArmor successfully initialized */
>> apparmor_initialized = 1;
>> diff --git a/security/commoncap.c b/security/commoncap.c
>> index afd9679ca866..973e6c7009d0 100644
>> --- a/security/commoncap.c
>> +++ b/security/commoncap.c
>> @@ -1344,6 +1344,11 @@ int cap_mmap_file(struct file *file, unsigned long reqprot,
>> #ifdef CONFIG_SECURITY
>> +static struct lsm_id capability_lsmid __lsm_ro_after_init = {
>> + .lsm = "capability",
>> + .slot = LSMBLOB_NOT_NEEDED
>> +};
>> +
>> static struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(capable, cap_capable),
>> LSM_HOOK_INIT(settime, cap_settime),
>> @@ -1368,7 +1373,7 @@ static struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
>> static int __init capability_init(void)
>> {
>> security_add_hooks(capability_hooks, ARRAY_SIZE(capability_hooks),
>> - "capability");
>> + &capability_lsmid);
>> return 0;
>> }
>> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
>> index 055fb0a64169..7b23fdf24e27 100644
>> --- a/security/loadpin/loadpin.c
>> +++ b/security/loadpin/loadpin.c
>> @@ -181,6 +181,11 @@ static int loadpin_load_data(enum kernel_load_data_id id)
>> return loadpin_read_file(NULL, (enum kernel_read_file_id) id);
>> }
>> +static struct lsm_id loadpin_lsmid __lsm_ro_after_init = {
>> + .lsm = "loadpin",
>> + .slot = LSMBLOB_NOT_NEEDED
>> +};
>> +
>> static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
>> LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
>> @@ -191,7 +196,8 @@ static int __init loadpin_init(void)
>> {
>> pr_info("ready to pin (currently %senforcing)\n",
>> enforce ? "" : "not ");
>> - security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
>> + security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks),
>> + &loadpin_lsmid);
>> return 0;
>> }
>> diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
>> index cecd38e2ac80..4a96cd8c0d15 100644
>> --- a/security/safesetid/lsm.c
>> +++ b/security/safesetid/lsm.c
>> @@ -255,6 +255,11 @@ void flush_safesetid_whitelist_entries(void)
>> }
>> }
>> +static struct lsm_id safesetid_lsmid __lsm_ro_after_init = {
>> + .lsm = "safesetid",
>> + .slot = LSMBLOB_NOT_NEEDED
>> +};
>> +
>> static struct security_hook_list safesetid_security_hooks[] = {
>> LSM_HOOK_INIT(task_fix_setuid, safesetid_task_fix_setuid),
>> LSM_HOOK_INIT(capable, safesetid_security_capable)
>> @@ -263,7 +268,8 @@ static struct security_hook_list safesetid_security_hooks[] = {
>> static int __init safesetid_security_init(void)
>> {
>> security_add_hooks(safesetid_security_hooks,
>> - ARRAY_SIZE(safesetid_security_hooks), "safesetid");
>> + ARRAY_SIZE(safesetid_security_hooks),
>> + &safesetid_lsmid);
>> /* Report that SafeSetID successfully initialized */
>> safesetid_initialized = 1;
>> diff --git a/security/security.c b/security/security.c
>> index 7cfedb90210a..27e2db3d6b04 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -317,6 +317,7 @@ static void __init ordered_lsm_init(void)
>> init_debug("sock blob size = %d\n", blob_sizes.lbs_sock);
>> init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
>> init_debug("task blob size = %d\n", blob_sizes.lbs_task);
>> + init_debug("lsmblob size = %lu\n", sizeof(struct lsmblob));
>> /*
>> * Create any kmem_caches needed for blobs
>> @@ -399,7 +400,7 @@ static bool match_last_lsm(const char *list, const char *lsm)
>> return !strcmp(last, lsm);
>> }
>> -static int lsm_append(char *new, char **result)
>> +static int lsm_append(const char *new, char **result)
>> {
>> char *cp;
>> @@ -420,24 +421,40 @@ static int lsm_append(char *new, char **result)
>> return 0;
>> }
>> +/*
>> + * Current index to use while initializing the lsmblob secid list.
>> + */
>> +static int lsm_slot __initdata;
>> +
>> /**
>> * security_add_hooks - Add a modules hooks to the hook lists.
>> * @hooks: the hooks to add
>> * @count: the number of hooks to add
>> - * @lsm: the name of the security module
>> + * @lsmid: the identification information for the security module
>> *
>> * Each LSM has to register its hooks with the infrastructure.
>> + * If the LSM is using hooks that export secids allocate a slot
>> + * for it in the lsmblob.
>> */
>> void __init security_add_hooks(struct security_hook_list *hooks, int count,
>> - char *lsm)
>> + struct lsm_id *lsmid)
>> {
>> int i;
>> + if (lsmid->slot == LSMBLOB_NEEDED) {
>> + if (lsm_slot >= LSMBLOB_ENTRIES)
>> + panic("%s Too many LSMs registered.\n", __func__);
>> + lsmid->slot = lsm_slot++;
>> + init_debug("%s assigned lsmblob slot %d\n", lsmid->lsm,
>> + lsmid->slot);
>> + }
>> +
>> for (i = 0; i < count; i++) {
>> - hooks[i].lsm = lsm;
>> + hooks[i].lsmid = lsmid;
>> hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
>> }
>> - if (lsm_append(lsm, &lsm_names) < 0)
>> +
>> + if (lsm_append(lsmid->lsm, &lsm_names) < 0)
>> panic("%s - Cannot get early memory.\n", __func__);
>> }
>> @@ -1917,7 +1934,7 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
>> struct security_hook_list *hp;
>> hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
>> - if (lsm != NULL && strcmp(lsm, hp->lsm))
>> + if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>> continue;
>> return hp->hook.getprocattr(p, name, value);
>> }
>> @@ -1930,7 +1947,7 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
>> struct security_hook_list *hp;
>> hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
>> - if (lsm != NULL && strcmp(lsm, hp->lsm))
>> + if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>> continue;
>> return hp->hook.setprocattr(name, value, size);
>> }
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index c83ec2652eda..74c491980ed2 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -6622,6 +6622,11 @@ struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = {
>> .lbs_superblock = sizeof(struct superblock_security_struct),
>> };
>> +static struct lsm_id selinux_lsmid __lsm_ro_after_init = {
>> + .lsm = "selinux",
>> + .slot = LSMBLOB_NEEDED
>> +};
>> +
>> static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr),
>> LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction),
>> @@ -6877,7 +6882,8 @@ static __init int selinux_init(void)
>> hashtab_cache_init();
>> - security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
>> + security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks),
>> + &selinux_lsmid);
>> if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
>> panic("SELinux: Unable to register AVC netcache callback\n");
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index e9560b078efe..7a0ead4da479 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -4553,6 +4553,11 @@ struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
>> .lbs_superblock = sizeof(struct superblock_smack),
>> };
>> +static struct lsm_id smack_lsmid __lsm_ro_after_init = {
>> + .lsm = "smack",
>> + .slot = LSMBLOB_NEEDED
>> +};
>> +
>> static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(ptrace_access_check, smack_ptrace_access_check),
>> LSM_HOOK_INIT(ptrace_traceme, smack_ptrace_traceme),
>> @@ -4743,7 +4748,7 @@ static __init int smack_init(void)
>> /*
>> * Register with LSM
>> */
>> - security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
>> + security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), &smack_lsmid);
>> smack_enabled = 1;
>> pr_info("Smack: Initializing.\n");
>> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
>> index 716c92ec941a..f1968e80f06d 100644
>> --- a/security/tomoyo/tomoyo.c
>> +++ b/security/tomoyo/tomoyo.c
>> @@ -529,6 +529,11 @@ static void tomoyo_task_free(struct task_struct *task)
>> }
>> }
>> +static struct lsm_id tomoyo_lsmid __lsm_ro_after_init = {
>> + .lsm = "tomoyo",
>> + .slot = LSMBLOB_NOT_NEEDED
>> +};
>> +
>> /*
>> * tomoyo_security_ops is a "struct security_operations" which is used for
>> * registering TOMOYO.
>> @@ -581,7 +586,8 @@ static int __init tomoyo_init(void)
>> struct tomoyo_task *s = tomoyo_task(current);
>> /* register ourselves with the security framework */
>> - security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks), "tomoyo");
>> + security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks),
>> + &tomoyo_lsmid);
>> pr_info("TOMOYO Linux initialized\n");
>> s->domain_info = &tomoyo_kernel_domain;
>> atomic_inc(&tomoyo_kernel_domain.users);
>> diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
>> index efac68556b45..0529ecc86954 100644
>> --- a/security/yama/yama_lsm.c
>> +++ b/security/yama/yama_lsm.c
>> @@ -425,6 +425,11 @@ static int yama_ptrace_traceme(struct task_struct *parent)
>> return rc;
>> }
>> +static struct lsm_id yama_lsmid __lsm_ro_after_init = {
>> + .lsm = "yama",
>> + .slot = LSMBLOB_NOT_NEEDED
>> +};
>> +
>> static struct security_hook_list yama_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
>> LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
>> @@ -482,7 +487,7 @@ static inline void yama_init_sysctl(void) { }
>> static int __init yama_init(void)
>> {
>> pr_info("Yama: becoming mindful.\n");
>> - security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), "yama");
>> + security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), &yama_lsmid);
>> yama_init_sysctl();
>> return 0;
>> }
>>
>
^ permalink raw reply
* [GIT PULL] Smack patches for v5.4
From: Casey Schaufler @ 2019-09-23 17:24 UTC (permalink / raw)
To: Linus Torvalds; +Cc: LKML, Linux Security Module list, casey
[-- Attachment #1.1: Type: text/plain, Size: 1321 bytes --]
Hello Linus
I have four patches for v5.4. Nothing is major. All but one are in
response to mechanically detected potential issues. The remaining
patch cleans up kernel-doc notations.
This is my first direct pull request. I think I have followed process
correctly, but if not I will attend to my error as required.
The following changes since commit 0ecfebd2b52404ae0c54a878c872bb93363ada36:
Linux 5.2 (2019-07-07 15:41:56 -0700)
are available in the Git repository at:
https://github.com/cschaufler/smack-next.git smack-for-5.4
for you to fetch changes up to e5bfad3d7acc5702f32aafeb388362994f4d7bd0:
smack: use GFP_NOFS while holding inode_smack::smk_lock (2019-09-04 09:37:07 -0700)
----------------------------------------------------------------
Eric Biggers (1):
smack: use GFP_NOFS while holding inode_smack::smk_lock
Jann Horn (1):
Smack: Don't ignore other bprm->unsafe flags if LSM_UNSAFE_PTRACE is set
Jia-Ju Bai (1):
security: smack: Fix possible null-pointer dereferences in smack_socket_sock_rcv_skb()
luanshi (1):
smack: fix some kernel-doc notations
security/smack/smack_access.c | 6 +++---
security/smack/smack_lsm.c | 40 ++++++++++++++++++++--------------------
2 files changed, 23 insertions(+), 23 deletions(-)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [GIT PULL] SafeSetID LSM changes for 5.4
From: Linus Torvalds @ 2019-09-23 19:01 UTC (permalink / raw)
To: Micah Morton, Jann Horn, Bart Van Assche, Paul E. McKenney
Cc: Linux Kernel Mailing List, linux-security-module
In-Reply-To: <CAJ-EccM49yBA+xgkR+3m5pEAJqmH_+FxfuAjijrQxaxxMUAt3Q@mail.gmail.com>
On Wed, Sep 18, 2019 at 10:41 AM Micah Morton <mortonm@chromium.org> wrote:
>
> Fix for SafeSetID bug that was introduced in 5.3
So this seems to be a good fix, but the bug itself came from the fact that
rcu_swap_protected(..)
is so hard to read, and I don't see *why* it's so pointlessly hard to read.
Yes, we have some macros that change their arguments, but they have a
_reason_ to do so (ie they return two different values) and they tend
to be very special in other ways too.
But rcu_swap_protected() has no reason for it's odd semantics.
Looking at that 'handle_policy_update()' function, it's entirely
reasonable to think "pol cannot possibly be NULL". When I looked at
the fix patch, that was my initial reaction too, and it's probably the
reason Jann's commit 03638e62f55f ("LSM: SafeSetID: rewrite userspace
API to atomic updates") had that bug to begin with.
I don't see the original discussion at all, it's not on
Linux-Security-Module for some reason, so I can't tell when/if the
NULL pointer test got deleted.
Anyway, this bug would likely had been avoided if rcu_swap_protected()
just returned the old pointer instead of changing the argument.
There are only a handful or users of that macro, maybe this could be fixed?
Adding some of the RCU parties to the participants..
Also, the commit message for this fix was a mess, I feel. It says
"SafeSetID: Stop releasing uninitialized ruleset", but the ruleset it
releases is perfectly initialized. It just might be NULL because it
doesn't _exist_.
Linus
^ permalink raw reply
* Re: [GIT PULL] SELinux patches for v5.4
From: pr-tracker-bot @ 2019-09-23 19:05 UTC (permalink / raw)
To: Paul Moore; +Cc: Linus Torvalds, selinux, linux-kernel, linux-security-module
In-Reply-To: <CAHC9VhT1n=zwWJRSqF+OLzQq2r_8Bf0TjO-1QEe3yfLHAnomfA@mail.gmail.com>
The pull request you sent on Tue, 17 Sep 2019 15:38:05 -0400:
> git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git tags/selinux-pr-20190917
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/5825a95fe92566ada2292a65de030850b5cff1da
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [GIT PULL] SafeSetID LSM changes for 5.4
From: pr-tracker-bot @ 2019-09-23 19:05 UTC (permalink / raw)
To: Micah Morton
Cc: Linus Torvalds, Linux Kernel Mailing List, linux-security-module
In-Reply-To: <CAJ-EccM49yBA+xgkR+3m5pEAJqmH_+FxfuAjijrQxaxxMUAt3Q@mail.gmail.com>
The pull request you sent on Wed, 18 Sep 2019 10:41:06 -0700:
> https://github.com/micah-morton/linux.git tags/safesetid-bugfix-5.4
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/1b5fb415442eb3ec946d48afe8c87b0f2fd42d7c
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [GIT PULL] Smack patches for v5.4
From: Linus Torvalds @ 2019-09-23 19:17 UTC (permalink / raw)
To: Casey Schaufler; +Cc: LKML, Linux Security Module list
In-Reply-To: <d2418da0-056b-2e6f-ae40-acdfa842e341@schaufler-ca.com>
On Mon, Sep 23, 2019 at 10:24 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> This is my first direct pull request. I think I have followed process
> correctly, but if not I will attend to my error as required.
The contents look fine.
However, it's from an open hosting site - github. Which is fine, I
take pull requests from github all the time. But I require that they
be sent using a signed tag, so that I can verify that yes, it's really
from you.
And no, I don't do pgp email, even t hough I see that there's a
signature on your email itself.
git uses pgp too, but unlike pgp email signatures, the git support for
pgp signing is useful and user-friendly and just _works_, rather than
the complete and useless disaster that is pgp email [1].
So please make it a signed tag with "git tag -s" and ask me to pull
that tag instead.
Linus
[1] https://www.vice.com/en_us/article/vvbw9a/even-the-inventor-of-pgp-doesnt-use-pgp
^ permalink raw reply
* Re: [GIT PULL] SafeSetID LSM changes for 5.4
From: Linus Torvalds @ 2019-09-23 19:28 UTC (permalink / raw)
To: Micah Morton, Jann Horn, Bart Van Assche, Paul E. McKenney
Cc: Linux Kernel Mailing List, linux-security-module
In-Reply-To: <CAHk-=wiAsJLw1egFEE=Z7-GGtM6wcvtyytXZA1+BHqta4gg6Hw@mail.gmail.com>
On Mon, Sep 23, 2019 at 12:01 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Anyway, this bug would likely had been avoided if rcu_swap_protected()
> just returned the old pointer instead of changing the argument.
Also, I have to say that the fact that I got the fundamentally buggy
commit in a pull request during the 5.3 merge window, and merged it
on July 16, but then get the pull request for the fix two months
later, after 5.3 has been released, makes me very unhappy with the
state of safesetid.
The pull request itself was clearly never tested. That's a big problem.
And *nobody* used it at all or tested it at all during the whole
release process. That's another big problem.
Should we just remove safesetid again? It's not really maintained, and
it's apparently not used. It was merged in March (with the first
commit in January), and here we are at end of September and this
happens.
So yes, syntactically I'll blame the bad RCU interfaces for why the
bug happened.
But the fact that the code didn't _work_ and was never tested by
anybody for two months, that's not the fault of the RCU code.
Linus
^ permalink raw reply
* [GIT PULL] Smack patches for v5.4 - retry
From: Casey Schaufler @ 2019-09-23 20:14 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Linux Security Module list, LKML, casey
Hello Linus
Thank for the instruction. I think this is correct.
I have four patches for v5.4. Nothing is major. All but one are in
response to mechanically detected potential issues. The remaining
patch cleans up kernel-doc notations.
The following changes since commit 0ecfebd2b52404ae0c54a878c872bb93363ada36:
Linux 5.2 (2019-07-07 15:41:56 -0700)
are available in the Git repository at:
https://github.com/cschaufler/smack-next.git tags/smack-for-5.4-rc1
for you to fetch changes up to e5bfad3d7acc5702f32aafeb388362994f4d7bd0:
smack: use GFP_NOFS while holding inode_smack::smk_lock (2019-09-04 09:37:07 -0700)
----------------------------------------------------------------
I have four patches for v5.4. Nothing is major. All but one are in
response to mechanically detected potential issues. The remaining
patch cleans up kernel-doc notations.
----------------------------------------------------------------
Eric Biggers (1):
smack: use GFP_NOFS while holding inode_smack::smk_lock
Jann Horn (1):
Smack: Don't ignore other bprm->unsafe flags if LSM_UNSAFE_PTRACE is set
Jia-Ju Bai (1):
security: smack: Fix possible null-pointer dereferences in smack_socket_sock_rcv_skb()
luanshi (1):
smack: fix some kernel-doc notations
security/smack/smack_access.c | 6 +++---
security/smack/smack_lsm.c | 40 ++++++++++++++++++++--------------------
2 files changed, 23 insertions(+), 23 deletions(-)
^ 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