05 Jun, 2018

1 commit

  • commit efe3de79e0b52ca281ef6691480c8c68c82a4657 upstream.

    Call trace:
    [] dump_backtrace+0x0/0x428
    [] show_stack+0x28/0x38
    [] dump_stack+0xd4/0x124
    [] print_address_description+0x68/0x258
    [] kasan_report.part.2+0x228/0x2f0
    [] kasan_report+0x5c/0x70
    [] check_memory_region+0x12c/0x1c0
    [] memcpy+0x34/0x68
    [] xattr_getsecurity+0xe0/0x160
    [] vfs_getxattr+0xc8/0x120
    [] getxattr+0x100/0x2c8
    [] SyS_fgetxattr+0x64/0xa0
    [] el0_svc_naked+0x24/0x28

    If user get root access and calls security.selinux setxattr() with an
    embedded NUL on a file and then if some process performs a getxattr()
    on that file with a length greater than the actual length of the string,
    it would result in a panic.

    To fix this, add the actual length of the string to the security context
    instead of the length passed by the userspace process.

    Signed-off-by: Sachin Grover
    Cc: stable@vger.kernel.org
    Signed-off-by: Paul Moore
    Signed-off-by: Greg Kroah-Hartman

    Sachin Grover
     

30 May, 2018

3 commits

  • [ Upstream commit ab60368ab6a452466885ef4edf0cefd089465132 ]

    IMA requires having it's hash algorithm be compiled-in due to it's
    early use. The default IMA algorithm is protected by Kconfig to be
    compiled-in.

    The ima_hash kernel parameter allows to choose the hash algorithm. When
    the specified algorithm is not available or available as a module, IMA
    initialization fails, which leads to a kernel panic (mknodat syscall calls
    ima_post_path_mknod()). Therefore as fallback we force IMA to use
    the default builtin Kconfig hash algorithm.

    Fixed crash:

    $ grep CONFIG_CRYPTO_MD4 .config
    CONFIG_CRYPTO_MD4=m

    [ 0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-4.12.14-2.3-default root=UUID=74ae8202-9ca7-4e39-813b-22287ec52f7a video=1024x768-16 plymouth.ignore-serial-consoles console=ttyS0 console=tty resume=/dev/disk/by-path/pci-0000:00:07.0-part3 splash=silent showopts ima_hash=md4
    ...
    [ 1.545190] ima: Can not allocate md4 (reason: -2)
    ...
    [ 2.610120] BUG: unable to handle kernel NULL pointer dereference at (null)
    [ 2.611903] IP: ima_match_policy+0x23/0x390
    [ 2.612967] PGD 0 P4D 0
    [ 2.613080] Oops: 0000 [#1] SMP
    [ 2.613080] Modules linked in: autofs4
    [ 2.613080] Supported: Yes
    [ 2.613080] CPU: 0 PID: 1 Comm: systemd Not tainted 4.12.14-2.3-default #1
    [ 2.613080] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.0.0-prebuilt.qemu-project.org 04/01/2014
    [ 2.613080] task: ffff88003e2d0040 task.stack: ffffc90000190000
    [ 2.613080] RIP: 0010:ima_match_policy+0x23/0x390
    [ 2.613080] RSP: 0018:ffffc90000193e88 EFLAGS: 00010296
    [ 2.613080] RAX: 0000000000000000 RBX: 000000000000000c RCX: 0000000000000004
    [ 2.613080] RDX: 0000000000000010 RSI: 0000000000000001 RDI: ffff880037071728
    [ 2.613080] RBP: 0000000000008000 R08: 0000000000000000 R09: 0000000000000000
    [ 2.613080] R10: 0000000000000008 R11: 61c8864680b583eb R12: 00005580ff10086f
    [ 2.613080] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000008000
    [ 2.613080] FS: 00007f5c1da08940(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000
    [ 2.613080] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    [ 2.613080] CR2: 0000000000000000 CR3: 0000000037002000 CR4: 00000000003406f0
    [ 2.613080] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
    [ 2.613080] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
    [ 2.613080] Call Trace:
    [ 2.613080] ? shmem_mknod+0xbf/0xd0
    [ 2.613080] ima_post_path_mknod+0x1c/0x40
    [ 2.613080] SyS_mknod+0x210/0x220
    [ 2.613080] entry_SYSCALL_64_fastpath+0x1a/0xa5
    [ 2.613080] RIP: 0033:0x7f5c1bfde570
    [ 2.613080] RSP: 002b:00007ffde1c90dc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000085
    [ 2.613080] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f5c1bfde570
    [ 2.613080] RDX: 0000000000000000 RSI: 0000000000008000 RDI: 00005580ff10086f
    [ 2.613080] RBP: 00007ffde1c91040 R08: 00005580ff10086f R09: 0000000000000000
    [ 2.613080] R10: 0000000000104000 R11: 0000000000000246 R12: 00005580ffb99660
    [ 2.613080] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000002
    [ 2.613080] Code: 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 41 57 41 56 44 8d 14 09 41 55 41 54 55 53 44 89 d3 09 cb 48 83 ec 38 48 8b 05 c5 03 29 01 8b 20 4c 39 e0 0f 84 d7 01 00 00 4c 89 44 24 08 89 54 24 20
    [ 2.613080] RIP: ima_match_policy+0x23/0x390 RSP: ffffc90000193e88
    [ 2.613080] CR2: 0000000000000000
    [ 2.613080] ---[ end trace 9a9f0a8a73079f6a ]---
    [ 2.673052] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009
    [ 2.673052]
    [ 2.675337] Kernel Offset: disabled
    [ 2.676405] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009

    Signed-off-by: Petr Vorel
    Signed-off-by: Mimi Zohar
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Petr Vorel
     
  • [ Upstream commit fac37c628fd5d68fd7298d9b57ae8601ee1b4723 ]

    TPM_CRB driver provides TPM CRB 2.0 support. If it is built as a
    module, the TPM chip is registered after IMA init. tpm_pcr_read() in
    IMA fails and displays the following message even though eventually
    there is a TPM chip on the system.

    ima: No TPM chip found, activating TPM-bypass! (rc=-19)

    Fix IMA Kconfig to select TPM_CRB so TPM_CRB driver is built in the kernel
    and initializes before IMA.

    Signed-off-by: Jiandi An
    Signed-off-by: Mimi Zohar
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Jiandi An
     
  • [ Upstream commit 120f3b11ef88fc38ce1d0ff9c9a4b37860ad3140 ]

    security/integrity/digsig.c has build errors on some $ARCH due to a
    missing header file, so add it.

    security/integrity/digsig.c:146:2: error: implicit declaration of function 'vfree' [-Werror=implicit-function-declaration]

    Reported-by: Michael Ellerman
    Signed-off-by: Randy Dunlap
    Cc: Mimi Zohar
    Cc: linux-integrity@vger.kernel.org
    Link: http://kisskb.ellerman.id.au/kisskb/head/13396/
    Signed-off-by: James Morris
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Randy Dunlap
     

29 Apr, 2018

1 commit

  • commit 1f5781725dcbb026438e77091c91a94f678c3522 upstream.

    syzbot is reporting NULL pointer dereference at xattr_getsecurity() [1],
    for cap_inode_getsecurity() is returning sizeof(struct vfs_cap_data) when
    memory allocation failed. Return -ENOMEM if memory allocation failed.

    [1] https://syzkaller.appspot.com/bug?id=a55ba438506fe68649a5f50d2d82d56b365e0107

    Signed-off-by: Tetsuo Handa
    Fixes: 8db6c34f1dbc8e06 ("Introduce v3 namespaced file capabilities")
    Reported-by: syzbot
    Cc: stable # 4.14+
    Acked-by: Serge E. Hallyn
    Acked-by: James Morris
    Signed-off-by: Eric W. Biederman
    Signed-off-by: Greg Kroah-Hartman

    Tetsuo Handa
     

19 Apr, 2018

3 commits

  • commit b5beb07ad32ab533027aa988d96a44965ec116f7 upstream.

    Resource auditing is using the peer field which is not available
    when the rlim data struct is used, because it is a different element
    of the same union. Accessing peer during resource auditing could
    cause garbage log entries or even oops the kernel.

    Move the rlim data block into the same struct as the peer field
    so they can be used together.

    CC:
    Fixes: 86b92cb782b3 ("apparmor: move resource checks to using labels")
    Signed-off-by: John Johansen
    Signed-off-by: Greg Kroah-Hartman

    John Johansen
     
  • commit 040d9e2bce0a5b321c402b79ee43a8e8d2fd3b06 upstream.

    The .ns_name should not be virtualized by the current ns view. It
    needs to report the ns base name as that is being used during startup
    as part of determining apparmor policy namespace support.

    BugLink: http://bugs.launchpad.net/bugs/1746463
    Fixes: d9f02d9c237aa ("apparmor: fix display of ns name")
    Cc: Stable
    Reported-by: Serge Hallyn
    Tested-by: Serge Hallyn
    Signed-off-by: John Johansen
    Signed-off-by: Greg Kroah-Hartman

    John Johansen
     
  • commit 98cf5bbff413eadf1b9cb195a7b80cc61c72a50e upstream.

    The existence test is not being properly logged as the signal mapping
    maps it to the last entry in the named signal table. This is done
    to help catch bugs by making the 0 mapped signal value invalid so
    that we can catch the signal value not being filled in.

    When fixing the off-by-one comparision logic the reporting of the
    existence test was broken, because the logic behind the mapped named
    table was hidden. Fix this by adding a define for the name lookup
    and using it.

    Cc: Stable
    Fixes: f7dc4c9a855a1 ("apparmor: fix off-by-one comparison on MAXMAPPED_SIG")
    Signed-off-by: John Johansen
    Signed-off-by: Greg Kroah-Hartman

    John Johansen
     

24 Mar, 2018

1 commit

  • [ Upstream commit 22ec1a2aea73b9dfe340dff7945bd85af4cc6280 ]

    As done for /proc/kcore in

    commit df04abfd181a ("fs/proc/kcore.c: Add bounce buffer for ktext data")

    this adds a bounce buffer when reading memory via /dev/mem. This
    is needed to allow kernel text memory to be read out when built with
    CONFIG_HARDENED_USERCOPY (which refuses to read out kernel text) and
    without CONFIG_STRICT_DEVMEM (which would have refused to read any RAM
    contents at all).

    Since this build configuration isn't common (most systems with
    CONFIG_HARDENED_USERCOPY also have CONFIG_STRICT_DEVMEM), this also tries
    to inform Kconfig about the recommended settings.

    This patch is modified from Brad Spengler/PaX Team's changes to /dev/mem
    code in the last public patch of grsecurity/PaX based on my understanding
    of the code. Changes or omissions from the original code are mine and
    don't reflect the original grsecurity/PaX code.

    Reported-by: Michael Holzheu
    Fixes: f5509cc18daa ("mm: Hardened usercopy")
    Signed-off-by: Kees Cook
    Signed-off-by: Greg Kroah-Hartman
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Kees Cook
     

19 Mar, 2018

1 commit

  • [ Upstream commit b7e27bc1d42e8e0cc58b602b529c25cd0071b336 ]

    Custom policies can require file signatures based on LSM labels. These
    files are normally created and only afterwards labeled, requiring them
    to be signed.

    Instead of requiring file signatures based on LSM labels, entire
    filesystems could require file signatures. In this case, we need the
    ability of writing new files without requiring file signatures.

    The definition of a "new" file was originally defined as any file with
    a length of zero. Subsequent patches redefined a "new" file to be based
    on the FILE_CREATE open flag. By combining the open flag with a file
    size of zero, this patch relaxes the file signature requirement.

    Fixes: 1ac202e978e1 ima: accept previously set IMA_NEW_FILE
    Signed-off-by: Mimi Zohar
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Mimi Zohar
     

25 Feb, 2018

2 commits

  • commit 4b14752ec4e0d87126e636384cf37c8dd9df157c upstream.

    We can't do anything reasonable in security_bounded_transition() if we
    don't have a policy loaded, and in fact we could run into problems
    with some of the code inside expecting a policy. Fix these problems
    like we do many others in security/selinux/ss/services.c by checking
    to see if the policy is loaded (ss_initialized) and returning quickly
    if it isn't.

    Reported-by: syzbot
    Signed-off-by: Paul Moore
    Acked-by: Stephen Smalley
    Reviewed-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Paul Moore
     
  • commit ef28df55ac27e1e5cd122e19fa311d886d47a756 upstream.

    The syzbot/syzkaller automated tests found a problem in
    security_context_to_sid_core() during early boot (before we load the
    SELinux policy) where we could potentially feed context strings without
    NUL terminators into the strcmp() function.

    We already guard against this during normal operation (after the SELinux
    policy has been loaded) by making a copy of the context strings and
    explicitly adding a NUL terminator to the end. The patch extends this
    protection to the early boot case (no loaded policy) by moving the context
    copy earlier in security_context_to_sid_core().

    Reported-by: syzbot
    Signed-off-by: Paul Moore
    Reviewed-By: William Roberts
    Signed-off-by: Greg Kroah-Hartman

    Paul Moore
     

04 Feb, 2018

1 commit

  • commit 36447456e1cca853188505f2a964dbbeacfc7a7a upstream.

    The switch to uuid_t invereted the logic of verfication that &entry->fsuuid
    is zero during parsing of "fsuuid=" rule. Instead of making sure the
    &entry->fsuuid field is not attempted to be overwritten, we bail out for
    perfectly correct rule.

    Fixes: 787d8c530af7 ("ima/policy: switch to use uuid_t")
    Signed-off-by: Mike Rapoport
    Signed-off-by: Mimi Zohar
    Signed-off-by: Greg Kroah-Hartman

    Mike Rapoport
     

17 Jan, 2018

2 commits

  • commit a237f762681e2a394ca67f21df2feb2b76a3609b upstream.

    When the config option for PTI was added a reference to documentation was
    added as well. But the documentation did not exist at that point. The final
    documentation has a different file name.

    Fix it up to point to the proper file.

    Fixes: 385ce0ea ("x86/mm/pti: Add Kconfig")
    Signed-off-by: W. Trevor King
    Signed-off-by: Thomas Gleixner
    Cc: Dave Hansen
    Cc: linux-mm@kvack.org
    Cc: linux-security-module@vger.kernel.org
    Cc: James Morris
    Cc: "Serge E. Hallyn"
    Cc: stable@vger.kernel.org
    Link: https://lkml.kernel.org/r/3009cc8ccbddcd897ec1e0cb6dda524929de0d14.1515799398.git.wking@tremily.us
    Signed-off-by: Greg Kroah-Hartman

    W. Trevor King
     
  • commit 0dda0b3fb255048a221f736c8a2a24c674da8bf3 upstream.

    Given a label with a profile stack of
    A//&B or A//&C ...

    A ptrace rule should be able to specify a generic trace pattern with
    a rule like

    ptrace trace A//&**,

    however this is failing because while the correct label match routine
    is called, it is being done post label decomposition so it is always
    being done against a profile instead of the stacked label.

    To fix this refactor the cross check to pass the full peer label in to
    the label_match.

    Fixes: 290f458a4f16 ("apparmor: allow ptrace checks to be finer grained than just capability")
    Reported-by: Matthew Garrett
    Tested-by: Matthew Garrett
    Signed-off-by: John Johansen
    Signed-off-by: Greg Kroah-Hartman

    John Johansen
     

10 Jan, 2018

1 commit

  • commit 5b9f57cf47b87f07210875d6a24776b4496b818d upstream.

    When the mount code was refactored for Labels it was not correctly
    updated to check whether policy supported mediation of the mount
    class. This causes a regression when the kernel feature set is
    reported as supporting mount and policy is pinned to a feature set
    that does not support mount mediation.

    BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=882697#41
    Fixes: 2ea3ffb7782a ("apparmor: add mount mediation")
    Reported-by: Fabian Grünbichler
    Signed-off-by: John Johansen
    Signed-off-by: Greg Kroah-Hartman

    John Johansen
     

05 Jan, 2018

1 commit

  • commit dc32b5c3e6e2ef29cef76d9ce1b92d394446150e upstream.

    If userspace attempted to set a "security.capability" xattr shorter than
    4 bytes (e.g. 'setfattr -n security.capability -v x file'), then
    cap_convert_nscap() read past the end of the buffer containing the xattr
    value because it accessed the ->magic_etc field without verifying that
    the xattr value is long enough to contain that field.

    Fix it by validating the xattr value size first.

    This bug was found using syzkaller with KASAN. The KASAN report was as
    follows (cleaned up slightly):

    BUG: KASAN: slab-out-of-bounds in cap_convert_nscap+0x514/0x630 security/commoncap.c:498
    Read of size 4 at addr ffff88002d8741c0 by task syz-executor1/2852

    CPU: 0 PID: 2852 Comm: syz-executor1 Not tainted 4.15.0-rc6-00200-gcc0aac99d977 #253
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-20171110_100015-anatol 04/01/2014
    Call Trace:
    __dump_stack lib/dump_stack.c:17 [inline]
    dump_stack+0xe3/0x195 lib/dump_stack.c:53
    print_address_description+0x73/0x260 mm/kasan/report.c:252
    kasan_report_error mm/kasan/report.c:351 [inline]
    kasan_report+0x235/0x350 mm/kasan/report.c:409
    cap_convert_nscap+0x514/0x630 security/commoncap.c:498
    setxattr+0x2bd/0x350 fs/xattr.c:446
    path_setxattr+0x168/0x1b0 fs/xattr.c:472
    SYSC_setxattr fs/xattr.c:487 [inline]
    SyS_setxattr+0x36/0x50 fs/xattr.c:483
    entry_SYSCALL_64_fastpath+0x18/0x85

    Fixes: 8db6c34f1dbc ("Introduce v3 namespaced file capabilities")
    Signed-off-by: Eric Biggers
    Reviewed-by: Serge Hallyn
    Signed-off-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

03 Jan, 2018

1 commit

  • commit 385ce0ea4c078517fa51c261882c4e72fba53005 upstream.

    Finally allow CONFIG_PAGE_TABLE_ISOLATION to be enabled.

    PARAVIRT generally requires that the kernel not manage its own page tables.
    It also means that the hypervisor and kernel must agree wholeheartedly
    about what format the page tables are in and what they contain.
    PAGE_TABLE_ISOLATION, unfortunately, changes the rules and they
    can not be used together.

    I've seen conflicting feedback from maintainers lately about whether they
    want the Kconfig magic to go first or last in a patch series. It's going
    last here because the partially-applied series leads to kernels that can
    not boot in a bunch of cases. I did a run through the entire series with
    CONFIG_PAGE_TABLE_ISOLATION=y to look for build errors, though.

    [ tglx: Removed SMP and !PARAVIRT dependencies as they not longer exist ]

    Signed-off-by: Dave Hansen
    Signed-off-by: Thomas Gleixner
    Cc: Andy Lutomirski
    Cc: Boris Ostrovsky
    Cc: Borislav Petkov
    Cc: Brian Gerst
    Cc: David Laight
    Cc: Denys Vlasenko
    Cc: Eduardo Valentin
    Cc: Greg KH
    Cc: H. Peter Anvin
    Cc: Josh Poimboeuf
    Cc: Juergen Gross
    Cc: Linus Torvalds
    Cc: Peter Zijlstra
    Cc: Will Deacon
    Cc: aliguori@amazon.com
    Cc: daniel.gruss@iaik.tugraz.at
    Cc: hughd@google.com
    Cc: keescook@google.com
    Cc: linux-mm@kvack.org
    Signed-off-by: Ingo Molnar
    Signed-off-by: Greg Kroah-Hartman

    Dave Hansen
     

14 Dec, 2017

3 commits

  • [ Upstream commit 4633307e5ed6128975595df43f796a10c41d11c1 ]

    Fixes: d07881d2edb0 ("apparmor: move new_null_profile to after profile lookup fns()")
    Reported-by: Seth Arnold
    Signed-off-by: John Johansen
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    John Johansen
     
  • commit 18026d866801d0c52e5550210563222bd6c7191d upstream.

    keyctl_restrict_keyring() allows through a NULL restriction when the
    "type" is non-NULL, which causes a NULL pointer dereference in
    asymmetric_lookup_restriction() when it calls strcmp() on the
    restriction string.

    But no key types actually use a "NULL restriction" to mean anything, so
    update keyctl_restrict_keyring() to reject it with EINVAL.

    Reported-by: syzbot
    Fixes: 97d3aa0f3134 ("KEYS: Add a lookup_restriction function for the asymmetric key type")
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit 4dca6ea1d9432052afb06baf2e3ae78188a4410b upstream.

    When the request_key() syscall is not passed a destination keyring, it
    links the requested key (if constructed) into the "default" request-key
    keyring. This should require Write permission to the keyring. However,
    there is actually no permission check.

    This can be abused to add keys to any keyring to which only Search
    permission is granted. This is because Search permission allows joining
    the keyring. keyctl_set_reqkey_keyring(KEY_REQKEY_DEFL_SESSION_KEYRING)
    then will set the default request-key keyring to the session keyring.
    Then, request_key() can be used to add keys to the keyring.

    Both negatively and positively instantiated keys can be added using this
    method. Adding negative keys is trivial. Adding a positive key is a
    bit trickier. It requires that either /sbin/request-key positively
    instantiates the key, or that another thread adds the key to the process
    keyring at just the right time, such that request_key() misses it
    initially but then finds it in construct_alloc_key().

    Fix this bug by checking for Write permission to the keyring in
    construct_get_dest_keyring() when the default keyring is being used.

    We don't do the permission check for non-default keyrings because that
    was already done by the earlier call to lookup_user_key(). Also,
    request_key_and_link() is currently passed a 'struct key *' rather than
    a key_ref_t, so the "possessed" bit is unavailable.

    We also don't do the permission check for the "requestor keyring", to
    continue to support the use case described by commit 8bbf4976b59f
    ("KEYS: Alter use of key instantiation link-to-keyring argument") where
    /sbin/request-key recursively calls request_key() to add keys to the
    original requestor's destination keyring. (I don't know of any users
    who actually do that, though...)

    Fixes: 3e30148c3d52 ("[PATCH] Keys: Make request-key create an authorisation key")
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

10 Dec, 2017

1 commit

  • [ Upstream commit ebe7c0a7be92bbd34c6ff5b55810546a0ee05bee ]

    The hash_setup function always sets the hash_setup_done flag, even
    when the hash algorithm is invalid. This prevents the default hash
    algorithm defined as CONFIG_IMA_DEFAULT_HASH from being used.

    This patch sets hash_setup_done flag only for valid hash algorithms.

    Fixes: e7a2ad7eb6f4 "ima: enable support for larger default filedata hash algorithms"
    Signed-off-by: Boshi Wang
    Signed-off-by: Mimi Zohar
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Boshi Wang
     

05 Dec, 2017

1 commit

  • commit b12cbb21586277f72533769832c24cc6c1d60ab3 upstream.

    The apparmor_audit_data struct ordering got messed up during a merge
    conflict, resulting in the signal integer and peer pointer being in
    a union instead of a struct.

    For most of the 4.13 and 4.14 life cycle, this was hidden by
    commit 651e28c5537a ("apparmor: add base infastructure for socket
    mediation") which fixed the apparmor_audit_data struct when its data
    was added. When that commit was reverted in -rc7 the signal audit bug
    was exposed, and unfortunately it never showed up in any of the
    testing until after 4.14 was released. Shaun Khan, Zephaniah
    E. Loss-Cutler-Hull filed nearly simultaneous bug reports (with
    different oopes, the smaller of which is included below).

    Full credit goes to Tetsuo Handa for jumping on this as well and
    noticing the audit data struct problem and reporting it.

    [ 76.178568] BUG: unable to handle kernel paging request at
    ffffffff0eee3bc0
    [ 76.178579] IP: audit_signal_cb+0x6c/0xe0
    [ 76.178581] PGD 1a640a067 P4D 1a640a067 PUD 0
    [ 76.178586] Oops: 0000 [#1] PREEMPT SMP
    [ 76.178589] Modules linked in: fuse rfcomm bnep usblp uvcvideo btusb
    btrtl btbcm btintel bluetooth ecdh_generic ip6table_filter ip6_tables
    xt_tcpudp nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack
    iptable_filter ip_tables x_tables intel_rapl joydev wmi_bmof serio_raw
    iwldvm iwlwifi shpchp kvm_intel kvm irqbypass autofs4 algif_skcipher
    nls_iso8859_1 nls_cp437 crc32_pclmul ghash_clmulni_intel
    [ 76.178620] CPU: 0 PID: 10675 Comm: pidgin Not tainted
    4.14.0-f1-dirty #135
    [ 76.178623] Hardware name: Hewlett-Packard HP EliteBook Folio
    9470m/18DF, BIOS 68IBD Ver. F.62 10/22/2015
    [ 76.178625] task: ffff9c7a94c31dc0 task.stack: ffffa09b02a4c000
    [ 76.178628] RIP: 0010:audit_signal_cb+0x6c/0xe0
    [ 76.178631] RSP: 0018:ffffa09b02a4fc08 EFLAGS: 00010292
    [ 76.178634] RAX: ffffa09b02a4fd60 RBX: ffff9c7aee0741f8 RCX:
    0000000000000000
    [ 76.178636] RDX: ffffffffee012290 RSI: 0000000000000006 RDI:
    ffff9c7a9493d800
    [ 76.178638] RBP: ffffa09b02a4fd40 R08: 000000000000004d R09:
    ffffa09b02a4fc46
    [ 76.178641] R10: ffffa09b02a4fcb8 R11: ffff9c7ab44f5072 R12:
    ffffa09b02a4fd40
    [ 76.178643] R13: ffffffff9e447be0 R14: ffff9c7a94c31dc0 R15:
    0000000000000001
    [ 76.178646] FS: 00007f8b11ba2a80(0000) GS:ffff9c7afea00000(0000)
    knlGS:0000000000000000
    [ 76.178648] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    [ 76.178650] CR2: ffffffff0eee3bc0 CR3: 00000003d5209002 CR4:
    00000000001606f0
    [ 76.178652] Call Trace:
    [ 76.178660] common_lsm_audit+0x1da/0x780
    [ 76.178665] ? d_absolute_path+0x60/0x90
    [ 76.178669] ? aa_check_perms+0xcd/0xe0
    [ 76.178672] aa_check_perms+0xcd/0xe0
    [ 76.178675] profile_signal_perm.part.0+0x90/0xa0
    [ 76.178679] aa_may_signal+0x16e/0x1b0
    [ 76.178686] apparmor_task_kill+0x51/0x120
    [ 76.178690] security_task_kill+0x44/0x60
    [ 76.178695] group_send_sig_info+0x25/0x60
    [ 76.178699] kill_pid_info+0x36/0x60
    [ 76.178703] SYSC_kill+0xdb/0x180
    [ 76.178707] ? preempt_count_sub+0x92/0xd0
    [ 76.178712] ? _raw_write_unlock_irq+0x13/0x30
    [ 76.178716] ? task_work_run+0x6a/0x90
    [ 76.178720] ? exit_to_usermode_loop+0x80/0xa0
    [ 76.178723] entry_SYSCALL_64_fastpath+0x13/0x94
    [ 76.178727] RIP: 0033:0x7f8b0e58b767
    [ 76.178729] RSP: 002b:00007fff19efd4d8 EFLAGS: 00000206 ORIG_RAX:
    000000000000003e
    [ 76.178732] RAX: ffffffffffffffda RBX: 0000557f3e3c2050 RCX:
    00007f8b0e58b767
    [ 76.178735] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
    000000000000263b
    [ 76.178737] RBP: 0000000000000000 R08: 0000557f3e3c2270 R09:
    0000000000000001
    [ 76.178739] R10: 000000000000022d R11: 0000000000000206 R12:
    0000000000000000
    [ 76.178741] R13: 0000000000000001 R14: 0000557f3e3c13c0 R15:
    0000000000000000
    [ 76.178745] Code: 48 8b 55 18 48 89 df 41 b8 20 00 08 01 5b 5d 48 8b
    42 10 48 8b 52 30 48 63 48 4c 48 8b 44 c8 48 31 c9 48 8b 70 38 e9 f4 fd
    00 00 8b 14 d5 40 27 e5 9e 48 c7 c6 7d 07 19 9f 48 89 df e8 fd 35
    [ 76.178794] RIP: audit_signal_cb+0x6c/0xe0 RSP: ffffa09b02a4fc08
    [ 76.178796] CR2: ffffffff0eee3bc0
    [ 76.178799] ---[ end trace 514af9529297f1a3 ]---

    Fixes: cd1dbf76b23d ("apparmor: add the ability to mediate signals")
    Reported-by: Zephaniah E. Loss-Cutler-Hull
    Reported-by: Shuah Khan
    Suggested-by: Tetsuo Handa
    Tested-by: Ivan Kozik
    Tested-by: Zephaniah E. Loss-Cutler-Hull
    Tested-by: Christian Boltz
    Tested-by: Shuah Khan
    Signed-off-by: John Johansen
    Signed-off-by: Greg Kroah-Hartman

    John Johansen
     

24 Nov, 2017

1 commit

  • commit 020aae3ee58c1af0e7ffc4e2cc9fe4dc630338cb upstream.

    Commit b65a9cfc2c38 ("Untangling ima mess, part 2: deal with counters")
    moved the call of ima_file_check() from may_open() to do_filp_open() at a
    point where the file descriptor is already opened.

    This breaks the assumption made by IMA that file descriptors being closed
    belong to files whose access was granted by ima_file_check(). The
    consequence is that security.ima and security.evm are updated with good
    values, regardless of the current appraisal status.

    For example, if a file does not have security.ima, IMA will create it after
    opening the file for writing, even if access is denied. Access to the file
    will be allowed afterwards.

    Avoid this issue by checking the appraisal status before updating
    security.ima.

    Signed-off-by: Roberto Sassu
    Signed-off-by: Mimi Zohar
    Signed-off-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Roberto Sassu
     

09 Nov, 2017

1 commit

  • This came in yesterday, and I have verified our regression tests
    were missing this and it can cause an oops. Please apply.

    There is a an off-by-one comparision on sig against MAXMAPPED_SIG
    that can lead to a read outside the sig_map array if sig
    is MAXMAPPED_SIG. Fix this.

    Verified that the check is an out of bounds case that can cause an oops.

    Revised: add comparison fix to second case
    Fixes: cd1dbf76b23d ("apparmor: add the ability to mediate signals")
    Signed-off-by: Colin Ian King
    Signed-off-by: John Johansen
    Signed-off-by: Linus Torvalds

    John Johansen
     

03 Nov, 2017

1 commit

  • …el/git/gregkh/driver-core

    Pull initial SPDX identifiers from Greg KH:
    "License cleanup: add SPDX license identifiers to some files

    Many source files in the tree are missing licensing information, which
    makes it harder for compliance tools to determine the correct license.

    By default all files without license information are under the default
    license of the kernel, which is GPL version 2.

    Update the files which contain no license information with the
    'GPL-2.0' SPDX license identifier. The SPDX identifier is a legally
    binding shorthand, which can be used instead of the full boiler plate
    text.

    This patch is based on work done by Thomas Gleixner and Kate Stewart
    and Philippe Ombredanne.

    How this work was done:

    Patches were generated and checked against linux-4.14-rc6 for a subset
    of the use cases:

    - file had no licensing information it it.

    - file was a */uapi/* one with no licensing information in it,

    - file was a */uapi/* one with existing licensing information,

    Further patches will be generated in subsequent months to fix up cases
    where non-standard license headers were used, and references to
    license had to be inferred by heuristics based on keywords.

    The analysis to determine which SPDX License Identifier to be applied
    to a file was done in a spreadsheet of side by side results from of
    the output of two independent scanners (ScanCode & Windriver)
    producing SPDX tag:value files created by Philippe Ombredanne.
    Philippe prepared the base worksheet, and did an initial spot review
    of a few 1000 files.

    The 4.13 kernel was the starting point of the analysis with 60,537
    files assessed. Kate Stewart did a file by file comparison of the
    scanner results in the spreadsheet to determine which SPDX license
    identifier(s) to be applied to the file. She confirmed any
    determination that was not immediately clear with lawyers working with
    the Linux Foundation.

    Criteria used to select files for SPDX license identifier tagging was:

    - Files considered eligible had to be source code files.

    - Make and config files were included as candidates if they contained
    >5 lines of source

    - File already had some variant of a license header in it (even if <5
    lines).

    All documentation files were explicitly excluded.

    The following heuristics were used to determine which SPDX license
    identifiers to apply.

    - when both scanners couldn't find any license traces, file was
    considered to have no license information in it, and the top level
    COPYING file license applied.

    For non */uapi/* files that summary was:

    SPDX license identifier # files
    ---------------------------------------------------|-------
    GPL-2.0 11139

    and resulted in the first patch in this series.

    If that file was a */uapi/* path one, it was "GPL-2.0 WITH
    Linux-syscall-note" otherwise it was "GPL-2.0". Results of that
    was:

    SPDX license identifier # files
    ---------------------------------------------------|-------
    GPL-2.0 WITH Linux-syscall-note 930

    and resulted in the second patch in this series.

    - if a file had some form of licensing information in it, and was one
    of the */uapi/* ones, it was denoted with the Linux-syscall-note if
    any GPL family license was found in the file or had no licensing in
    it (per prior point). Results summary:

    SPDX license identifier # files
    ---------------------------------------------------|------
    GPL-2.0 WITH Linux-syscall-note 270
    GPL-2.0+ WITH Linux-syscall-note 169
    ((GPL-2.0 WITH Linux-syscall-note) OR BSD-2-Clause) 21
    ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) 17
    LGPL-2.1+ WITH Linux-syscall-note 15
    GPL-1.0+ WITH Linux-syscall-note 14
    ((GPL-2.0+ WITH Linux-syscall-note) OR BSD-3-Clause) 5
    LGPL-2.0+ WITH Linux-syscall-note 4
    LGPL-2.1 WITH Linux-syscall-note 3
    ((GPL-2.0 WITH Linux-syscall-note) OR MIT) 3
    ((GPL-2.0 WITH Linux-syscall-note) AND MIT) 1

    and that resulted in the third patch in this series.

    - when the two scanners agreed on the detected license(s), that
    became the concluded license(s).

    - when there was disagreement between the two scanners (one detected
    a license but the other didn't, or they both detected different
    licenses) a manual inspection of the file occurred.

    - In most cases a manual inspection of the information in the file
    resulted in a clear resolution of the license that should apply
    (and which scanner probably needed to revisit its heuristics).

    - When it was not immediately clear, the license identifier was
    confirmed with lawyers working with the Linux Foundation.

    - If there was any question as to the appropriate license identifier,
    the file was flagged for further research and to be revisited later
    in time.

    In total, over 70 hours of logged manual review was done on the
    spreadsheet to determine the SPDX license identifiers to apply to the
    source files by Kate, Philippe, Thomas and, in some cases,
    confirmation by lawyers working with the Linux Foundation.

    Kate also obtained a third independent scan of the 4.13 code base from
    FOSSology, and compared selected files where the other two scanners
    disagreed against that SPDX file, to see if there was new insights.
    The Windriver scanner is based on an older version of FOSSology in
    part, so they are related.

    Thomas did random spot checks in about 500 files from the spreadsheets
    for the uapi headers and agreed with SPDX license identifier in the
    files he inspected. For the non-uapi files Thomas did random spot
    checks in about 15000 files.

    In initial set of patches against 4.14-rc6, 3 files were found to have
    copy/paste license identifier errors, and have been fixed to reflect
    the correct identifier.

    Additionally Philippe spent 10 hours this week doing a detailed manual
    inspection and review of the 12,461 patched files from the initial
    patch version early this week with:

    - a full scancode scan run, collecting the matched texts, detected
    license ids and scores

    - reviewing anything where there was a license detected (about 500+
    files) to ensure that the applied SPDX license was correct

    - reviewing anything where there was no detection but the patch
    license was not GPL-2.0 WITH Linux-syscall-note to ensure that the
    applied SPDX license was correct

    This produced a worksheet with 20 files needing minor correction. This
    worksheet was then exported into 3 different .csv files for the
    different types of files to be modified.

    These .csv files were then reviewed by Greg. Thomas wrote a script to
    parse the csv files and add the proper SPDX tag to the file, in the
    format that the file expected. This script was further refined by Greg
    based on the output to detect more types of files automatically and to
    distinguish between header and source .c files (which need different
    comment types.) Finally Greg ran the script using the .csv files to
    generate the patches.

    Reviewed-by: Kate Stewart <kstewart@linuxfoundation.org>
    Reviewed-by: Philippe Ombredanne <pombredanne@nexb.com>
    Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>"

    * tag 'spdx_identifiers-4.14-rc8' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core:
    License cleanup: add SPDX license identifier to uapi header files with a license
    License cleanup: add SPDX license identifier to uapi header files with no license
    License cleanup: add SPDX GPL-2.0 license identifier to files with no license

    Linus Torvalds
     

02 Nov, 2017

3 commits

  • Many source files in the tree are missing licensing information, which
    makes it harder for compliance tools to determine the correct license.

    By default all files without license information are under the default
    license of the kernel, which is GPL version 2.

    Update the files which contain no license information with the 'GPL-2.0'
    SPDX license identifier. The SPDX identifier is a legally binding
    shorthand, which can be used instead of the full boiler plate text.

    This patch is based on work done by Thomas Gleixner and Kate Stewart and
    Philippe Ombredanne.

    How this work was done:

    Patches were generated and checked against linux-4.14-rc6 for a subset of
    the use cases:
    - file had no licensing information it it.
    - file was a */uapi/* one with no licensing information in it,
    - file was a */uapi/* one with existing licensing information,

    Further patches will be generated in subsequent months to fix up cases
    where non-standard license headers were used, and references to license
    had to be inferred by heuristics based on keywords.

    The analysis to determine which SPDX License Identifier to be applied to
    a file was done in a spreadsheet of side by side results from of the
    output of two independent scanners (ScanCode & Windriver) producing SPDX
    tag:value files created by Philippe Ombredanne. Philippe prepared the
    base worksheet, and did an initial spot review of a few 1000 files.

    The 4.13 kernel was the starting point of the analysis with 60,537 files
    assessed. Kate Stewart did a file by file comparison of the scanner
    results in the spreadsheet to determine which SPDX license identifier(s)
    to be applied to the file. She confirmed any determination that was not
    immediately clear with lawyers working with the Linux Foundation.

    Criteria used to select files for SPDX license identifier tagging was:
    - Files considered eligible had to be source code files.
    - Make and config files were included as candidates if they contained >5
    lines of source
    - File already had some variant of a license header in it (even if
    Reviewed-by: Philippe Ombredanne
    Reviewed-by: Thomas Gleixner
    Signed-off-by: Greg Kroah-Hartman

    Greg Kroah-Hartman
     
  • When calling keyctl_read() on a key of type "trusted", if the
    user-supplied buffer was too small, the kernel ignored the buffer length
    and just wrote past the end of the buffer, potentially corrupting
    userspace memory. Fix it by instead returning the size required, as per
    the documentation for keyctl_read().

    We also don't even fill the buffer at all in this case, as this is
    slightly easier to implement than doing a short read, and either
    behavior appears to be permitted. It also makes it match the behavior
    of the "encrypted" key type.

    Fixes: d00a1c72f7f4 ("keys: add new trusted key-type")
    Reported-by: Ben Hutchings
    Cc: # v2.6.38+
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Reviewed-by: Mimi Zohar
    Reviewed-by: James Morris
    Signed-off-by: James Morris

    Eric Biggers
     
  • Commit e645016abc80 ("KEYS: fix writing past end of user-supplied buffer
    in keyring_read()") made keyring_read() stop corrupting userspace memory
    when the user-supplied buffer is too small. However it also made the
    return value in that case be the short buffer size rather than the size
    required, yet keyctl_read() is actually documented to return the size
    required. Therefore, switch it over to the documented behavior.

    Note that for now we continue to have it fill the short buffer, since it
    did that before (pre-v3.13) and dump_key_tree_aux() in keyutils arguably
    relies on it.

    Fixes: e645016abc80 ("KEYS: fix writing past end of user-supplied buffer in keyring_read()")
    Reported-by: Ben Hutchings
    Cc: # v3.13+
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Reviewed-by: James Morris
    Signed-off-by: James Morris

    Eric Biggers
     

27 Oct, 2017

1 commit

  • This reverts commit 651e28c5537abb39076d3949fb7618536f1d242e.

    This caused a regression:
    "The specific problem is that dnsmasq refuses to start on openSUSE Leap
    42.2. The specific cause is that and attempt to open a PF_LOCAL socket
    gets EACCES. This means that networking doesn't function on a system
    with a 4.14-rc2 system."

    Sadly, the developers involved seemed to be in denial for several weeks
    about this, delaying the revert. This has not been a good release for
    the security subsystem, and this area needs to change development
    practices.

    Reported-and-bisected-by: James Bottomley
    Tracked-by: Thorsten Leemhuis
    Cc: John Johansen
    Cc: Vlastimil Babka
    Cc: Seth Arnold
    Signed-off-by: Linus Torvalds

    Linus Torvalds
     

19 Oct, 2017

2 commits


18 Oct, 2017

6 commits

  • In proc_keys_show(), the key semaphore is not held, so the key ->flags
    and ->expiry can be changed concurrently. We therefore should read them
    atomically just once.

    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells

    Eric Biggers
     
  • Similar to the case for key_validate(), we should load the key ->expiry
    once atomically in keyring_search_iterator(), since it can be changed
    concurrently with the flags whenever the key semaphore isn't held.

    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells

    Eric Biggers
     
  • In key_validate(), load the flags and expiry time once atomically, since
    these can change concurrently if key_validate() is called without the
    key semaphore held. And we don't want to get inconsistent results if a
    variable is referenced multiple times. For example, key->expiry was
    referenced in both 'if (key->expiry)' and in 'if (now.tv_sec >=
    key->expiry)', making it theoretically possible to see a spurious
    EKEYEXPIRED while the expiration time was being removed, i.e. set to 0.

    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells

    Eric Biggers
     
  • Currently, when passed a key that already exists, add_key() will call the
    key's ->update() method if such exists. But this is heavily broken in the
    case where the key is uninstantiated because it doesn't call
    __key_instantiate_and_link(). Consequently, it doesn't do most of the
    things that are supposed to happen when the key is instantiated, such as
    setting the instantiation state, clearing KEY_FLAG_USER_CONSTRUCT and
    awakening tasks waiting on it, and incrementing key->user->nikeys.

    It also never takes key_construction_mutex, which means that
    ->instantiate() can run concurrently with ->update() on the same key. In
    the case of the "user" and "logon" key types this causes a memory leak, at
    best. Maybe even worse, the ->update() methods of the "encrypted" and
    "trusted" key types actually just dereference a NULL pointer when passed an
    uninstantiated key.

    Change key_create_or_update() to wait interruptibly for the key to finish
    construction before continuing.

    This patch only affects *uninstantiated* keys. For now we still allow a
    negatively instantiated key to be updated (thereby positively
    instantiating it), although that's broken too (the next patch fixes it)
    and I'm not sure that anyone actually uses that functionality either.

    Here is a simple reproducer for the bug using the "encrypted" key type
    (requires CONFIG_ENCRYPTED_KEYS=y), though as noted above the bug
    pertained to more than just the "encrypted" key type:

    #include
    #include
    #include

    int main(void)
    {
    int ringid = keyctl_join_session_keyring(NULL);

    if (fork()) {
    for (;;) {
    const char payload[] = "update user:foo 32";

    usleep(rand() % 10000);
    add_key("encrypted", "desc", payload, sizeof(payload), ringid);
    keyctl_clear(ringid);
    }
    } else {
    for (;;)
    request_key("encrypted", "desc", "callout_info", ringid);
    }
    }

    It causes:

    BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
    IP: encrypted_update+0xb0/0x170
    PGD 7a178067 P4D 7a178067 PUD 77269067 PMD 0
    PREEMPT SMP
    CPU: 0 PID: 340 Comm: reproduce Tainted: G D 4.14.0-rc1-00025-g428490e38b2e #796
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
    task: ffff8a467a39a340 task.stack: ffffb15c40770000
    RIP: 0010:encrypted_update+0xb0/0x170
    RSP: 0018:ffffb15c40773de8 EFLAGS: 00010246
    RAX: 0000000000000000 RBX: ffff8a467a275b00 RCX: 0000000000000000
    RDX: 0000000000000005 RSI: ffff8a467a275b14 RDI: ffffffffb742f303
    RBP: ffffb15c40773e20 R08: 0000000000000000 R09: ffff8a467a275b17
    R10: 0000000000000020 R11: 0000000000000000 R12: 0000000000000000
    R13: 0000000000000000 R14: ffff8a4677057180 R15: ffff8a467a275b0f
    FS: 00007f5d7fb08700(0000) GS:ffff8a467f200000(0000) knlGS:0000000000000000
    CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 0000000000000018 CR3: 0000000077262005 CR4: 00000000001606f0
    Call Trace:
    key_create_or_update+0x2bc/0x460
    SyS_add_key+0x10c/0x1d0
    entry_SYSCALL_64_fastpath+0x1f/0xbe
    RIP: 0033:0x7f5d7f211259
    RSP: 002b:00007ffed03904c8 EFLAGS: 00000246 ORIG_RAX: 00000000000000f8
    RAX: ffffffffffffffda RBX: 000000003b2a7955 RCX: 00007f5d7f211259
    RDX: 00000000004009e4 RSI: 00000000004009ff RDI: 0000000000400a04
    RBP: 0000000068db8bad R08: 000000003b2a7955 R09: 0000000000000004
    R10: 000000000000001a R11: 0000000000000246 R12: 0000000000400868
    R13: 00007ffed03905d0 R14: 0000000000000000 R15: 0000000000000000
    Code: 77 28 e8 64 34 1f 00 45 31 c0 31 c9 48 8d 55 c8 48 89 df 48 8d 75 d0 e8 ff f9 ff ff 85 c0 41 89 c4 0f 88 84 00 00 00 4c 8b 7d c8 8b 75 18 4c 89 ff e8 24 f8 ff ff 85 c0 41 89 c4 78 6d 49 8b
    RIP: encrypted_update+0xb0/0x170 RSP: ffffb15c40773de8
    CR2: 0000000000000018

    Cc: # v2.6.12+
    Reported-by: Eric Biggers
    Signed-off-by: David Howells
    cc: Eric Biggers

    David Howells
     
  • Consolidate KEY_FLAG_INSTANTIATED, KEY_FLAG_NEGATIVE and the rejection
    error into one field such that:

    (1) The instantiation state can be modified/read atomically.

    (2) The error can be accessed atomically with the state.

    (3) The error isn't stored unioned with the payload pointers.

    This deals with the problem that the state is spread over three different
    objects (two bits and a separate variable) and reading or updating them
    atomically isn't practical, given that not only can uninstantiated keys
    change into instantiated or rejected keys, but rejected keys can also turn
    into instantiated keys - and someone accessing the key might not be using
    any locking.

    The main side effect of this problem is that what was held in the payload
    may change, depending on the state. For instance, you might observe the
    key to be in the rejected state. You then read the cached error, but if
    the key semaphore wasn't locked, the key might've become instantiated
    between the two reads - and you might now have something in hand that isn't
    actually an error code.

    The state is now KEY_IS_UNINSTANTIATED, KEY_IS_POSITIVE or a negative error
    code if the key is negatively instantiated. The key_is_instantiated()
    function is replaced with key_is_positive() to avoid confusion as negative
    keys are also 'instantiated'.

    Additionally, barriering is included:

    (1) Order payload-set before state-set during instantiation.

    (2) Order state-read before payload-read when using the key.

    Further separate barriering is necessary if RCU is being used to access the
    payload content after reading the payload pointers.

    Fixes: 146aa8b1453b ("KEYS: Merge the type-specific data with the payload data")
    Cc: stable@vger.kernel.org # v4.4+
    Reported-by: Eric Biggers
    Signed-off-by: David Howells
    Reviewed-by: Eric Biggers

    David Howells
     
  • The recent rework introduced a possible randconfig build failure
    when CONFIG_CRYPTO configured to only allow modules:

    security/keys/big_key.o: In function `big_key_crypt':
    big_key.c:(.text+0x29f): undefined reference to `crypto_aead_setkey'
    security/keys/big_key.o: In function `big_key_init':
    big_key.c:(.init.text+0x1a): undefined reference to `crypto_alloc_aead'
    big_key.c:(.init.text+0x45): undefined reference to `crypto_aead_setauthsize'
    big_key.c:(.init.text+0x77): undefined reference to `crypto_destroy_tfm'
    crypto/gcm.o: In function `gcm_hash_crypt_remain_continue':
    gcm.c:(.text+0x167): undefined reference to `crypto_ahash_finup'
    crypto/gcm.o: In function `crypto_gcm_exit_tfm':
    gcm.c:(.text+0x847): undefined reference to `crypto_destroy_tfm'

    When we 'select CRYPTO' like the other users, we always get a
    configuration that builds.

    Fixes: 428490e38b2e ("security/keys: rewrite all of big_key crypto")
    Signed-off-by: Arnd Bergmann
    Signed-off-by: David Howells

    Arnd Bergmann
     

12 Oct, 2017

1 commit

  • A key of type "encrypted" references a "master key" which is used to
    encrypt and decrypt the encrypted key's payload. However, when we
    accessed the master key's payload, we failed to handle the case where
    the master key has been revoked, which sets the payload pointer to NULL.
    Note that request_key() *does* skip revoked keys, but there is still a
    window where the key can be revoked before we acquire its semaphore.

    Fix it by checking for a NULL payload, treating it like a key which was
    already revoked at the time it was requested.

    This was an issue for master keys of type "user" only. Master keys can
    also be of type "trusted", but those cannot be revoked.

    Fixes: 7e70cb497850 ("keys: add new key-type encrypted")
    Reviewed-by: James Morris
    Cc: [v2.6.38+]
    Cc: Mimi Zohar
    Cc: David Safford
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells

    Eric Biggers
     

04 Oct, 2017

1 commit

  • security_inode_getsecurity() provides the text string value
    of a security attribute. It does not provide a "secctx".
    The code in xattr_getsecurity() that calls security_inode_getsecurity()
    and then calls security_release_secctx() happened to work because
    SElinux and Smack treat the attribute and the secctx the same way.
    It fails for cap_inode_getsecurity(), because that module has no
    secctx that ever needs releasing. It turns out that Smack is the
    one that's doing things wrong by not allocating memory when instructed
    to do so by the "alloc" parameter.

    The fix is simple enough. Change the security_release_secctx() to
    kfree() because it isn't a secctx being returned by
    security_inode_getsecurity(). Change Smack to allocate the string when
    told to do so.

    Note: this also fixes memory leaks for LSMs which implement
    inode_getsecurity but not release_secctx, such as capabilities.

    Signed-off-by: Casey Schaufler
    Reported-by: Konstantin Khlebnikov
    Cc: stable@vger.kernel.org
    Signed-off-by: James Morris

    Casey Schaufler