1. 05 Apr, 2019 2 commits
    • Ondrej Mosnáček's avatar
      selinux: do not override context on context mounts · f836093a
      Ondrej Mosnáček authored
      [ Upstream commit 53e0c2aa ]
      
      Ignore all selinux_inode_notifysecctx() calls on mounts with SBLABEL_MNT
      flag unset. This is achived by returning -EOPNOTSUPP for this case in
      selinux_inode_setsecurtity() (because that function should not be called
      in such case anyway) and translating this error to 0 in
      selinux_inode_notifysecctx().
      
      This fixes behavior of kernfs-based filesystems when mounted with the
      'context=' option. Before this patch, if a node's context had been
      explicitly set to a non-default value and later the filesystem has been
      remounted with the 'context=' option, then this node would show up as
      having the manually-set context and not the mount-specified one.
      
      Steps to reproduce:
          # mount -t cgroup2 cgroup2 /sys/fs/cgroup/unified
          # chcon unconfined_u:object_r:user_home_t:s0 /sys/fs/cgroup/unified/cgroup.stat
          # ls -lZ /sys/fs/cgroup/unified
          total 0
          -r--r--r--. 1 root root system_u:object_r:cgroup_t:s0        0 Dec 13 10:41 cgroup.controllers
          -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0        0 Dec 13 10:41 cgroup.max.depth
          -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0        0 Dec 13 10:41 cgroup.max.descendants
          -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0        0 Dec 13 10:41 cgroup.procs
          -r--r--r--. 1 root root unconfined_u:object_r:user_home_t:s0 0 Dec 13 10:41 cgroup.stat
          -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0        0 Dec 13 10:41 cgroup.subtree_control
          -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0        0 Dec 13 10:41 cgroup.threads
          # umount /sys/fs/cgroup/unified
          # mount -o context=system_u:object_r:tmpfs_t:s0 -t cgroup2 cgroup2 /sys/fs/cgroup/unified
      
      Result before:
          # ls -lZ /sys/fs/cgroup/unified
          total 0
          -r--r--r--. 1 root root system_u:object_r:tmpfs_t:s0         0 Dec 13 10:41 cgroup.controllers
          -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0         0 Dec 13 10:41 cgroup.max.depth
          -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0         0 Dec 13 10:41 cgroup.max.descendants
          -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0         0 Dec 13 10:41 cgroup.procs
          -r--r--r--. 1 root root unconfined_u:object_r:user_home_t:s0 0 Dec 13 10:41 cgroup.stat
          -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0         0 Dec 13 10:41 cgroup.subtree_control
          -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0         0 Dec 13 10:41 cgroup.threads
      
      Result after:
          # ls -lZ /sys/fs/cgroup/unified
          total 0
          -r--r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.controllers
          -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.max.depth
          -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.max.descendants
          -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.procs
          -r--r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.stat
          -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.subtree_control
          -rw-r--r--. 1 root root system_u:object_r:tmpfs_t:s0 0 Dec 13 10:41 cgroup.threads
      Signed-off-by: Ondrej Mosnáček's avatarOndrej Mosnacek <omosnace@redhat.com>
      Reviewed-by: Stephen Smalley's avatarStephen Smalley <sds@tycho.nsa.gov>
      Signed-off-by: 's avatarPaul Moore <paul@paul-moore.com>
      Signed-off-by: 's avatarSasha Levin <sashal@kernel.org>
      f836093a
    • John Johansen's avatar
      apparmor: fix double free when unpack of secmark rules fails · 4c6df358
      John Johansen authored
      [ Upstream commit d8dbb581 ]
      
      if secmark rules fail to unpack a double free happens resulting in
      the following oops
      
      [ 1295.584074] audit: type=1400 audit(1549970525.256:51): apparmor="STATUS" info="failed to unpack profile secmark rules" error=-71 profile="unconfined" name="/root/test" pid=29882 comm="apparmor_parser" name="/root/test" offset=120
      [ 1374.042334] ------------[ cut here ]------------
      [ 1374.042336] kernel BUG at mm/slub.c:294!
      [ 1374.042404] invalid opcode: 0000 [#1] SMP PTI
      [ 1374.042436] CPU: 0 PID: 29921 Comm: apparmor_parser Not tainted 4.20.7-042007-generic #201902061234
      [ 1374.042461] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
      [ 1374.042489] RIP: 0010:kfree+0x164/0x180
      [ 1374.042502] Code: 74 05 41 0f b6 72 51 4c 89 d7 e8 37 cd f8 ff eb 8b 41 b8 01 00 00 00 48 89 d9 48 89 da 4c 89 d6 e8 11 f6 ff ff e9 72 ff ff ff <0f> 0b 49 8b 42 08 a8 01 75 c2 0f 0b 48 8b 3d a9 f4 19 01 e9 c5 fe
      [ 1374.042552] RSP: 0018:ffffaf7b812d7b90 EFLAGS: 00010246
      [ 1374.042568] RAX: ffff91e437679200 RBX: ffff91e437679200 RCX: ffff91e437679200
      [ 1374.042589] RDX: 00000000000088b6 RSI: ffff91e43da27060 RDI: ffff91e43d401a80
      [ 1374.042609] RBP: ffffaf7b812d7ba8 R08: 0000000000027080 R09: ffffffffa6627a6d
      [ 1374.042629] R10: ffffd3af41dd9e40 R11: ffff91e43a1740dc R12: ffff91e3f52e8000
      [ 1374.042650] R13: ffffffffa6627a6d R14: ffffffffffffffb9 R15: 0000000000000001
      [ 1374.042675] FS:  00007f928df77740(0000) GS:ffff91e43da00000(0000) knlGS:0000000000000000
      [ 1374.042697] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
      [ 1374.042714] CR2: 000055a0c3ab6b50 CR3: 0000000079ed8004 CR4: 0000000000360ef0
      [ 1374.042737] Call Trace:
      [ 1374.042750]  kzfree+0x2d/0x40
      [ 1374.042763]  aa_free_profile+0x12b/0x270
      [ 1374.042776]  unpack_profile+0xc1/0xf10
      [ 1374.042790]  aa_unpack+0x115/0x4e0
      [ 1374.042802]  aa_replace_profiles+0x8e/0xcc0
      [ 1374.042817]  ? kvmalloc_node+0x6d/0x80
      [ 1374.042831]  ? __check_object_size+0x166/0x192
      [ 1374.042845]  policy_update+0xcf/0x1b0
      [ 1374.042858]  profile_load+0x7d/0xa0
      [ 1374.042871]  __vfs_write+0x3a/0x190
      [ 1374.042883]  ? apparmor_file_permission+0x1a/0x20
      [ 1374.042899]  ? security_file_permission+0x31/0xc0
      [ 1374.042918]  ? _cond_resched+0x19/0x30
      [ 1374.042931]  vfs_write+0xab/0x1b0
      [ 1374.042963]  ksys_write+0x55/0xc0
      [ 1374.043004]  __x64_sys_write+0x1a/0x20
      [ 1374.043046]  do_syscall_64+0x5a/0x110
      [ 1374.043087]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
      
      Fixes: 9caafbe2 ("apparmor: Parse secmark policy")
      Reported-by: 's avatarAlex Murray <alex.murray@canonical.com>
      Signed-off-by: 's avatarJohn Johansen <john.johansen@canonical.com>
      Signed-off-by: 's avatarSasha Levin <sashal@kernel.org>
      4c6df358
  2. 23 Mar, 2019 2 commits
  3. 22 Feb, 2019 1 commit
    • Eric Biggers's avatar
      KEYS: always initialize keyring_index_key::desc_len · ede0fa98
      Eric Biggers authored
      syzbot hit the 'BUG_ON(index_key->desc_len == 0);' in __key_link_begin()
      called from construct_alloc_key() during sys_request_key(), because the
      length of the key description was never calculated.
      
      The problem is that we rely on ->desc_len being initialized by
      search_process_keyrings(), specifically by search_nested_keyrings().
      But, if the process isn't subscribed to any keyrings that never happens.
      
      Fix it by always initializing keyring_index_key::desc_len as soon as the
      description is set, like we already do in some places.
      
      The following program reproduces the BUG_ON() when it's run as root and
      no session keyring has been installed.  If it doesn't work, try removing
      pam_keyinit.so from /etc/pam.d/login and rebooting.
      
          #include <stdlib.h>
          #include <unistd.h>
          #include <keyutils.h>
      
          int main(void)
          {
                  int id = add_key("keyring", "syz", NULL, 0, KEY_SPEC_USER_KEYRING);
      
                  keyctl_setperm(id, KEY_OTH_WRITE);
                  setreuid(5000, 5000);
                  request_key("user", "desc", "", id);
          }
      
      Reported-by: syzbot+ec24e95ea483de0a24da@syzkaller.appspotmail.com
      Fixes: b2a4df20 ("KEYS: Expand the capacity of a keyring")
      Signed-off-by: 's avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: 's avatarDavid Howells <dhowells@redhat.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: 's avatarJames Morris <james.morris@microsoft.com>
      ede0fa98
  4. 21 Feb, 2019 1 commit
    • Al Viro's avatar
      missing barriers in some of unix_sock ->addr and ->path accesses · ae3b5641
      Al Viro authored
      Several u->addr and u->path users are not holding any locks in
      common with unix_bind().  unix_state_lock() is useless for those
      purposes.
      
      u->addr is assign-once and *(u->addr) is fully set up by the time
      we set u->addr (all under unix_table_lock).  u->path is also
      set in the same critical area, also before setting u->addr, and
      any unix_sock with ->path filled will have non-NULL ->addr.
      
      So setting ->addr with smp_store_release() is all we need for those
      "lockless" users - just have them fetch ->addr with smp_load_acquire()
      and don't even bother looking at ->path if they see NULL ->addr.
      
      Users of ->addr and ->path fall into several classes now:
          1) ones that do smp_load_acquire(u->addr) and access *(u->addr)
      and u->path only if smp_load_acquire() has returned non-NULL.
          2) places holding unix_table_lock.  These are guaranteed that
      *(u->addr) is seen fully initialized.  If unix_sock is in one of the
      "bound" chains, so's ->path.
          3) unix_sock_destructor() using ->addr is safe.  All places
      that set u->addr are guaranteed to have seen all stores *(u->addr)
      while holding a reference to u and unix_sock_destructor() is called
      when (atomic) refcount hits zero.
          4) unix_release_sock() using ->path is safe.  unix_bind()
      is serialized wrt unix_release() (normally - by struct file
      refcount), and for the instances that had ->path set by unix_bind()
      unix_release_sock() comes from unix_release(), so they are fine.
      Instances that had it set in unix_stream_connect() either end up
      attached to a socket (in unix_accept()), in which case the call
      chain to unix_release_sock() and serialization are the same as in
      the previous case, or they never get accept'ed and unix_release_sock()
      is called when the listener is shut down and its queue gets purged.
      In that case the listener's queue lock provides the barriers needed -
      unix_stream_connect() shoves our unix_sock into listener's queue
      under that lock right after having set ->path and eventual
      unix_release_sock() caller picks them from that queue under the
      same lock right before calling unix_release_sock().
          5) unix_find_other() use of ->path is pointless, but safe -
      it happens with successful lookup by (abstract) name, so ->path.dentry
      is guaranteed to be NULL there.
      earlier-variant-reviewed-by: 's avatar"Paul E. McKenney" <paulmck@linux.ibm.com>
      Signed-off-by: 's avatarAl Viro <viro@zeniv.linux.org.uk>
      Signed-off-by: 's avatarDavid S. Miller <davem@davemloft.net>
      ae3b5641
  5. 15 Feb, 2019 3 commits
    • David Howells's avatar
      keys: Timestamp new keys · 7c1857bd
      David Howells authored
      Set the timestamp on new keys rather than leaving it unset.
      
      Fixes: 31d5a79d ("KEYS: Do LRU discard in full keyrings")
      Signed-off-by: 's avatarDavid Howells <dhowells@redhat.com>
      Signed-off-by: 's avatarJames Morris <james.morris@microsoft.com>
      7c1857bd
    • David Howells's avatar
      keys: Fix dependency loop between construction record and auth key · 822ad64d
      David Howells authored
      In the request_key() upcall mechanism there's a dependency loop by which if
      a key type driver overrides the ->request_key hook and the userspace side
      manages to lose the authorisation key, the auth key and the internal
      construction record (struct key_construction) can keep each other pinned.
      
      Fix this by the following changes:
      
       (1) Killing off the construction record and using the auth key instead.
      
       (2) Including the operation name in the auth key payload and making the
           payload available outside of security/keys/.
      
       (3) The ->request_key hook is given the authkey instead of the cons
           record and operation name.
      
      Changes (2) and (3) allow the auth key to naturally be cleaned up if the
      keyring it is in is destroyed or cleared or the auth key is unlinked.
      
      Fixes: 7ee02a316600 ("keys: Fix dependency loop between construction record and auth key")
      Signed-off-by: 's avatarDavid Howells <dhowells@redhat.com>
      Signed-off-by: 's avatarJames Morris <james.morris@microsoft.com>
      822ad64d
    • Eric Biggers's avatar
      KEYS: allow reaching the keys quotas exactly · a08bf91c
      Eric Biggers authored
      If the sysctl 'kernel.keys.maxkeys' is set to some number n, then
      actually users can only add up to 'n - 1' keys.  Likewise for
      'kernel.keys.maxbytes' and the root_* versions of these sysctls.  But
      these sysctls are apparently supposed to be *maximums*, as per their
      names and all documentation I could find -- the keyrings(7) man page,
      Documentation/security/keys/core.rst, and all the mentions of EDQUOT
      meaning that the key quota was *exceeded* (as opposed to reached).
      
      Thus, fix the code to allow reaching the quotas exactly.
      
      Fixes: 0b77f5bf ("keys: make the keyring quotas controllable through /proc/sys")
      Cc: stable@vger.kernel.org
      Signed-off-by: 's avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: 's avatarDavid Howells <dhowells@redhat.com>
      Signed-off-by: 's avatarJames Morris <james.morris@microsoft.com>
      a08bf91c
  6. 01 Feb, 2019 2 commits
  7. 16 Jan, 2019 2 commits
  8. 11 Jan, 2019 1 commit
  9. 04 Jan, 2019 1 commit
    • Linus Torvalds's avatar
      Remove 'type' argument from access_ok() function · 96d4f267
      Linus Torvalds authored
      Nobody has actually used the type (VERIFY_READ vs VERIFY_WRITE) argument
      of the user address range verification function since we got rid of the
      old racy i386-only code to walk page tables by hand.
      
      It existed because the original 80386 would not honor the write protect
      bit when in kernel mode, so you had to do COW by hand before doing any
      user access.  But we haven't supported that in a long time, and these
      days the 'type' argument is a purely historical artifact.
      
      A discussion about extending 'user_access_begin()' to do the range
      checking resulted this patch, because there is no way we're going to
      move the old VERIFY_xyz interface to that model.  And it's best done at
      the end of the merge window when I've done most of my merges, so let's
      just get this done once and for all.
      
      This patch was mostly done with a sed-script, with manual fix-ups for
      the cases that weren't of the trivial 'access_ok(VERIFY_xyz' form.
      
      There were a couple of notable cases:
      
       - csky still had the old "verify_area()" name as an alias.
      
       - the iter_iov code had magical hardcoded knowledge of the actual
         values of VERIFY_{READ,WRITE} (not that they mattered, since nothing
         really used it)
      
       - microblaze used the type argument for a debug printout
      
      but other than those oddities this should be a total no-op patch.
      
      I tried to fix up all architectures, did fairly extensive grepping for
      access_ok() uses, and the changes are trivial, but I may have missed
      something.  Any missed conversion should be trivially fixable, though.
      Signed-off-by: 's avatarLinus Torvalds <torvalds@linux-foundation.org>
      96d4f267
  10. 01 Jan, 2019 1 commit
    • Eric Biggers's avatar
      KEYS: fix parsing invalid pkey info string · 57b0e314
      Eric Biggers authored
      We need to check the return value of match_token() for Opt_err before
      doing anything with it.
      
      [ Not only did the old "-1" value for Opt_err cause problems for the
        __test_and_set_bit(), as fixed in commit 94c13f66 ("security:
        don't use a negative Opt_err token index"), but accessing
        "args[0].from" is invalid for the Opt_err case, as pointed out by Eric
        later.  - Linus ]
      
      Reported-by: syzbot+a22e0dc07567662c50bc@syzkaller.appspotmail.com
      Fixes: 00d60fd3 ("KEYS: Provide keyctls to drive the new key type ops for asymmetric keys [ver #2]")
      Signed-off-by: 's avatarEric Biggers <ebiggers@google.com>
      Cc: stable@kernel.org # 4.20
      Signed-off-by: 's avatarLinus Torvalds <torvalds@linux-foundation.org>
      57b0e314
  11. 28 Dec, 2018 1 commit
  12. 21 Dec, 2018 21 commits
  13. 20 Dec, 2018 2 commits