An error occurred fetching the project authors.
  1. 30 May, 2019 4 commits
  2. 29 May, 2019 1 commit
  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: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarJames Morris <james.morris@microsoft.com>
      ede0fa98
  4. 23 Jan, 2019 1 commit
  5. 12 Dec, 2018 1 commit
    • Paul Gortmaker's avatar
      security: audit and remove any unnecessary uses of module.h · 876979c9
      Paul Gortmaker authored
      Historically a lot of these existed because we did not have
      a distinction between what was modular code and what was providing
      support to modules via EXPORT_SYMBOL and friends.  That changed
      when we forked out support for the latter into the export.h file.
      This means we should be able to reduce the usage of module.h
      in code that is obj-y Makefile or bool Kconfig.
      
      The advantage in removing such instances is that module.h itself
      sources about 15 other headers; adding significantly to what we feed
      cpp, and it can obscure what headers we are effectively using.
      
      Since module.h might have been the implicit source for init.h
      (for __init) and for export.h (for EXPORT_SYMBOL) we consider each
      instance for the presence of either and replace as needed.
      
      Cc: James Morris <jmorris@namei.org>
      Cc: "Serge E. Hallyn" <serge@hallyn.com>
      Cc: John Johansen <john.johansen@canonical.com>
      Cc: Mimi Zohar <zohar@linux.ibm.com>
      Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
      Cc: David Howells <dhowells@redhat.com>
      Cc: linux-security-module@vger.kernel.org
      Cc: linux-integrity@vger.kernel.org
      Cc: keyrings@vger.kernel.org
      Signed-off-by: default avatarPaul Gortmaker <paul.gortmaker@windriver.com>
      Signed-off-by: default avatarJames Morris <james.morris@microsoft.com>
      876979c9
  6. 04 Dec, 2017 1 commit
  7. 15 Nov, 2017 1 commit
    • Baolin Wang's avatar
      security: keys: Replace time_t/timespec with time64_t · 074d5898
      Baolin Wang authored
      The 'struct key' will use 'time_t' which we try to remove in the
      kernel, since 'time_t' is not year 2038 safe on 32bit systems.
      Also the 'struct keyring_search_context' will use 'timespec' type
      to record current time, which is also not year 2038 safe on 32bit
      systems.
      
      Thus this patch replaces 'time_t' with 'time64_t' which is year 2038
      safe for 'struct key', and replace 'timespec' with 'time64_t' for the
      'struct keyring_search_context', since we only look at the the seconds
      part of 'timespec' variable. Moreover we also change the codes where
      using the 'time_t' and 'timespec', and we can get current time by
      ktime_get_real_seconds() instead of current_kernel_time(), and use
      'TIME64_MAX' macro to initialize the 'time64_t' type variable.
      
      Especially in proc.c file, we have replaced 'unsigned long' and 'timespec'
      type with 'u64' and 'time64_t' type to save the timeout value, which means
      user will get one 'u64' type timeout value by issuing proc_keys_show()
      function.
      Signed-off-by: default avatarBaolin Wang <baolin.wang@linaro.org>
      Reviewed-by: default avatarArnd Bergmann <arnd@arndb.de>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarJames Morris <james.l.morris@oracle.com>
      074d5898
  8. 02 Nov, 2017 1 commit
  9. 18 Oct, 2017 2 commits
    • Eric Biggers's avatar
      KEYS: Load key expiry time atomically in keyring_search_iterator() · 9d6c8711
      Eric Biggers authored
      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: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      9d6c8711
    • David Howells's avatar
      KEYS: Fix race between updating and finding a negative key · 363b02da
      David Howells authored
      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: 146aa8b1 ("KEYS: Merge the type-specific data with the payload data")
      Cc: stable@vger.kernel.org # v4.4+
      Reported-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarEric Biggers <ebiggers@google.com>
      363b02da
  10. 25 Sep, 2017 2 commits
    • Eric Biggers's avatar
      KEYS: prevent creating a different user's keyrings · 237bbd29
      Eric Biggers authored
      It was possible for an unprivileged user to create the user and user
      session keyrings for another user.  For example:
      
          sudo -u '#3000' sh -c 'keyctl add keyring _uid.4000 "" @u
                                 keyctl add keyring _uid_ses.4000 "" @u
                                 sleep 15' &
          sleep 1
          sudo -u '#4000' keyctl describe @u
          sudo -u '#4000' keyctl describe @us
      
      This is problematic because these "fake" keyrings won't have the right
      permissions.  In particular, the user who created them first will own
      them and will have full access to them via the possessor permissions,
      which can be used to compromise the security of a user's keys:
      
          -4: alswrv-----v------------  3000     0 keyring: _uid.4000
          -5: alswrv-----v------------  3000     0 keyring: _uid_ses.4000
      
      Fix it by marking user and user session keyrings with a flag
      KEY_FLAG_UID_KEYRING.  Then, when searching for a user or user session
      keyring by name, skip all keyrings that don't have the flag set.
      
      Fixes: 69664cf1 ("keys: don't generate user and user session keyrings unless they're accessed")
      Cc: <stable@vger.kernel.org>	[v2.6.26+]
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      237bbd29
    • Eric Biggers's avatar
      KEYS: fix writing past end of user-supplied buffer in keyring_read() · e645016a
      Eric Biggers authored
      Userspace can call keyctl_read() on a keyring to get the list of IDs of
      keys in the keyring.  But if the user-supplied buffer is too small, the
      kernel would write the full list anyway --- which will corrupt whatever
      userspace memory happened to be past the end of the buffer.  Fix it by
      only filling the space that is available.
      
      Fixes: b2a4df20 ("KEYS: Expand the capacity of a keyring")
      Cc: <stable@vger.kernel.org>	[v3.13+]
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      e645016a
  11. 09 Jun, 2017 1 commit
  12. 04 Apr, 2017 2 commits
    • Mat Martineau's avatar
      KEYS: Add KEYCTL_RESTRICT_KEYRING · 6563c91f
      Mat Martineau authored
      Keyrings recently gained restrict_link capabilities that allow
      individual keys to be validated prior to linking.  This functionality
      was only available using internal kernel APIs.
      
      With the KEYCTL_RESTRICT_KEYRING command existing keyrings can be
      configured to check the content of keys before they are linked, and
      then allow or disallow linkage of that key to the keyring.
      
      To restrict a keyring, call:
      
        keyctl(KEYCTL_RESTRICT_KEYRING, key_serial_t keyring, const char *type,
               const char *restriction)
      
      where 'type' is the name of a registered key type and 'restriction' is a
      string describing how key linkage is to be restricted. The restriction
      option syntax is specific to each key type.
      Signed-off-by: default avatarMat Martineau <mathew.j.martineau@linux.intel.com>
      6563c91f
    • Mat Martineau's avatar
      KEYS: Use structure to capture key restriction function and data · 2b6aa412
      Mat Martineau authored
      Replace struct key's restrict_link function pointer with a pointer to
      the new struct key_restriction. The structure contains pointers to the
      restriction function as well as relevant data for evaluating the
      restriction.
      
      The garbage collector checks restrict_link->keytype when key types are
      unregistered. Restrictions involving a removed key type are converted
      to use restrict_link_reject so that restrictions cannot be removed by
      unregistering key types.
      Signed-off-by: default avatarMat Martineau <mathew.j.martineau@linux.intel.com>
      2b6aa412
  13. 03 Apr, 2017 3 commits
  14. 11 Apr, 2016 2 commits
    • David Howells's avatar
      KEYS: Remove KEY_FLAG_TRUSTED and KEY_ALLOC_TRUSTED · 77f68bac
      David Howells authored
      Remove KEY_FLAG_TRUSTED and KEY_ALLOC_TRUSTED as they're no longer
      meaningful.  Also we can drop the trusted flag from the preparse structure.
      
      Given this, we no longer need to pass the key flags through to
      restrict_link().
      
      Further, we can now get rid of keyring_restrict_trusted_only() also.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      77f68bac
    • David Howells's avatar
      KEYS: Add a facility to restrict new links into a keyring · 5ac7eace
      David Howells authored
      Add a facility whereby proposed new links to be added to a keyring can be
      vetted, permitting them to be rejected if necessary.  This can be used to
      block public keys from which the signature cannot be verified or for which
      the signature verification fails.  It could also be used to provide
      blacklisting.
      
      This affects operations like add_key(), KEYCTL_LINK and KEYCTL_INSTANTIATE.
      
      To this end:
      
       (1) A function pointer is added to the key struct that, if set, points to
           the vetting function.  This is called as:
      
      	int (*restrict_link)(struct key *keyring,
      			     const struct key_type *key_type,
      			     unsigned long key_flags,
      			     const union key_payload *key_payload),
      
           where 'keyring' will be the keyring being added to, key_type and
           key_payload will describe the key being added and key_flags[*] can be
           AND'ed with KEY_FLAG_TRUSTED.
      
           [*] This parameter will be removed in a later patch when
           	 KEY_FLAG_TRUSTED is removed.
      
           The function should return 0 to allow the link to take place or an
           error (typically -ENOKEY, -ENOPKG or -EKEYREJECTED) to reject the
           link.
      
           The pointer should not be set directly, but rather should be set
           through keyring_alloc().
      
           Note that if called during add_key(), preparse is called before this
           method, but a key isn't actually allocated until after this function
           is called.
      
       (2) KEY_ALLOC_BYPASS_RESTRICTION is added.  This can be passed to
           key_create_or_update() or key_instantiate_and_link() to bypass the
           restriction check.
      
       (3) KEY_FLAG_TRUSTED_ONLY is removed.  The entire contents of a keyring
           with this restriction emplaced can be considered 'trustworthy' by
           virtue of being in the keyring when that keyring is consulted.
      
       (4) key_alloc() and keyring_alloc() take an extra argument that will be
           used to set restrict_link in the new key.  This ensures that the
           pointer is set before the key is published, thus preventing a window
           of unrestrictedness.  Normally this argument will be NULL.
      
       (5) As a temporary affair, keyring_restrict_trusted_only() is added.  It
           should be passed to keyring_alloc() as the extra argument instead of
           setting KEY_FLAG_TRUSTED_ONLY on a keyring.  This will be replaced in
           a later patch with functions that look in the appropriate places for
           authoritative keys.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarMimi Zohar <zohar@linux.vnet.ibm.com>
      5ac7eace
  15. 21 Oct, 2015 1 commit
    • David Howells's avatar
      KEYS: Merge the type-specific data with the payload data · 146aa8b1
      David Howells authored
      Merge the type-specific data with the payload data into one four-word chunk
      as it seems pointless to keep them separate.
      
      Use user_key_payload() for accessing the payloads of overloaded
      user-defined keys.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      cc: linux-cifs@vger.kernel.org
      cc: ecryptfs@vger.kernel.org
      cc: linux-ext4@vger.kernel.org
      cc: linux-f2fs-devel@lists.sourceforge.net
      cc: linux-nfs@vger.kernel.org
      cc: ceph-devel@vger.kernel.org
      cc: linux-ima-devel@lists.sourceforge.net
      146aa8b1
  16. 28 Jul, 2015 1 commit
  17. 01 Dec, 2014 2 commits
    • David Howells's avatar
      KEYS: request_key() should reget expired keys rather than give EKEYEXPIRED · 0b0a8415
      David Howells authored
      Since the keyring facility can be viewed as a cache (at least in some
      applications), the local expiration time on the key should probably be viewed
      as a 'needs updating after this time' property rather than an absolute 'anyone
      now wanting to use this object is out of luck' property.
      
      Since request_key() is the main interface for the usage of keys, this should
      update or replace an expired key rather than issuing EKEYEXPIRED if the local
      expiration has been reached (ie. it should refresh the cache).
      
      For absolute conditions where refreshing the cache probably doesn't help, the
      key can be negatively instantiated using KEYCTL_REJECT_KEY with EKEYEXPIRED
      given as the error to issue.  This will still cause request_key() to return
      EKEYEXPIRED as that was explicitly set.
      
      In the future, if the key type has an update op available, we might want to
      upcall with the expired key and allow the upcall to update it.  We would pass
      a different operation name (the first column in /etc/request-key.conf) to the
      request-key program.
      
      request_key() returning EKEYEXPIRED is causing an NFS problem which Chuck
      Lever describes thusly:
      
      	After about 10 minutes, my NFSv4 functional tests fail because the
      	ownership of the test files goes to "-2". Looking at /proc/keys
      	shows that the id_resolv keys that map to my test user ID have
      	expired. The ownership problem persists until the expired keys are
      	purged from the keyring, and fresh keys are obtained.
      
      	I bisected the problem to 3.13 commit b2a4df20 ("KEYS: Expand
      	the capacity of a keyring"). This commit inadvertantly changes the
      	API contract of the internal function keyring_search_aux().
      
      	The root cause appears to be that b2a4df20 made "no state check"
      	the default behavior. "No state check" means the keyring search
      	iterator function skips checking the key's expiry timeout, and
      	returns expired keys.  request_key_and_link() depends on getting
      	an -EAGAIN result code to know when to perform an upcall to refresh
      	an expired key.
      
      This patch can be tested directly by:
      
      	keyctl request2 user debug:fred a @s
      	keyctl timeout %user:debug:fred 3
      	sleep 4
      	keyctl request2 user debug:fred a @s
      
      Without the patch, the last command gives error EKEYEXPIRED, but with the
      command it gives a new key.
      Reported-by: default avatarCarl Hetherington <cth@carlh.net>
      Reported-by: default avatarChuck Lever <chuck.lever@oracle.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarChuck Lever <chuck.lever@oracle.com>
      0b0a8415
    • David Howells's avatar
      KEYS: Simplify KEYRING_SEARCH_{NO,DO}_STATE_CHECK flags · 054f6180
      David Howells authored
      Simplify KEYRING_SEARCH_{NO,DO}_STATE_CHECK flags to be two variations of the
      same flag.  They are effectively mutually exclusive and one or the other
      should be provided, but not both.
      
      Keyring cycle detection and key possession determination are the only things
      that set NO_STATE_CHECK, except that neither flag really does anything there
      because neither purpose makes use of the keyring_search_iterator() function,
      but rather provides their own.
      
      For cycle detection we definitely want to check inside of expired keyrings,
      just so that we don't create a cycle we can't get rid of.  Revoked keyrings
      are cleared at revocation time and can't then be reused, so shouldn't be a
      problem either way.
      
      For possession determination, we *might* want to validate each keyring before
      searching it: do you possess a key that's hidden behind an expired or just
      plain inaccessible keyring?  Currently, the answer is yes.  Note that you
      cannot, however, possess a key behind a revoked keyring because they are
      cleared on revocation.
      
      keyring_search() sets DO_STATE_CHECK, which is correct.
      
      request_key_and_link() currently doesn't specify whether to check the key
      state or not - but it should set DO_STATE_CHECK.
      
      key_get_instantiation_authkey() also currently doesn't specify whether to
      check the key state or not - but it probably should also set DO_STATE_CHECK.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarChuck Lever <chuck.lever@oracle.com>
      054f6180
  18. 16 Sep, 2014 3 commits
  19. 22 Jul, 2014 1 commit
  20. 14 Mar, 2014 1 commit
  21. 10 Mar, 2014 1 commit
  22. 02 Dec, 2013 3 commits
    • David Howells's avatar
      KEYS: Fix searching of nested keyrings · 9c5e45df
      David Howells authored
      If a keyring contains more than 16 keyrings (the capacity of a single node in
      the associative array) then those keyrings are split over multiple nodes
      arranged as a tree.
      
      If search_nested_keyrings() is called to search the keyring then it will
      attempt to manually walk over just the 0 branch of the associative array tree
      where all the keyring links are stored.  This works provided the key is found
      before the algorithm steps from one node containing keyrings to a child node
      or if there are sufficiently few keyring links that the keyrings are all in
      one node.
      
      However, if the algorithm does need to step from a node to a child node, it
      doesn't change the node pointer unless a shortcut also gets transited.  This
      means that the algorithm will keep scanning the same node over and over again
      without terminating and without returning.
      
      To fix this, move the internal-pointer-to-node translation from inside the
      shortcut transit handler so that it applies it to node arrival as well.
      
      This can be tested by:
      
      	r=`keyctl newring sandbox @s`
      	for ((i=0; i<=16; i++)); do keyctl newring ring$i $r; done
      	for ((i=0; i<=16; i++)); do keyctl add user a$i a %:ring$i; done
      	for ((i=0; i<=16; i++)); do keyctl search $r user a$i; done
      	for ((i=17; i<=20; i++)); do keyctl search $r user a$i; done
      
      The searches should all complete successfully (or with an error for 17-20),
      but instead one or more of them will hang.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarStephen Gallagher <sgallagh@redhat.com>
      9c5e45df
    • David Howells's avatar
      KEYS: Fix multiple key add into associative array · 23fd78d7
      David Howells authored
      If sufficient keys (or keyrings) are added into a keyring such that a node in
      the associative array's tree overflows (each node has a capacity N, currently
      16) and such that all N+1 keys have the same index key segment for that level
      of the tree (the level'th nibble of the index key), then assoc_array_insert()
      calls ops->diff_objects() to indicate at which bit position the two index keys
      vary.
      
      However, __key_link_begin() passes a NULL object to assoc_array_insert() with
      the intention of supplying the correct pointer later before we commit the
      change.  This means that keyring_diff_objects() is given a NULL pointer as one
      of its arguments which it does not expect.  This results in an oops like the
      attached.
      
      With the previous patch to fix the keyring hash function, this can be forced
      much more easily by creating a keyring and only adding keyrings to it.  Add any
      other sort of key and a different insertion path is taken - all 16+1 objects
      must want to cluster in the same node slot.
      
      This can be tested by:
      
      	r=`keyctl newring sandbox @s`
      	for ((i=0; i<=16; i++)); do keyctl newring ring$i $r; done
      
      This should work fine, but oopses when the 17th keyring is added.
      
      Since ops->diff_objects() is always called with the first pointer pointing to
      the object to be inserted (ie. the NULL pointer), we can fix the problem by
      changing the to-be-inserted object pointer to point to the index key passed
      into assoc_array_insert() instead.
      
      Whilst we're at it, we also switch the arguments so that they are the same as
      for ->compare_object().
      
      BUG: unable to handle kernel NULL pointer dereference at 0000000000000088
      IP: [<ffffffff81191ee4>] hash_key_type_and_desc+0x18/0xb0
      ...
      RIP: 0010:[<ffffffff81191ee4>] hash_key_type_and_desc+0x18/0xb0
      ...
      Call Trace:
       [<ffffffff81191f9d>] keyring_diff_objects+0x21/0xd2
       [<ffffffff811f09ef>] assoc_array_insert+0x3b6/0x908
       [<ffffffff811929a7>] __key_link_begin+0x78/0xe5
       [<ffffffff81191a2e>] key_create_or_update+0x17d/0x36a
       [<ffffffff81192e0a>] SyS_add_key+0x123/0x183
       [<ffffffff81400ddb>] tracesys+0xdd/0xe2
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarStephen Gallagher <sgallagh@redhat.com>
      23fd78d7
    • David Howells's avatar
      KEYS: Fix the keyring hash function · d54e58b7
      David Howells authored
      The keyring hash function (used by the associative array) is supposed to clear
      the bottommost nibble of the index key (where the hash value resides) for
      keyrings and make sure it is non-zero for non-keyrings.  This is done to make
      keyrings cluster together on one branch of the tree separately to other keys.
      
      Unfortunately, the wrong mask is used, so only the bottom two bits are
      examined and cleared and not the whole bottom nibble.  This means that keys
      and keyrings can still be successfully searched for under most circumstances
      as the hash is consistent in its miscalculation, but if a keyring's
      associative array bottom node gets filled up then approx 75% of the keyrings
      will not be put into the 0 branch.
      
      The consequence of this is that a key in a keyring linked to by another
      keyring, ie.
      
      	keyring A -> keyring B -> key
      
      may not be found if the search starts at keyring A and then descends into
      keyring B because search_nested_keyrings() only searches up the 0 branch (as it
      "knows" all keyrings must be there and not elsewhere in the tree).
      
      The fix is to use the right mask.
      
      This can be tested with:
      
      	r=`keyctl newring sandbox @s`
      	for ((i=0; i<=16; i++)); do keyctl newring ring$i $r; done
      	for ((i=0; i<=16; i++)); do keyctl add user a$i a %:ring$i; done
      	for ((i=0; i<=16; i++)); do keyctl search $r user a$i; done
      
      This creates a sandbox keyring, then creates 17 keyrings therein (labelled
      ring0..ring16).  This causes the root node of the sandbox's associative array
      to overflow and for the tree to have extra nodes inserted.
      
      Each keyring then is given a user key (labelled aN for ringN) for us to search
      for.
      
      We then search for the user keys we added, starting from the sandbox.  If
      working correctly, it should return the same ordered list of key IDs as
      for...keyctl add... did.  Without this patch, it reports ENOKEY "Required key
      not available" for some of the keys.  Just which keys get this depends as the
      kernel pointer to the key type forms part of the hash function.
      Reported-by: default avatarNalin Dahyabhai <nalin@redhat.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarStephen Gallagher <sgallagh@redhat.com>
      d54e58b7
  23. 14 Nov, 2013 1 commit
    • David Howells's avatar
      KEYS: Fix keyring content gc scanner · 62fe3182
      David Howells authored
      Key pointers stored in the keyring are marked in bit 1 to indicate if they
      point to a keyring.  We need to strip off this bit before using the pointer
      when iterating over the keyring for the purpose of looking for links to garbage
      collect.
      
      This means that expirable keyrings aren't correctly expiring because the
      checker is seeing their key pointer with 2 added to it.
      
      Since the fix for this involves knowing about the internals of the keyring,
      key_gc_keyring() is moved to keyring.c and merged into keyring_gc().
      
      This can be tested by:
      
      	echo 2 >/proc/sys/kernel/keys/gc_delay
      	keyctl timeout `keyctl add keyring qwerty "" @s` 2
      	cat /proc/keys
      	sleep 5; cat /proc/keys
      
      which should see a keyring called "qwerty" appear in the session keyring and
      then disappear after it expires, and:
      
      	echo 2 >/proc/sys/kernel/keys/gc_delay
      	a=`keyctl get_persistent @s`
      	b=`keyctl add keyring 0 "" $a`
      	keyctl add user a a $b
      	keyctl timeout $b 2
      	cat /proc/keys
      	sleep 5; cat /proc/keys
      
      which should see a keyring called "0" with a key called "a" in it appear in the
      user's persistent keyring (which will be attached to the session keyring) and
      then both the "0" keyring and the "a" key should disappear when the "0" keyring
      expires.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarSimo Sorce <simo@redhat.com>
      62fe3182
  24. 30 Oct, 2013 2 commits
    • David Howells's avatar
      KEYS: Fix keyring quota misaccounting on key replacement and unlink · 034faeb9
      David Howells authored
      If a key is displaced from a keyring by a matching one, then four more bytes
      of quota are allocated to the keyring - despite the fact that the keyring does
      not change in size.
      
      Further, when a key is unlinked from a keyring, the four bytes of quota
      allocated the link isn't recovered and returned to the user's pool.
      
      The first can be tested by repeating:
      
      	keyctl add big_key a fred @s
      	cat /proc/key-users
      
      (Don't put it in a shell loop otherwise the garbage collector won't have time
      to clear the displaced keys, thus affecting the result).
      
      This was causing the kerberos keyring to run out of room fairly quickly.
      
      The second can be tested by:
      
      	cat /proc/key-users
      	a=`keyctl add user a a @s`
      	cat /proc/key-users
      	keyctl unlink $a
      	sleep 1 # Give RCU a chance to delete the key
      	cat /proc/key-users
      
      assuming no system activity that otherwise adds/removes keys, the amount of
      key data allocated should go up (say 40/20000 -> 47/20000) and then return to
      the original value at the end.
      Reported-by: default avatarStephen Gallagher <sgallagh@redhat.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      034faeb9
    • David Howells's avatar
      KEYS: Fix a race between negating a key and reading the error set · 74792b00
      David Howells authored
      key_reject_and_link() marking a key as negative and setting the error with
      which it was negated races with keyring searches and other things that read
      that error.
      
      The fix is to switch the order in which the assignments are done in
      key_reject_and_link() and to use memory barriers.
      
      Kudos to Dave Wysochanski <dwysocha@redhat.com> and Scott Mayhew
      <smayhew@redhat.com> for tracking this down.
      
      This may be the cause of:
      
      BUG: unable to handle kernel NULL pointer dereference at 0000000000000070
      IP: [<ffffffff81219011>] wait_for_key_construction+0x31/0x80
      PGD c6b2c3067 PUD c59879067 PMD 0
      Oops: 0000 [#1] SMP
      last sysfs file: /sys/devices/system/cpu/cpu3/cache/index2/shared_cpu_map
      CPU 0
      Modules linked in: ...
      
      Pid: 13359, comm: amqzxma0 Not tainted 2.6.32-358.20.1.el6.x86_64 #1 IBM System x3650 M3 -[7945PSJ]-/00J6159
      RIP: 0010:[<ffffffff81219011>] wait_for_key_construction+0x31/0x80
      RSP: 0018:ffff880c6ab33758  EFLAGS: 00010246
      RAX: ffffffff81219080 RBX: 0000000000000000 RCX: 0000000000000002
      RDX: ffffffff81219060 RSI: 0000000000000000 RDI: 0000000000000000
      RBP: ffff880c6ab33768 R08: 0000000000000000 R09: 0000000000000000
      R10: 0000000000000001 R11: 0000000000000000 R12: ffff880adfcbce40
      R13: ffffffffa03afb84 R14: ffff880adfcbce40 R15: ffff880adfcbce43
      FS:  00007f29b8042700(0000) GS:ffff880028200000(0000) knlGS:0000000000000000
      CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
      CR2: 0000000000000070 CR3: 0000000c613dc000 CR4: 00000000000007f0
      DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
      DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
      Process amqzxma0 (pid: 13359, threadinfo ffff880c6ab32000, task ffff880c610deae0)
      Stack:
       ffff880adfcbce40 0000000000000000 ffff880c6ab337b8 ffffffff81219695
      <d> 0000000000000000 ffff880a000000d0 ffff880c6ab337a8 000000000000000f
      <d> ffffffffa03afb93 000000000000000f ffff88186c7882c0 0000000000000014
      Call Trace:
       [<ffffffff81219695>] request_key+0x65/0xa0
       [<ffffffffa03a0885>] nfs_idmap_request_key+0xc5/0x170 [nfs]
       [<ffffffffa03a0eb4>] nfs_idmap_lookup_id+0x34/0x80 [nfs]
       [<ffffffffa03a1255>] nfs_map_group_to_gid+0x75/0xa0 [nfs]
       [<ffffffffa039a9ad>] decode_getfattr_attrs+0xbdd/0xfb0 [nfs]
       [<ffffffff81057310>] ? __dequeue_entity+0x30/0x50
       [<ffffffff8100988e>] ? __switch_to+0x26e/0x320
       [<ffffffffa039ae03>] decode_getfattr+0x83/0xe0 [nfs]
       [<ffffffffa039b610>] ? nfs4_xdr_dec_getattr+0x0/0xa0 [nfs]
       [<ffffffffa039b69f>] nfs4_xdr_dec_getattr+0x8f/0xa0 [nfs]
       [<ffffffffa02dada4>] rpcauth_unwrap_resp+0x84/0xb0 [sunrpc]
       [<ffffffffa039b610>] ? nfs4_xdr_dec_getattr+0x0/0xa0 [nfs]
       [<ffffffffa02cf923>] call_decode+0x1b3/0x800 [sunrpc]
       [<ffffffff81096de0>] ? wake_bit_function+0x0/0x50
       [<ffffffffa02cf770>] ? call_decode+0x0/0x800 [sunrpc]
       [<ffffffffa02d99a7>] __rpc_execute+0x77/0x350 [sunrpc]
       [<ffffffff81096c67>] ? bit_waitqueue+0x17/0xd0
       [<ffffffffa02d9ce1>] rpc_execute+0x61/0xa0 [sunrpc]
       [<ffffffffa02d03a5>] rpc_run_task+0x75/0x90 [sunrpc]
       [<ffffffffa02d04c2>] rpc_call_sync+0x42/0x70 [sunrpc]
       [<ffffffffa038ff80>] _nfs4_call_sync+0x30/0x40 [nfs]
       [<ffffffffa038836c>] _nfs4_proc_getattr+0xac/0xc0 [nfs]
       [<ffffffff810aac87>] ? futex_wait+0x227/0x380
       [<ffffffffa038b856>] nfs4_proc_getattr+0x56/0x80 [nfs]
       [<ffffffffa0371403>] __nfs_revalidate_inode+0xe3/0x220 [nfs]
       [<ffffffffa037158e>] nfs_revalidate_mapping+0x4e/0x170 [nfs]
       [<ffffffffa036f147>] nfs_file_read+0x77/0x130 [nfs]
       [<ffffffff811811aa>] do_sync_read+0xfa/0x140
       [<ffffffff81096da0>] ? autoremove_wake_function+0x0/0x40
       [<ffffffff8100bb8e>] ? apic_timer_interrupt+0xe/0x20
       [<ffffffff8100b9ce>] ? common_interrupt+0xe/0x13
       [<ffffffff81228ffb>] ? selinux_file_permission+0xfb/0x150
       [<ffffffff8121bed6>] ? security_file_permission+0x16/0x20
       [<ffffffff81181a95>] vfs_read+0xb5/0x1a0
       [<ffffffff81181bd1>] sys_read+0x51/0x90
       [<ffffffff810dc685>] ? __audit_syscall_exit+0x265/0x290
       [<ffffffff8100b072>] system_call_fastpath+0x16/0x1b
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      cc: Dave Wysochanski <dwysocha@redhat.com>
      cc: Scott Mayhew <smayhew@redhat.com>
      74792b00
  25. 25 Sep, 2013 1 commit