Skip to content
Snippets Groups Projects
  1. Oct 20, 2021
    • Eric W. Biederman's avatar
      ucounts: Move get_ucounts from cred_alloc_blank to key_change_session_keyring · 5ebcbe34
      Eric W. Biederman authored
      Setting cred->ucounts in cred_alloc_blank does not make sense.  The
      uid and user_ns are deliberately not set in cred_alloc_blank but
      instead the setting is delayed until key_change_session_keyring.
      
      So move dealing with ucounts into key_change_session_keyring as well.
      
      Unfortunately that movement of get_ucounts adds a new failure mode to
      key_change_session_keyring.  I do not see anything stopping the parent
      process from calling setuid and changing the relevant part of it's
      cred while keyctl_session_to_parent is running making it fundamentally
      necessary to call get_ucounts in key_change_session_keyring.  Which
      means that the new failure mode cannot be avoided.
      
      A failure of key_change_session_keyring results in a single threaded
      parent keeping it's existing credentials.  Which results in the parent
      process not being able to access the session keyring and whichever
      keys are in the new keyring.
      
      Further get_ucounts is only expected to fail if the number of bits in
      the refernece count for the structure is too few.
      
      Since the code has no other way to report the failure of get_ucounts
      and because such failures are not expected to be common add a WARN_ONCE
      to report this problem to userspace.
      
      Between the WARN_ONCE and the parent process not having access to
      the keys in the new session keyring I expect any failure of get_ucounts
      will be noticed and reported and we can find another way to handle this
      condition.  (Possibly by just making ucounts->count an atomic_long_t).
      
      Cc: stable@vger.kernel.org
      Fixes: 905ae01c ("Add a reference to ucounts for each cred")
      Link: https://lkml.kernel.org/r/7k0ias0uf.fsf_-_@disp2133
      
      
      Tested-by: default avatarYu Zhao <yuzhao@google.com>
      Reviewed-by: default avatarAlexey Gladkov <legion@kernel.org>
      Signed-off-by: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
      5ebcbe34
  2. May 12, 2021
  3. Apr 21, 2021
  4. Apr 14, 2021
    • Colin Ian King's avatar
      KEYS: trusted: Fix missing null return from kzalloc call · aec00aa0
      Colin Ian King authored
      
      The kzalloc call can return null with the GFP_KERNEL flag so
      add a null check and exit via a new error exit label. Use the
      same exit error label for another error path too.
      
      Addresses-Coverity: ("Dereference null return value")
      Fixes: 830027e2cb55 ("KEYS: trusted: Add generic trusted keys framework")
      Signed-off-by: default avatarColin Ian King <colin.king@canonical.com>
      Reviewed-by: default avatarSumit Garg <sumit.garg@linaro.org>
      Reviewed-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      Signed-off-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      aec00aa0
    • Sumit Garg's avatar
      KEYS: trusted: Introduce TEE based Trusted Keys · 0a95ebc9
      Sumit Garg authored
      
      Add support for TEE based trusted keys where TEE provides the functionality
      to seal and unseal trusted keys using hardware unique key.
      
      Refer to Documentation/staging/tee.rst for detailed information about TEE.
      
      Signed-off-by: default avatarSumit Garg <sumit.garg@linaro.org>
      Tested-by: default avatarJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
      Reviewed-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      Signed-off-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      0a95ebc9
    • Sumit Garg's avatar
      KEYS: trusted: Add generic trusted keys framework · 5d0682be
      Sumit Garg authored
      
      Current trusted keys framework is tightly coupled to use TPM device as
      an underlying implementation which makes it difficult for implementations
      like Trusted Execution Environment (TEE) etc. to provide trusted keys
      support in case platform doesn't posses a TPM device.
      
      Add a generic trusted keys framework where underlying implementations
      can be easily plugged in. Create struct trusted_key_ops to achieve this,
      which contains necessary functions of a backend.
      
      Also, define a module parameter in order to select a particular trust
      source in case a platform support multiple trust sources. In case its
      not specified then implementation itetrates through trust sources list
      starting with TPM and assign the first trust source as a backend which
      has initiazed successfully during iteration.
      
      Note that current implementation only supports a single trust source at
      runtime which is either selectable at compile time or during boot via
      aforementioned module parameter.
      
      Suggested-by: default avatarJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
      Signed-off-by: default avatarSumit Garg <sumit.garg@linaro.org>
      Reviewed-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      Signed-off-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      5d0682be
    • James Bottomley's avatar
      security: keys: trusted: Make sealed key properly interoperable · e5fb5d2c
      James Bottomley authored
      
      The current implementation appends a migratable flag to the end of a
      key, meaning the format isn't exactly interoperable because the using
      party needs to know to strip this extra byte.  However, all other
      consumers of TPM sealed blobs expect the unseal to return exactly the
      key.  Since TPM2 keys have a key property flag that corresponds to
      migratable, use that flag instead and make the actual key the only
      sealed quantity.  This is secure because the key properties are bound
      to a hash in the private part, so if they're altered the key won't
      load.
      
      Backwards compatibility is implemented by detecting whether we're
      loading a new format key or not and correctly setting migratable from
      the last byte of old format keys.
      
      Signed-off-by: default avatarJames Bottomley <James.Bottomley@HansenPartnership.com>
      Reviewed-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      Tested-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      Signed-off-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      e5fb5d2c
    • James Bottomley's avatar
      security: keys: trusted: use ASN.1 TPM2 key format for the blobs · f2219745
      James Bottomley authored
      
      Modify the TPM2 key format blob output to export and import in the
      ASN.1 form for TPM2 sealed object keys.  For compatibility with prior
      trusted keys, the importer will also accept two TPM2B quantities
      representing the public and private parts of the key.  However, the
      export via keyctl pipe will only output the ASN.1 format.
      
      The benefit of the ASN.1 format is that it's a standard and thus the
      exported key can be used by userspace tools (openssl_tpm2_engine,
      openconnect and tpm2-tss-engine).  The format includes policy
      specifications, thus it gets us out of having to construct policy
      handles in userspace and the format includes the parent meaning you
      don't have to keep passing it in each time.
      
      This patch only implements basic handling for the ASN.1 format, so
      keys with passwords but no policy.
      
      Signed-off-by: default avatarJames Bottomley <James.Bottomley@HansenPartnership.com>
      Tested-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      Signed-off-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      f2219745
    • James Bottomley's avatar
      security: keys: trusted: fix TPM2 authorizations · de66514d
      James Bottomley authored
      
      In TPM 1.2 an authorization was a 20 byte number.  The spec actually
      recommended you to hash variable length passwords and use the sha1
      hash as the authorization.  Because the spec doesn't require this
      hashing, the current authorization for trusted keys is a 40 digit hex
      number.  For TPM 2.0 the spec allows the passing in of variable length
      passwords and passphrases directly, so we should allow that in trusted
      keys for ease of use.  Update the 'blobauth' parameter to take this
      into account, so we can now use plain text passwords for the keys.
      
      so before
      
      keyctl add trusted kmk "new 32 blobauth=f572d396fae9206628714fb2ce00f72e94f2258fkeyhandle=81000001" @u
      
      after we will accept both the old hex sha1 form as well as a new
      directly supplied password:
      
      keyctl add trusted kmk "new 32 blobauth=hello keyhandle=81000001" @u
      
      Since a sha1 hex code must be exactly 40 bytes long and a direct
      password must be 20 or less, we use the length as the discriminator
      for which form is input.
      
      Note this is both and enhancement and a potential bug fix.  The TPM
      2.0 spec requires us to strip leading zeros, meaning empyty
      authorization is a zero length HMAC whereas we're currently passing in
      20 bytes of zeros.  A lot of TPMs simply accept this as OK, but the
      Microsoft TPM emulator rejects it with TPM_RC_BAD_AUTH, so this patch
      makes the Microsoft TPM emulator work with trusted keys.
      
      Fixes: 0fe54803 ("keys, trusted: seal/unseal with TPM 2.0 chips")
      Signed-off-by: default avatarJames Bottomley <James.Bottomley@HansenPartnership.com>
      Reviewed-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      Tested-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      Signed-off-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      de66514d
  5. Feb 16, 2021
    • Jarkko Sakkinen's avatar
      KEYS: trusted: Reserve TPM for seal and unseal operations · 8c657a05
      Jarkko Sakkinen authored
      
      When TPM 2.0 trusted keys code was moved to the trusted keys subsystem,
      the operations were unwrapped from tpm_try_get_ops() and tpm_put_ops(),
      which are used to take temporarily the ownership of the TPM chip. The
      ownership is only taken inside tpm_send(), but this is not sufficient,
      as in the key load TPM2_CC_LOAD, TPM2_CC_UNSEAL and TPM2_FLUSH_CONTEXT
      need to be done as a one single atom.
      
      Take the TPM chip ownership before sending anything with
      tpm_try_get_ops() and tpm_put_ops(), and use tpm_transmit_cmd() to send
      TPM commands instead of tpm_send(), reverting back to the old behaviour.
      
      Fixes: 2e19e101 ("KEYS: trusted: Move TPM2 trusted keys code")
      Reported-by: default avatar"James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
      Cc: stable@vger.kernel.org
      Cc: David Howells <dhowells@redhat.com>
      Cc: Mimi Zohar <zohar@linux.ibm.com>
      Cc: Sumit Garg <sumit.garg@linaro.org>
      Acked-by Sumit Garg <sumit.garg@linaro.org>
      Tested-by: default avatarMimi Zohar <zohar@linux.ibm.com>
      Signed-off-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      8c657a05
    • Jarkko Sakkinen's avatar
      KEYS: trusted: Fix migratable=1 failing · 8da7520c
      Jarkko Sakkinen authored
      
      Consider the following transcript:
      
      $ keyctl add trusted kmk "new 32 blobauth=helloworld keyhandle=80000000 migratable=1" @u
      add_key: Invalid argument
      
      The documentation has the following description:
      
        migratable=   0|1 indicating permission to reseal to new PCR values,
                      default 1 (resealing allowed)
      
      The consequence is that "migratable=1" should succeed. Fix this by
      allowing this condition to pass instead of return -EINVAL.
      
      [*] Documentation/security/keys/trusted-encrypted.rst
      
      Cc: stable@vger.kernel.org
      Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
      Cc: Mimi Zohar <zohar@linux.ibm.com>
      Cc: David Howells <dhowells@redhat.com>
      Fixes: d00a1c72 ("keys: add new trusted key-type")
      Signed-off-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      8da7520c
    • Jarkko Sakkinen's avatar
      KEYS: trusted: Fix incorrect handling of tpm_get_random() · 5df16caa
      Jarkko Sakkinen authored
      
      When tpm_get_random() was introduced, it defined the following API for the
      return value:
      
      1. A positive value tells how many bytes of random data was generated.
      2. A negative value on error.
      
      However, in the call sites the API was used incorrectly, i.e. as it would
      only return negative values and otherwise zero. Returning he positive read
      counts to the user space does not make any possible sense.
      
      Fix this by returning -EIO when tpm_get_random() returns a positive value.
      
      Fixes: 41ab999c ("tpm: Move tpm_get_random api into the TPM device driver")
      Cc: stable@vger.kernel.org
      Cc: Mimi Zohar <zohar@linux.ibm.com>
      Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
      Cc: David Howells <dhowells@redhat.com>
      Cc: Kent Yoder <key@linux.vnet.ibm.com>
      Signed-off-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
      Reviewed-by: default avatarMimi Zohar <zohar@linux.ibm.com>
      5df16caa
  6. Jan 21, 2021
  7. Nov 23, 2020
    • David Howells's avatar
      keys: Provide the original description to the key preparser · 8eb62169
      David Howells authored
      
      Provide the proposed description (add key) or the original description
      (update/instantiate key) when preparsing a key so that the key type can
      validate it against the data.
      
      This is important for rxrpc server keys as we need to check that they have
      the right amount of key material present - and it's better to do that when
      the key is loaded rather than deep in trying to process a response packet.
      
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
      cc: keyrings@vger.kernel.org
      8eb62169
  8. Nov 20, 2020
    • Eric Biggers's avatar
      crypto: sha - split sha.h into sha1.h and sha2.h · a24d22b2
      Eric Biggers authored
      
      Currently <crypto/sha.h> contains declarations for both SHA-1 and SHA-2,
      and <crypto/sha3.h> contains declarations for SHA-3.
      
      This organization is inconsistent, but more importantly SHA-1 is no
      longer considered to be cryptographically secure.  So to the extent
      possible, SHA-1 shouldn't be grouped together with any of the other SHA
      versions, and usage of it should be phased out.
      
      Therefore, split <crypto/sha.h> into two headers <crypto/sha1.h> and
      <crypto/sha2.h>, and make everyone explicitly specify whether they want
      the declarations for SHA-1, SHA-2, or both.
      
      This avoids making the SHA-1 declarations visible to files that don't
      want anything to do with SHA-1.  It also prepares for potentially moving
      sha1.h into a new insecure/ or dangerous/ directory.
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Acked-by: default avatarArd Biesheuvel <ardb@kernel.org>
      Acked-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      a24d22b2
  9. Oct 17, 2020
    • Jens Axboe's avatar
      task_work: cleanup notification modes · 91989c70
      Jens Axboe authored
      
      A previous commit changed the notification mode from true/false to an
      int, allowing notify-no, notify-yes, or signal-notify. This was
      backwards compatible in the sense that any existing true/false user
      would translate to either 0 (on notification sent) or 1, the latter
      which mapped to TWA_RESUME. TWA_SIGNAL was assigned a value of 2.
      
      Clean this up properly, and define a proper enum for the notification
      mode. Now we have:
      
      - TWA_NONE. This is 0, same as before the original change, meaning no
        notification requested.
      - TWA_RESUME. This is 1, same as before the original change, meaning
        that we use TIF_NOTIFY_RESUME.
      - TWA_SIGNAL. This uses TIF_SIGPENDING/JOBCTL_TASK_WORK for the
        notification.
      
      Clean up all the callers, switching their 0/1/false/true to using the
      appropriate TWA_* mode for notifications.
      
      Fixes: e91b4816 ("task_work: teach task_work_add() to do signal_wake_up()")
      Reviewed-by: default avatarThomas Gleixner <tglx@linutronix.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      91989c70
  10. Oct 03, 2020
  11. Aug 23, 2020
  12. Aug 07, 2020
    • Waiman Long's avatar
      mm, treewide: rename kzfree() to kfree_sensitive() · 453431a5
      Waiman Long authored
      
      As said by Linus:
      
        A symmetric naming is only helpful if it implies symmetries in use.
        Otherwise it's actively misleading.
      
        In "kzalloc()", the z is meaningful and an important part of what the
        caller wants.
      
        In "kzfree()", the z is actively detrimental, because maybe in the
        future we really _might_ want to use that "memfill(0xdeadbeef)" or
        something. The "zero" part of the interface isn't even _relevant_.
      
      The main reason that kzfree() exists is to clear sensitive information
      that should not be leaked to other future users of the same memory
      objects.
      
      Rename kzfree() to kfree_sensitive() to follow the example of the recently
      added kvfree_sensitive() and make the intention of the API more explicit.
      In addition, memzero_explicit() is used to clear the memory to make sure
      that it won't get optimized away by the compiler.
      
      The renaming is done by using the command sequence:
      
        git grep -w --name-only kzfree |\
        xargs sed -i 's/kzfree/kfree_sensitive/'
      
      followed by some editing of the kfree_sensitive() kerneldoc and adding
      a kzfree backward compatibility macro in slab.h.
      
      [akpm@linux-foundation.org: fs/crypto/inline_crypt.c needs linux/slab.h]
      [akpm@linux-foundation.org: fix fs/crypto/inline_crypt.c some more]
      
      Suggested-by: default avatarJoe Perches <joe@perches.com>
      Signed-off-by: default avatarWaiman Long <longman@redhat.com>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Acked-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarMichal Hocko <mhocko@suse.com>
      Acked-by: default avatarJohannes Weiner <hannes@cmpxchg.org>
      Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
      Cc: James Morris <jmorris@namei.org>
      Cc: "Serge E. Hallyn" <serge@hallyn.com>
      Cc: Joe Perches <joe@perches.com>
      Cc: Matthew Wilcox <willy@infradead.org>
      Cc: David Rientjes <rientjes@google.com>
      Cc: Dan Carpenter <dan.carpenter@oracle.com>
      Cc: "Jason A . Donenfeld" <Jason@zx2c4.com>
      Link: http://lkml.kernel.org/r/20200616154311.12314-3-longman@redhat.com
      
      
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      453431a5
  13. Aug 06, 2020
  14. Jun 09, 2020
  15. Jun 05, 2020
  16. Jun 02, 2020
    • David Howells's avatar
      keys: Implement update for the big_key type · b6f61c31
      David Howells authored
      
      Implement the ->update op for the big_key type.
      
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      b6f61c31
    • Jason A. Donenfeld's avatar
      security/keys: rewrite big_key crypto to use library interface · 521fd61c
      Jason A. Donenfeld authored
      
      A while back, I noticed that the crypto and crypto API usage in big_keys
      were entirely broken in multiple ways, so I rewrote it. Now, I'm
      rewriting it again, but this time using the simpler ChaCha20Poly1305
      library function. This makes the file considerably more simple; the
      diffstat alone should justify this commit. It also should be faster,
      since it no longer requires a mutex around the "aead api object" (nor
      allocations), allowing us to encrypt multiple items in parallel. We also
      benefit from being able to pass any type of pointer, so we can get rid
      of the ridiculously complex custom page allocator that big_key really
      doesn't need.
      
      [DH: Change the select CRYPTO_LIB_CHACHA20POLY1305 to a depends on as
       select doesn't propagate and the build can end up with an =y dependending
       on some =m pieces.
      
       The depends on CRYPTO also had to be removed otherwise the configurator
       complains about a recursive dependency.]
      
      Cc: Andy Lutomirski <luto@kernel.org>
      Cc: Greg KH <gregkh@linuxfoundation.org>
      Cc: Linus Torvalds <torvalds@linux-foundation.org>
      Cc: kernel-hardening@lists.openwall.com
      Reviewed-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      521fd61c
  17. May 19, 2020
    • David Howells's avatar
      keys: Make the KEY_NEED_* perms an enum rather than a mask · 8c0637e9
      David Howells authored
      
      Since the meaning of combining the KEY_NEED_* constants is undefined, make
      it so that you can't do that by turning them into an enum.
      
      The enum is also given some extra values to represent special
      circumstances, such as:
      
       (1) The '0' value is reserved and causes a warning to trap the parameter
           being unset.
      
       (2) The key is to be unlinked and we require no permissions on it, only
           the keyring, (this replaces the KEY_LOOKUP_FOR_UNLINK flag).
      
       (3) An override due to CAP_SYS_ADMIN.
      
       (4) An override due to an instantiation token being present.
      
       (5) The permissions check is being deferred to later key_permission()
           calls.
      
      The extra values give the opportunity for LSMs to audit these situations.
      
      [Note: This really needs overhauling so that lookup_user_key() tells
       key_task_permission() and the LSM what operation is being done and leaves
       it to those functions to decide how to map that onto the available
       permits.  However, I don't really want to make these change in the middle
       of the notifications patchset.]
      
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
      cc: Paul Moore <paul@paul-moore.com>
      cc: Stephen Smalley <stephen.smalley.work@gmail.com>
      cc: Casey Schaufler <casey@schaufler-ca.com>
      cc: keyrings@vger.kernel.org
      cc: selinux@vger.kernel.org
      8c0637e9
    • David Howells's avatar
      watch_queue: Add a key/keyring notification facility · f7e47677
      David Howells authored
      
      Add a key/keyring change notification facility whereby notifications about
      changes in key and keyring content and attributes can be received.
      
      Firstly, an event queue needs to be created:
      
      	pipe2(fds, O_NOTIFICATION_PIPE);
      	ioctl(fds[1], IOC_WATCH_QUEUE_SET_SIZE, 256);
      
      then a notification can be set up to report notifications via that queue:
      
      	struct watch_notification_filter filter = {
      		.nr_filters = 1,
      		.filters = {
      			[0] = {
      				.type = WATCH_TYPE_KEY_NOTIFY,
      				.subtype_filter[0] = UINT_MAX,
      			},
      		},
      	};
      	ioctl(fds[1], IOC_WATCH_QUEUE_SET_FILTER, &filter);
      	keyctl_watch_key(KEY_SPEC_SESSION_KEYRING, fds[1], 0x01);
      
      After that, records will be placed into the queue when events occur in
      which keys are changed in some way.  Records are of the following format:
      
      	struct key_notification {
      		struct watch_notification watch;
      		__u32	key_id;
      		__u32	aux;
      	} *n;
      
      Where:
      
      	n->watch.type will be WATCH_TYPE_KEY_NOTIFY.
      
      	n->watch.subtype will indicate the type of event, such as
      	NOTIFY_KEY_REVOKED.
      
      	n->watch.info & WATCH_INFO_LENGTH will indicate the length of the
      	record.
      
      	n->watch.info & WATCH_INFO_ID will be the second argument to
      	keyctl_watch_key(), shifted.
      
      	n->key will be the ID of the affected key.
      
      	n->aux will hold subtype-dependent information, such as the key
      	being linked into the keyring specified by n->key in the case of
      	NOTIFY_KEY_LINKED.
      
      Note that it is permissible for event records to be of variable length -
      or, at least, the length may be dependent on the subtype.  Note also that
      the queue can be shared between multiple notifications of various types.
      
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarJames Morris <jamorris@linux.microsoft.com>
      f7e47677
  18. May 08, 2020
  19. Apr 16, 2020
    • Vasily Averin's avatar
      keys: Fix proc_keys_next to increase position index · 86d32f9a
      Vasily Averin authored
      If seq_file .next function does not change position index,
      read after some lseek can generate unexpected output:
      
          $ dd if=/proc/keys bs=1  # full usual output
          0f6bfdf5 I--Q---     2 perm 3f010000  1000  1000 user      4af2f79ab8848d0a: 740
          1fb91b32 I--Q---     3 perm 1f3f0000  1000 65534 keyring   _uid.1000: 2
          27589480 I--Q---     1 perm 0b0b0000     0     0 user      invocation_id: 16
          2f33ab67 I--Q---   152 perm 3f030000     0     0 keyring   _ses: 2
          33f1d8fa I--Q---     4 perm 3f030000  1000  1000 keyring   _ses: 1
          3d427fda I--Q---     2 perm 3f010000  1000  1000 user      69ec44aec7678e5a: 740
          3ead4096 I--Q---     1 perm 1f3f0000  1000 65534 keyring   _uid_ses.1000: 1
          521+0 records in
          521+0 records out
          521 bytes copied, 0,00123769 s, 421 kB/s
      
      But a read after lseek in middle of last line results in the partial
      last line and then a repeat of the final line:
      
          $ dd if=/proc/keys bs=500 skip=1
          dd: /proc/keys: cannot skip to specified offset
          g   _uid_ses.1000: 1
          3ead4096 I--Q---     1 perm 1f3f0000  1000 65534 keyring   _uid_ses.1000: 1
          0+1 records in
          0+1 records out
          97 bytes copied, 0,000135035 s, 718 kB/s
      
      and a read after lseek beyond end of file results in the last line being
      shown:
      
          $ dd if=/proc/keys bs=1000 skip=1   # read after lseek beyond end of file
          dd: /proc/keys: cannot skip to specified offset
          3ead4096 I--Q---     1 perm 1f3f0000  1000 65534 keyring   _uid_ses.1000: 1
          0+1 records in
          0+1 records out
          76 bytes copied, 0,000119981 s, 633 kB/s
      
      See https://bugzilla.kernel.org/show_bug.cgi?id=206283
      
      
      
      Fixes: 1f4aace6 ("fs/seq_file.c: simplify seq_file iteration code ...")
      Signed-off-by: default avatarVasily Averin <vvs@virtuozzo.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      86d32f9a
  20. Mar 29, 2020
    • Waiman Long's avatar
      KEYS: Avoid false positive ENOMEM error on key read · 4f088249
      Waiman Long authored
      
      By allocating a kernel buffer with a user-supplied buffer length, it
      is possible that a false positive ENOMEM error may be returned because
      the user-supplied length is just too large even if the system do have
      enough memory to hold the actual key data.
      
      Moreover, if the buffer length is larger than the maximum amount of
      memory that can be returned by kmalloc() (2^(MAX_ORDER-1) number of
      pages), a warning message will also be printed.
      
      To reduce this possibility, we set a threshold (PAGE_SIZE) over which we
      do check the actual key length first before allocating a buffer of the
      right size to hold it. The threshold is arbitrary, it is just used to
      trigger a buffer length check. It does not limit the actual key length
      as long as there is enough memory to satisfy the memory request.
      
      To further avoid large buffer allocation failure due to page
      fragmentation, kvmalloc() is used to allocate the buffer so that vmapped
      pages can be used when there is not a large enough contiguous set of
      pages available for allocation.
      
      In the extremely unlikely scenario that the key keeps on being changed
      and made longer (still <= buflen) in between 2 __keyctl_read_key()
      calls, the __keyctl_read_key() calling loop in keyctl_read_key() may
      have to be iterated a large number of times, but definitely not infinite.
      
      Signed-off-by: default avatarWaiman Long <longman@redhat.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      4f088249
    • Waiman Long's avatar
      KEYS: Don't write out to userspace while holding key semaphore · d3ec10aa
      Waiman Long authored
      
      A lockdep circular locking dependency report was seen when running a
      keyutils test:
      
      [12537.027242] ======================================================
      [12537.059309] WARNING: possible circular locking dependency detected
      [12537.088148] 4.18.0-147.7.1.el8_1.x86_64+debug #1 Tainted: G OE    --------- -  -
      [12537.125253] ------------------------------------------------------
      [12537.153189] keyctl/25598 is trying to acquire lock:
      [12537.175087] 000000007c39f96c (&mm->mmap_sem){++++}, at: __might_fault+0xc4/0x1b0
      [12537.208365]
      [12537.208365] but task is already holding lock:
      [12537.234507] 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220
      [12537.270476]
      [12537.270476] which lock already depends on the new lock.
      [12537.270476]
      [12537.307209]
      [12537.307209] the existing dependency chain (in reverse order) is:
      [12537.340754]
      [12537.340754] -> #3 (&type->lock_class){++++}:
      [12537.367434]        down_write+0x4d/0x110
      [12537.385202]        __key_link_begin+0x87/0x280
      [12537.405232]        request_key_and_link+0x483/0xf70
      [12537.427221]        request_key+0x3c/0x80
      [12537.444839]        dns_query+0x1db/0x5a5 [dns_resolver]
      [12537.468445]        dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs]
      [12537.496731]        cifs_reconnect+0xe04/0x2500 [cifs]
      [12537.519418]        cifs_readv_from_socket+0x461/0x690 [cifs]
      [12537.546263]        cifs_read_from_socket+0xa0/0xe0 [cifs]
      [12537.573551]        cifs_demultiplex_thread+0x311/0x2db0 [cifs]
      [12537.601045]        kthread+0x30c/0x3d0
      [12537.617906]        ret_from_fork+0x3a/0x50
      [12537.636225]
      [12537.636225] -> #2 (root_key_user.cons_lock){+.+.}:
      [12537.664525]        __mutex_lock+0x105/0x11f0
      [12537.683734]        request_key_and_link+0x35a/0xf70
      [12537.705640]        request_key+0x3c/0x80
      [12537.723304]        dns_query+0x1db/0x5a5 [dns_resolver]
      [12537.746773]        dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs]
      [12537.775607]        cifs_reconnect+0xe04/0x2500 [cifs]
      [12537.798322]        cifs_readv_from_socket+0x461/0x690 [cifs]
      [12537.823369]        cifs_read_from_socket+0xa0/0xe0 [cifs]
      [12537.847262]        cifs_demultiplex_thread+0x311/0x2db0 [cifs]
      [12537.873477]        kthread+0x30c/0x3d0
      [12537.890281]        ret_from_fork+0x3a/0x50
      [12537.908649]
      [12537.908649] -> #1 (&tcp_ses->srv_mutex){+.+.}:
      [12537.935225]        __mutex_lock+0x105/0x11f0
      [12537.954450]        cifs_call_async+0x102/0x7f0 [cifs]
      [12537.977250]        smb2_async_readv+0x6c3/0xc90 [cifs]
      [12538.000659]        cifs_readpages+0x120a/0x1e50 [cifs]
      [12538.023920]        read_pages+0xf5/0x560
      [12538.041583]        __do_page_cache_readahead+0x41d/0x4b0
      [12538.067047]        ondemand_readahead+0x44c/0xc10
      [12538.092069]        filemap_fault+0xec1/0x1830
      [12538.111637]        __do_fault+0x82/0x260
      [12538.129216]        do_fault+0x419/0xfb0
      [12538.146390]        __handle_mm_fault+0x862/0xdf0
      [12538.167408]        handle_mm_fault+0x154/0x550
      [12538.187401]        __do_page_fault+0x42f/0xa60
      [12538.207395]        do_page_fault+0x38/0x5e0
      [12538.225777]        page_fault+0x1e/0x30
      [12538.243010]
      [12538.243010] -> #0 (&mm->mmap_sem){++++}:
      [12538.267875]        lock_acquire+0x14c/0x420
      [12538.286848]        __might_fault+0x119/0x1b0
      [12538.306006]        keyring_read_iterator+0x7e/0x170
      [12538.327936]        assoc_array_subtree_iterate+0x97/0x280
      [12538.352154]        keyring_read+0xe9/0x110
      [12538.370558]        keyctl_read_key+0x1b9/0x220
      [12538.391470]        do_syscall_64+0xa5/0x4b0
      [12538.410511]        entry_SYSCALL_64_after_hwframe+0x6a/0xdf
      [12538.435535]
      [12538.435535] other info that might help us debug this:
      [12538.435535]
      [12538.472829] Chain exists of:
      [12538.472829]   &mm->mmap_sem --> root_key_user.cons_lock --> &type->lock_class
      [12538.472829]
      [12538.524820]  Possible unsafe locking scenario:
      [12538.524820]
      [12538.551431]        CPU0                    CPU1
      [12538.572654]        ----                    ----
      [12538.595865]   lock(&type->lock_class);
      [12538.613737]                                lock(root_key_user.cons_lock);
      [12538.644234]                                lock(&type->lock_class);
      [12538.672410]   lock(&mm->mmap_sem);
      [12538.687758]
      [12538.687758]  *** DEADLOCK ***
      [12538.687758]
      [12538.714455] 1 lock held by keyctl/25598:
      [12538.732097]  #0: 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220
      [12538.770573]
      [12538.770573] stack backtrace:
      [12538.790136] CPU: 2 PID: 25598 Comm: keyctl Kdump: loaded Tainted: G
      [12538.844855] Hardware name: HP ProLiant DL360 Gen9/ProLiant DL360 Gen9, BIOS P89 12/27/2015
      [12538.881963] Call Trace:
      [12538.892897]  dump_stack+0x9a/0xf0
      [12538.907908]  print_circular_bug.isra.25.cold.50+0x1bc/0x279
      [12538.932891]  ? save_trace+0xd6/0x250
      [12538.948979]  check_prev_add.constprop.32+0xc36/0x14f0
      [12538.971643]  ? keyring_compare_object+0x104/0x190
      [12538.992738]  ? check_usage+0x550/0x550
      [12539.009845]  ? sched_clock+0x5/0x10
      [12539.025484]  ? sched_clock_cpu+0x18/0x1e0
      [12539.043555]  __lock_acquire+0x1f12/0x38d0
      [12539.061551]  ? trace_hardirqs_on+0x10/0x10
      [12539.080554]  lock_acquire+0x14c/0x420
      [12539.100330]  ? __might_fault+0xc4/0x1b0
      [12539.119079]  __might_fault+0x119/0x1b0
      [12539.135869]  ? __might_fault+0xc4/0x1b0
      [12539.153234]  keyring_read_iterator+0x7e/0x170
      [12539.172787]  ? keyring_read+0x110/0x110
      [12539.190059]  assoc_array_subtree_iterate+0x97/0x280
      [12539.211526]  keyring_read+0xe9/0x110
      [12539.227561]  ? keyring_gc_check_iterator+0xc0/0xc0
      [12539.249076]  keyctl_read_key+0x1b9/0x220
      [12539.266660]  do_syscall_64+0xa5/0x4b0
      [12539.283091]  entry_SYSCALL_64_after_hwframe+0x6a/0xdf
      
      One way to prevent this deadlock scenario from happening is to not
      allow writing to userspace while holding the key semaphore. Instead,
      an internal buffer is allocated for getting the keys out from the
      read method first before copying them out to userspace without holding
      the lock.
      
      That requires taking out the __user modifier from all the relevant
      read methods as well as additional changes to not use any userspace
      write helpers. That is,
      
        1) The put_user() call is replaced by a direct copy.
        2) The copy_to_user() call is replaced by memcpy().
        3) All the fault handling code is removed.
      
      Compiling on a x86-64 system, the size of the rxrpc_read() function is
      reduced from 3795 bytes to 2384 bytes with this patch.
      
      Fixes: ^1da177e4 ("Linux-2.6.12-rc2")
      Reviewed-by: default avatarJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
      Signed-off-by: default avatarWaiman Long <longman@redhat.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      d3ec10aa
  21. Mar 15, 2020
    • yang xu's avatar
      KEYS: reaching the keys quotas correctly · 2e356101
      yang xu authored
      
      Currently, when we add a new user key, the calltrace as below:
      
      add_key()
        key_create_or_update()
          key_alloc()
          __key_instantiate_and_link
            generic_key_instantiate
              key_payload_reserve
                ......
      
      Since commit a08bf91c ("KEYS: allow reaching the keys quotas exactly"),
      we can reach max bytes/keys in key_alloc, but we forget to remove this
      limit when we reserver space for payload in key_payload_reserve. So we
      can only reach max keys but not max bytes when having delta between plen
      and type->def_datalen. Remove this limit when instantiating the key, so we
      can keep consistent with key_alloc.
      
      Also, fix the similar problem in keyctl_chown_key().
      
      Fixes: 0b77f5bf ("keys: make the keyring quotas controllable through /proc/sys")
      Fixes: a08bf91c ("KEYS: allow reaching the keys quotas exactly")
      Cc: stable@vger.kernel.org # 5.0.x
      Cc: Eric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarYang Xu <xuyang2018.jy@cn.fujitsu.com>
      Reviewed-by: default avatarJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
      Reviewed-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
      2e356101
Loading