1. 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: default avatarEric Biggers <ebiggers@google.com>
      Cc: stable@kernel.org # 4.20
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      57b0e314
  2. 18 Dec, 2018 1 commit
    • Linus Torvalds's avatar
      security: don't use a negative Opt_err token index · 94c13f66
      Linus Torvalds authored
      The code uses a bitmap to check for duplicate tokens during parsing, and
      that doesn't work at all for the negative Opt_err token case.
      
      There is absolutely no reason to make Opt_err be negative, and in fact
      it only confuses things, since some of the affected functions actually
      return a positive Opt_xyz enum _or_ a regular negative error code (eg
      -EINVAL), and using -1 for Opt_err makes no sense.
      
      There are similar problems in ima_policy.c and key encryption, but they
      don't have the immediate bug wrt bitmap handing, and ima_policy.c in
      particular needs a different patch to make the enum values match the
      token array index.  Mimi is sending that separately.
      
      Reported-by: syzbot+a22e0dc07567662c50bc@syzkaller.appspotmail.com
      Reported-by: default avatarEric Biggers <ebiggers@kernel.org>
      Fixes: 5208cc83 ("keys, trusted: fix: *do not* allow duplicate key options")
      Fixes: 00d60fd3 ("KEYS: Provide keyctls to drive the new key type ops for asymmetric keys [ver #2]")
      Cc: James Morris James Morris <jmorris@namei.org>
      Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
      Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
      Cc: Peter Huewe <peterhuewe@gmx.de>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      94c13f66
  3. 14 Dec, 2018 2 commits
  4. 12 Dec, 2018 2 commits
    • 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
    • Paul Gortmaker's avatar
      keys: remove needless modular infrastructure from ecryptfs_format · a7986080
      Paul Gortmaker authored
      Even though the support can be modular, only one file needs to use
      all the macros like MODULE_AUTHOR, MODULE_LICENSE etc.  Only the one
      responsible for registering/removal with module_init/module_exit
      needs to declare these.  In this case, that file is "encrypted.c"
      and it already has the MODULE_LICENSE that we are removing here.
      
      Since the file does EXPORT_SYMBOL, we add export.h - and build tests
      show that module.h (which includes everything) was hiding an implicit
      use of string.h - so that is added as well.
      
      Cc: Mimi Zohar <zohar@linux.ibm.com>
      Cc: David Howells <dhowells@redhat.com>
      Cc: James Morris <jmorris@namei.org>
      Cc: "Serge E. Hallyn" <serge@hallyn.com>
      Cc: linux-integrity@vger.kernel.org
      Cc: keyrings@vger.kernel.org
      Cc: linux-security-module@vger.kernel.org
      Signed-off-by: default avatarPaul Gortmaker <paul.gortmaker@windriver.com>
      Signed-off-by: default avatarJames Morris <james.morris@microsoft.com>
      a7986080
  5. 20 Nov, 2018 1 commit
  6. 26 Oct, 2018 3 commits
    • Denis Kenzior's avatar
    • Denis Kenzior's avatar
      KEYS: trusted: Expose common functionality [ver #2] · e1ea9f86
      Denis Kenzior authored
      This patch exposes some common functionality needed to send TPM commands.
      Several functions from keys/trusted.c are exposed for use by the new tpm
      key subtype and a module dependency is introduced.
      
      In the future, common functionality between the trusted key type and the
      asym_tpm subtype should be factored out into a common utility library.
      Signed-off-by: default avatarDenis Kenzior <denkenz@gmail.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarMarcel Holtmann <marcel@holtmann.org>
      Reviewed-by: default avatarMarcel Holtmann <marcel@holtmann.org>
      Signed-off-by: default avatarJames Morris <james.morris@microsoft.com>
      e1ea9f86
    • David Howells's avatar
      KEYS: Provide keyctls to drive the new key type ops for asymmetric keys [ver #2] · 00d60fd3
      David Howells authored
      Provide five keyctl functions that permit userspace to make use of the new
      key type ops for accessing and driving asymmetric keys.
      
       (*) Query an asymmetric key.
      
      	long keyctl(KEYCTL_PKEY_QUERY,
      		    key_serial_t key, unsigned long reserved,
      		    struct keyctl_pkey_query *info);
      
           Get information about an asymmetric key.  The information is returned
           in the keyctl_pkey_query struct:
      
      	__u32	supported_ops;
      
           A bit mask of flags indicating which ops are supported.  This is
           constructed from a bitwise-OR of:
      
      	KEYCTL_SUPPORTS_{ENCRYPT,DECRYPT,SIGN,VERIFY}
      
      	__u32	key_size;
      
           The size in bits of the key.
      
      	__u16	max_data_size;
      	__u16	max_sig_size;
      	__u16	max_enc_size;
      	__u16	max_dec_size;
      
           The maximum sizes in bytes of a blob of data to be signed, a signature
           blob, a blob to be encrypted and a blob to be decrypted.
      
           reserved must be set to 0.  This is intended for future use to hand
           over one or more passphrases needed unlock a key.
      
           If successful, 0 is returned.  If the key is not an asymmetric key,
           EOPNOTSUPP is returned.
      
       (*) Encrypt, decrypt, sign or verify a blob using an asymmetric key.
      
      	long keyctl(KEYCTL_PKEY_ENCRYPT,
      		    const struct keyctl_pkey_params *params,
      		    const char *info,
      		    const void *in,
      		    void *out);
      
      	long keyctl(KEYCTL_PKEY_DECRYPT,
      		    const struct keyctl_pkey_params *params,
      		    const char *info,
      		    const void *in,
      		    void *out);
      
      	long keyctl(KEYCTL_PKEY_SIGN,
      		    const struct keyctl_pkey_params *params,
      		    const char *info,
      		    const void *in,
      		    void *out);
      
      	long keyctl(KEYCTL_PKEY_VERIFY,
      		    const struct keyctl_pkey_params *params,
      		    const char *info,
      		    const void *in,
      		    const void *in2);
      
           Use an asymmetric key to perform a public-key cryptographic operation
           a blob of data.
      
           The parameter block pointed to by params contains a number of integer
           values:
      
      	__s32		key_id;
      	__u32		in_len;
      	__u32		out_len;
      	__u32		in2_len;
      
           For a given operation, the in and out buffers are used as follows:
      
      	Operation ID		in,in_len	out,out_len	in2,in2_len
      	=======================	===============	===============	===========
      	KEYCTL_PKEY_ENCRYPT	Raw data	Encrypted data	-
      	KEYCTL_PKEY_DECRYPT	Encrypted data	Raw data	-
      	KEYCTL_PKEY_SIGN	Raw data	Signature	-
      	KEYCTL_PKEY_VERIFY	Raw data	-		Signature
      
           info is a string of key=value pairs that supply supplementary
           information.
      
           The __spare space in the parameter block must be set to 0.  This is
           intended, amongst other things, to allow the passing of passphrases
           required to unlock a key.
      
           If successful, encrypt, decrypt and sign all return the amount of data
           written into the output buffer.  Verification returns 0 on success.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarMarcel Holtmann <marcel@holtmann.org>
      Reviewed-by: default avatarMarcel Holtmann <marcel@holtmann.org>
      Reviewed-by: default avatarDenis Kenzior <denkenz@gmail.com>
      Tested-by: default avatarDenis Kenzior <denkenz@gmail.com>
      Signed-off-by: default avatarJames Morris <james.morris@microsoft.com>
      00d60fd3
  7. 25 Sep, 2018 1 commit
  8. 04 Sep, 2018 1 commit
  9. 08 Jul, 2018 1 commit
  10. 26 Jun, 2018 1 commit
    • Eric Biggers's avatar
      dh key: fix rounding up KDF output length · 3619dec5
      Eric Biggers authored
      Commit 383203ef ("dh key: get rid of stack allocated array") changed
      kdf_ctr() to assume that the length of key material to derive is a
      multiple of the digest size.  The length was supposed to be rounded up
      accordingly.  However, the round_up() macro was used which only gives
      the correct result on power-of-2 arguments, whereas not all hash
      algorithms have power-of-2 digest sizes.  In some cases this resulted in
      a write past the end of the 'outbuf' buffer.
      
      Fix it by switching to roundup(), which works for non-power-of-2 inputs.
      
      Reported-by: syzbot+486f97f892efeb2075a3@syzkaller.appspotmail.com
      Reported-by: syzbot+29d17b7898b41ee120a5@syzkaller.appspotmail.com
      Reported-by: syzbot+8a608baf8751184ec727@syzkaller.appspotmail.com
      Reported-by: syzbot+d04e58bd384f1fe0b112@syzkaller.appspotmail.com
      Fixes: 383203ef ("dh key: get rid of stack allocated array")
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Acked-by: default avatarKees Cook <keescook@chromium.org>
      Acked-by: default avatarTycho Andersen <tycho@tycho.ws>
      Signed-off-by: default avatarJames Morris <james.morris@microsoft.com>
      3619dec5
  11. 12 Jun, 2018 1 commit
    • Kees Cook's avatar
      treewide: kmalloc() -> kmalloc_array() · 6da2ec56
      Kees Cook authored
      The kmalloc() function has a 2-factor argument form, kmalloc_array(). This
      patch replaces cases of:
      
              kmalloc(a * b, gfp)
      
      with:
              kmalloc_array(a * b, gfp)
      
      as well as handling cases of:
      
              kmalloc(a * b * c, gfp)
      
      with:
      
              kmalloc(array3_size(a, b, c), gfp)
      
      as it's slightly less ugly than:
      
              kmalloc_array(array_size(a, b), c, gfp)
      
      This does, however, attempt to ignore constant size factors like:
      
              kmalloc(4 * 1024, gfp)
      
      though any constants defined via macros get caught up in the conversion.
      
      Any factors with a sizeof() of "unsigned char", "char", and "u8" were
      dropped, since they're redundant.
      
      The tools/ directory was manually excluded, since it has its own
      implementation of kmalloc().
      
      The Coccinelle script used for this was:
      
      // Fix redundant parens around sizeof().
      @@
      type TYPE;
      expression THING, E;
      @@
      
      (
        kmalloc(
      -	(sizeof(TYPE)) * E
      +	sizeof(TYPE) * E
        , ...)
      |
        kmalloc(
      -	(sizeof(THING)) * E
      +	sizeof(THING) * E
        , ...)
      )
      
      // Drop single-byte sizes and redundant parens.
      @@
      expression COUNT;
      typedef u8;
      typedef __u8;
      @@
      
      (
        kmalloc(
      -	sizeof(u8) * (COUNT)
      +	COUNT
        , ...)
      |
        kmalloc(
      -	sizeof(__u8) * (COUNT)
      +	COUNT
        , ...)
      |
        kmalloc(
      -	sizeof(char) * (COUNT)
      +	COUNT
        , ...)
      |
        kmalloc(
      -	sizeof(unsigned char) * (COUNT)
      +	COUNT
        , ...)
      |
        kmalloc(
      -	sizeof(u8) * COUNT
      +	COUNT
        , ...)
      |
        kmalloc(
      -	sizeof(__u8) * COUNT
      +	COUNT
        , ...)
      |
        kmalloc(
      -	sizeof(char) * COUNT
      +	COUNT
        , ...)
      |
        kmalloc(
      -	sizeof(unsigned char) * COUNT
      +	COUNT
        , ...)
      )
      
      // 2-factor product with sizeof(type/expression) and identifier or constant.
      @@
      type TYPE;
      expression THING;
      identifier COUNT_ID;
      constant COUNT_CONST;
      @@
      
      (
      - kmalloc
      + kmalloc_array
        (
      -	sizeof(TYPE) * (COUNT_ID)
      +	COUNT_ID, sizeof(TYPE)
        , ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	sizeof(TYPE) * COUNT_ID
      +	COUNT_ID, sizeof(TYPE)
        , ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	sizeof(TYPE) * (COUNT_CONST)
      +	COUNT_CONST, sizeof(TYPE)
        , ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	sizeof(TYPE) * COUNT_CONST
      +	COUNT_CONST, sizeof(TYPE)
        , ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	sizeof(THING) * (COUNT_ID)
      +	COUNT_ID, sizeof(THING)
        , ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	sizeof(THING) * COUNT_ID
      +	COUNT_ID, sizeof(THING)
        , ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	sizeof(THING) * (COUNT_CONST)
      +	COUNT_CONST, sizeof(THING)
        , ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	sizeof(THING) * COUNT_CONST
      +	COUNT_CONST, sizeof(THING)
        , ...)
      )
      
      // 2-factor product, only identifiers.
      @@
      identifier SIZE, COUNT;
      @@
      
      - kmalloc
      + kmalloc_array
        (
      -	SIZE * COUNT
      +	COUNT, SIZE
        , ...)
      
      // 3-factor product with 1 sizeof(type) or sizeof(expression), with
      // redundant parens removed.
      @@
      expression THING;
      identifier STRIDE, COUNT;
      type TYPE;
      @@
      
      (
        kmalloc(
      -	sizeof(TYPE) * (COUNT) * (STRIDE)
      +	array3_size(COUNT, STRIDE, sizeof(TYPE))
        , ...)
      |
        kmalloc(
      -	sizeof(TYPE) * (COUNT) * STRIDE
      +	array3_size(COUNT, STRIDE, sizeof(TYPE))
        , ...)
      |
        kmalloc(
      -	sizeof(TYPE) * COUNT * (STRIDE)
      +	array3_size(COUNT, STRIDE, sizeof(TYPE))
        , ...)
      |
        kmalloc(
      -	sizeof(TYPE) * COUNT * STRIDE
      +	array3_size(COUNT, STRIDE, sizeof(TYPE))
        , ...)
      |
        kmalloc(
      -	sizeof(THING) * (COUNT) * (STRIDE)
      +	array3_size(COUNT, STRIDE, sizeof(THING))
        , ...)
      |
        kmalloc(
      -	sizeof(THING) * (COUNT) * STRIDE
      +	array3_size(COUNT, STRIDE, sizeof(THING))
        , ...)
      |
        kmalloc(
      -	sizeof(THING) * COUNT * (STRIDE)
      +	array3_size(COUNT, STRIDE, sizeof(THING))
        , ...)
      |
        kmalloc(
      -	sizeof(THING) * COUNT * STRIDE
      +	array3_size(COUNT, STRIDE, sizeof(THING))
        , ...)
      )
      
      // 3-factor product with 2 sizeof(variable), with redundant parens removed.
      @@
      expression THING1, THING2;
      identifier COUNT;
      type TYPE1, TYPE2;
      @@
      
      (
        kmalloc(
      -	sizeof(TYPE1) * sizeof(TYPE2) * COUNT
      +	array3_size(COUNT, sizeof(TYPE1), sizeof(TYPE2))
        , ...)
      |
        kmalloc(
      -	sizeof(TYPE1) * sizeof(THING2) * (COUNT)
      +	array3_size(COUNT, sizeof(TYPE1), sizeof(TYPE2))
        , ...)
      |
        kmalloc(
      -	sizeof(THING1) * sizeof(THING2) * COUNT
      +	array3_size(COUNT, sizeof(THING1), sizeof(THING2))
        , ...)
      |
        kmalloc(
      -	sizeof(THING1) * sizeof(THING2) * (COUNT)
      +	array3_size(COUNT, sizeof(THING1), sizeof(THING2))
        , ...)
      |
        kmalloc(
      -	sizeof(TYPE1) * sizeof(THING2) * COUNT
      +	array3_size(COUNT, sizeof(TYPE1), sizeof(THING2))
        , ...)
      |
        kmalloc(
      -	sizeof(TYPE1) * sizeof(THING2) * (COUNT)
      +	array3_size(COUNT, sizeof(TYPE1), sizeof(THING2))
        , ...)
      )
      
      // 3-factor product, only identifiers, with redundant parens removed.
      @@
      identifier STRIDE, SIZE, COUNT;
      @@
      
      (
        kmalloc(
      -	(COUNT) * STRIDE * SIZE
      +	array3_size(COUNT, STRIDE, SIZE)
        , ...)
      |
        kmalloc(
      -	COUNT * (STRIDE) * SIZE
      +	array3_size(COUNT, STRIDE, SIZE)
        , ...)
      |
        kmalloc(
      -	COUNT * STRIDE * (SIZE)
      +	array3_size(COUNT, STRIDE, SIZE)
        , ...)
      |
        kmalloc(
      -	(COUNT) * (STRIDE) * SIZE
      +	array3_size(COUNT, STRIDE, SIZE)
        , ...)
      |
        kmalloc(
      -	COUNT * (STRIDE) * (SIZE)
      +	array3_size(COUNT, STRIDE, SIZE)
        , ...)
      |
        kmalloc(
      -	(COUNT) * STRIDE * (SIZE)
      +	array3_size(COUNT, STRIDE, SIZE)
        , ...)
      |
        kmalloc(
      -	(COUNT) * (STRIDE) * (SIZE)
      +	array3_size(COUNT, STRIDE, SIZE)
        , ...)
      |
        kmalloc(
      -	COUNT * STRIDE * SIZE
      +	array3_size(COUNT, STRIDE, SIZE)
        , ...)
      )
      
      // Any remaining multi-factor products, first at least 3-factor products,
      // when they're not all constants...
      @@
      expression E1, E2, E3;
      constant C1, C2, C3;
      @@
      
      (
        kmalloc(C1 * C2 * C3, ...)
      |
        kmalloc(
      -	(E1) * E2 * E3
      +	array3_size(E1, E2, E3)
        , ...)
      |
        kmalloc(
      -	(E1) * (E2) * E3
      +	array3_size(E1, E2, E3)
        , ...)
      |
        kmalloc(
      -	(E1) * (E2) * (E3)
      +	array3_size(E1, E2, E3)
        , ...)
      |
        kmalloc(
      -	E1 * E2 * E3
      +	array3_size(E1, E2, E3)
        , ...)
      )
      
      // And then all remaining 2 factors products when they're not all constants,
      // keeping sizeof() as the second factor argument.
      @@
      expression THING, E1, E2;
      type TYPE;
      constant C1, C2, C3;
      @@
      
      (
        kmalloc(sizeof(THING) * C2, ...)
      |
        kmalloc(sizeof(TYPE) * C2, ...)
      |
        kmalloc(C1 * C2 * C3, ...)
      |
        kmalloc(C1 * C2, ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	sizeof(TYPE) * (E2)
      +	E2, sizeof(TYPE)
        , ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	sizeof(TYPE) * E2
      +	E2, sizeof(TYPE)
        , ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	sizeof(THING) * (E2)
      +	E2, sizeof(THING)
        , ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	sizeof(THING) * E2
      +	E2, sizeof(THING)
        , ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	(E1) * E2
      +	E1, E2
        , ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	(E1) * (E2)
      +	E1, E2
        , ...)
      |
      - kmalloc
      + kmalloc_array
        (
      -	E1 * E2
      +	E1, E2
        , ...)
      )
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      6da2ec56
  12. 16 May, 2018 1 commit
  13. 11 May, 2018 3 commits
    • Tycho Andersen's avatar
      dh key: get rid of stack allocated array for zeroes · 890e2abe
      Tycho Andersen authored
      We're interested in getting rid of all of the stack allocated arrays in
      the kernel: https://lkml.org/lkml/2018/3/7/621
      
      This case is interesting, since we really just need an array of bytes that
      are zero. The loop already ensures that if the array isn't exactly the
      right size that enough zero bytes will be copied in. So, instead of
      choosing this value to be the size of the hash, let's just choose it to be
      32, since that is a common size, is not too big, and will not result in too
      many extra iterations of the loop.
      
      v2: split out from other patch, just hardcode array size instead of
          dynamically allocating something the right size
      v3: fix typo of 256 -> 32
      Signed-off-by: default avatarTycho Andersen <tycho@tycho.ws>
      Reviewed-by: default avatarKees Cook <keescook@chromium.org>
      CC: David Howells <dhowells@redhat.com>
      CC: James Morris <jmorris@namei.org>
      CC: "Serge E. Hallyn" <serge@hallyn.com>
      CC: Eric Biggers <ebiggers3@gmail.com>
      Signed-off-by: default avatarJames Morris <james.morris@microsoft.com>
      890e2abe
    • Tycho Andersen's avatar
      dh key: get rid of stack allocated array · 383203ef
      Tycho Andersen authored
      We're interested in getting rid of all of the stack allocated arrays in the
      kernel: https://lkml.org/lkml/2018/3/7/621
      
      This particular vla is used as a temporary output buffer in case there is
      too much hash output for the destination buffer. Instead, let's just
      allocate a buffer that's big enough initially, but only copy back to
      userspace the amount that was originally asked for.
      
      v2: allocate enough in the original output buffer vs creating a temporary
          output buffer
      Signed-off-by: default avatarTycho Andersen <tycho@tycho.ws>
      Reviewed-by: default avatarKees Cook <keescook@chromium.org>
      CC: David Howells <dhowells@redhat.com>
      CC: James Morris <jmorris@namei.org>
      CC: "Serge E. Hallyn" <serge@hallyn.com>
      CC: Eric Biggers <ebiggers3@gmail.com>
      Signed-off-by: default avatarJames Morris <james.morris@microsoft.com>
      383203ef
    • Tycho Andersen's avatar
      big key: get rid of stack array allocation · a964f395
      Tycho Andersen authored
      We're interested in getting rid of all of the stack allocated arrays in the
      kernel [1]. This patch simply hardcodes the iv length to match that of the
      hardcoded cipher.
      
      [1]: https://lkml.org/lkml/2018/3/7/621
      
      v2: hardcode the length of the nonce to be the GCM AES IV length, and do a
          sanity check in init(), Eric Biggers
      v3: * remember to free big_key_aead when sanity check fails
          * define a constant for big key IV size so it can be changed along side
            the algorithm in the code
      Signed-off-by: default avatarTycho Andersen <tycho@tycho.ws>
      Reviewed-by: default avatarKees Cook <keescook@chromium.org>
      CC: David Howells <dhowells@redhat.com>
      CC: James Morris <jmorris@namei.org>
      CC: "Serge E. Hallyn" <serge@hallyn.com>
      CC: Jason A. Donenfeld <Jason@zx2c4.com>
      CC: Eric Biggers <ebiggers3@gmail.com>
      Signed-off-by: default avatarJames Morris <james.morris@microsoft.com>
      a964f395
  14. 06 Apr, 2018 1 commit
    • Randy Dunlap's avatar
      headers: untangle kmemleak.h from mm.h · 514c6032
      Randy Dunlap authored
      Currently <linux/slab.h> #includes <linux/kmemleak.h> for no obvious
      reason.  It looks like it's only a convenience, so remove kmemleak.h
      from slab.h and add <linux/kmemleak.h> to any users of kmemleak_* that
      don't already #include it.  Also remove <linux/kmemleak.h> from source
      files that do not use it.
      
      This is tested on i386 allmodconfig and x86_64 allmodconfig.  It would
      be good to run it through the 0day bot for other $ARCHes.  I have
      neither the horsepower nor the storage space for the other $ARCHes.
      
      Update: This patch has been extensively build-tested by both the 0day
      bot & kisskb/ozlabs build farms.  Both of them reported 2 build failures
      for which patches are included here (in v2).
      
      [ slab.h is the second most used header file after module.h; kernel.h is
        right there with slab.h. There could be some minor error in the
        counting due to some #includes having comments after them and I didn't
        combine all of those. ]
      
      [akpm@linux-foundation.org: security/keys/big_key.c needs vmalloc.h, per sfr]
      Link: http://lkml.kernel.org/r/e4309f98-3749-93e1-4bb7-d9501a39d015@infradead.org
      Link: http://kisskb.ellerman.id.au/kisskb/head/13396/Signed-off-by: default avatarRandy Dunlap <rdunlap@infradead.org>
      Reviewed-by: default avatarIngo Molnar <mingo@kernel.org>
      Reported-by: Michael Ellerman <mpe@ellerman.id.au>	[2 build failures]
      Reported-by: Fengguang Wu <fengguang.wu@intel.com>	[2 build failures]
      Reviewed-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Cc: Wei Yongjun <weiyongjun1@huawei.com>
      Cc: Luis R. Rodriguez <mcgrof@kernel.org>
      Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
      Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
      Cc: John Johansen <john.johansen@canonical.com>
      Cc: Stephen Rothwell <sfr@canb.auug.org.au>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      514c6032
  15. 22 Feb, 2018 1 commit
    • David Howells's avatar
      KEYS: Use individual pages in big_key for crypto buffers · d9f4bb1a
      David Howells authored
      kmalloc() can't always allocate large enough buffers for big_key to use for
      crypto (1MB + some metadata) so we cannot use that to allocate the buffer.
      Further, vmalloc'd pages can't be passed to sg_init_one() and the aead
      crypto accessors cannot be called progressively and must be passed all the
      data in one go (which means we can't pass the data in one block at a time).
      
      Fix this by allocating the buffer pages individually and passing them
      through a multientry scatterlist to the crypto layer.  This has the bonus
      advantage that we don't have to allocate a contiguous series of pages.
      
      We then vmap() the page list and pass that through to the VFS read/write
      routines.
      
      This can trigger a warning:
      
      	WARNING: CPU: 0 PID: 60912 at mm/page_alloc.c:3883 __alloc_pages_nodemask+0xb7c/0x15f8
      	([<00000000002acbb6>] __alloc_pages_nodemask+0x1ee/0x15f8)
      	 [<00000000002dd356>] kmalloc_order+0x46/0x90
      	 [<00000000002dd3e0>] kmalloc_order_trace+0x40/0x1f8
      	 [<0000000000326a10>] __kmalloc+0x430/0x4c0
      	 [<00000000004343e4>] big_key_preparse+0x7c/0x210
      	 [<000000000042c040>] key_create_or_update+0x128/0x420
      	 [<000000000042e52c>] SyS_add_key+0x124/0x220
      	 [<00000000007bba2c>] system_call+0xc4/0x2b0
      
      from the keyctl/padd/useradd test of the keyutils testsuite on s390x.
      
      Note that it might be better to shovel data through in page-sized lumps
      instead as there's no particular need to use a monolithic buffer unless the
      kernel itself wants to access the data.
      
      Fixes: 13100a72 ("Security: Keys: Big keys stored encrypted")
      Reported-by: default avatarPaul Bunyan <pbunyan@redhat.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      cc: Kirill Marinushkin <k.marinushkin@gmail.com>
      d9f4bb1a
  16. 08 Jan, 2018 1 commit
  17. 08 Dec, 2017 4 commits
    • Eric Biggers's avatar
      KEYS: reject NULL restriction string when type is specified · 18026d86
      Eric Biggers authored
      keyctl_restrict_keyring() allows through a NULL restriction when the
      "type" is non-NULL, which causes a NULL pointer dereference in
      asymmetric_lookup_restriction() when it calls strcmp() on the
      restriction string.
      
      But no key types actually use a "NULL restriction" to mean anything, so
      update keyctl_restrict_keyring() to reject it with EINVAL.
      Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
      Fixes: 97d3aa0f ("KEYS: Add a lookup_restriction function for the asymmetric key type")
      Cc: <stable@vger.kernel.org> # v4.12+
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      18026d86
    • Colin Ian King's avatar
      security: keys: remove redundant assignment to key_ref · 3d1f0255
      Colin Ian King authored
      Variable key_ref is being assigned a value that is never read;
      key_ref is being re-assigned a few statements later.  Hence this
      assignment is redundant and can be removed.
      Signed-off-by: default avatarColin Ian King <colin.king@canonical.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarJames Morris <james.l.morris@oracle.com>
      3d1f0255
    • Eric Biggers's avatar
      KEYS: add missing permission check for request_key() destination · 4dca6ea1
      Eric Biggers authored
      When the request_key() syscall is not passed a destination keyring, it
      links the requested key (if constructed) into the "default" request-key
      keyring.  This should require Write permission to the keyring.  However,
      there is actually no permission check.
      
      This can be abused to add keys to any keyring to which only Search
      permission is granted.  This is because Search permission allows joining
      the keyring.  keyctl_set_reqkey_keyring(KEY_REQKEY_DEFL_SESSION_KEYRING)
      then will set the default request-key keyring to the session keyring.
      Then, request_key() can be used to add keys to the keyring.
      
      Both negatively and positively instantiated keys can be added using this
      method.  Adding negative keys is trivial.  Adding a positive key is a
      bit trickier.  It requires that either /sbin/request-key positively
      instantiates the key, or that another thread adds the key to the process
      keyring at just the right time, such that request_key() misses it
      initially but then finds it in construct_alloc_key().
      
      Fix this bug by checking for Write permission to the keyring in
      construct_get_dest_keyring() when the default keyring is being used.
      
      We don't do the permission check for non-default keyrings because that
      was already done by the earlier call to lookup_user_key().  Also,
      request_key_and_link() is currently passed a 'struct key *' rather than
      a key_ref_t, so the "possessed" bit is unavailable.
      
      We also don't do the permission check for the "requestor keyring", to
      continue to support the use case described by commit 8bbf4976
      ("KEYS: Alter use of key instantiation link-to-keyring argument") where
      /sbin/request-key recursively calls request_key() to add keys to the
      original requestor's destination keyring.  (I don't know of any users
      who actually do that, though...)
      
      Fixes: 3e30148c ("[PATCH] Keys: Make request-key create an authorisation key")
      Cc: <stable@vger.kernel.org>	# v2.6.13+
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      4dca6ea1
    • Eric Biggers's avatar
      KEYS: remove unnecessary get/put of explicit dest_keyring · a2d8737d
      Eric Biggers authored
      In request_key_and_link(), in the case where the dest_keyring was
      explicitly specified, there is no need to get another reference to
      dest_keyring before calling key_link(), then drop it afterwards.  This
      is because by definition, we already have a reference to dest_keyring.
      
      This change is useful because we'll be making
      construct_get_dest_keyring() able to return an error code, and we don't
      want to have to handle that error here for no reason.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      a2d8737d
  18. 04 Dec, 2017 1 commit
  19. 21 Nov, 2017 1 commit
    • Kees Cook's avatar
      treewide: Switch DEFINE_TIMER callbacks to struct timer_list * · 24ed960a
      Kees Cook authored
      This changes all DEFINE_TIMER() callbacks to use a struct timer_list
      pointer instead of unsigned long. Since the data argument has already been
      removed, none of these callbacks are using their argument currently, so
      this renames the argument to "unused".
      
      Done using the following semantic patch:
      
      @match_define_timer@
      declarer name DEFINE_TIMER;
      identifier _timer, _callback;
      @@
      
       DEFINE_TIMER(_timer, _callback);
      
      @change_callback depends on match_define_timer@
      identifier match_define_timer._callback;
      type _origtype;
      identifier _origarg;
      @@
      
       void
      -_callback(_origtype _origarg)
      +_callback(struct timer_list *unused)
       { ... }
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      24ed960a
  20. 15 Nov, 2017 2 commits
  21. 02 Nov, 2017 3 commits
    • Greg Kroah-Hartman's avatar
      License cleanup: add SPDX GPL-2.0 license identifier to files with no license · b2441318
      Greg Kroah-Hartman authored
      Many source files in the tree are missing licensing information, which
      makes it harder for compliance tools to determine the correct license.
      
      By default all files without license information are under the default
      license of the kernel, which is GPL version 2.
      
      Update the files which contain no license information with the 'GPL-2.0'
      SPDX license identifier.  The SPDX identifier is a legally binding
      shorthand, which can be used instead of the full boiler plate text.
      
      This patch is based on work done by Thomas Gleixner and Kate Stewart and
      Philippe Ombredanne.
      
      How this work was done:
      
      Patches were generated and checked against linux-4.14-rc6 for a subset of
      the use cases:
       - file had no licensing information it it.
       - file was a */uapi/* one with no licensing information in it,
       - file was a */uapi/* one with existing licensing information,
      
      Further patches will be generated in subsequent months to fix up cases
      where non-standard license headers were used, and references to license
      had to be inferred by heuristics based on keywords.
      
      The analysis to determine which SPDX License Identifier to be applied to
      a file was done in a spreadsheet of side by side results from of the
      output of two independent scanners (ScanCode & Windriver) producing SPDX
      tag:value files created by Philippe Ombredanne.  Philippe prepared the
      base worksheet, and did an initial spot review of a few 1000 files.
      
      The 4.13 kernel was the starting point of the analysis with 60,537 files
      assessed.  Kate Stewart did a file by file comparison of the scanner
      results in the spreadsheet to determine which SPDX license identifier(s)
      to be applied to the file. She confirmed any determination that was not
      immediately clear with lawyers working with the Linux Foundation.
      
      Criteria used to select files for SPDX license identifier tagging was:
       - Files considered eligible had to be source code files.
       - Make and config files were included as candidates if they contained >5
         lines of source
       - File already had some variant of a license header in it (even if <5
         lines).
      
      All documentation files were explicitly excluded.
      
      The following heuristics were used to determine which SPDX license
      identifiers to apply.
      
       - when both scanners couldn't find any license traces, file was
         considered to have no license information in it, and the top level
         COPYING file license applied.
      
         For non */uapi/* files that summary was:
      
         SPDX license identifier                            # files
         ---------------------------------------------------|-------
         GPL-2.0                                              11139
      
         and resulted in the first patch in this series.
      
         If that file was a */uapi/* path one, it was "GPL-2.0 WITH
         Linux-syscall-note" otherwise it was "GPL-2.0".  Results of that was:
      
         SPDX license identifier                            # files
         ---------------------------------------------------|-------
         GPL-2.0 WITH Linux-syscall-note                        930
      
         and resulted in the second patch in this series.
      
       - if a file had some form of licensing information in it, and was one
         of the */uapi/* ones, it was denoted with the Linux-syscall-note if
         any GPL family license was found in the file or had no licensing in
         it (per prior point).  Results summary:
      
         SPDX license identifier                            # files
         ---------------------------------------------------|------
         GPL-2.0 WITH Linux-syscall-note                       270
         GPL-2.0+ WITH Linux-syscall-note                      169
         ((GPL-2.0 WITH Linux-syscall-note) OR BSD-2-Clause)    21
         ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)    17
         LGPL-2.1+ WITH Linux-syscall-note                      15
         GPL-1.0+ WITH Linux-syscall-note                       14
         ((GPL-2.0+ WITH Linux-syscall-note) OR BSD-3-Clause)    5
         LGPL-2.0+ WITH Linux-syscall-note                       4
         LGPL-2.1 WITH Linux-syscall-note                        3
         ((GPL-2.0 WITH Linux-syscall-note) OR MIT)              3
         ((GPL-2.0 WITH Linux-syscall-note) AND MIT)             1
      
         and that resulted in the third patch in this series.
      
       - when the two scanners agreed on the detected license(s), that became
         the concluded license(s).
      
       - when there was disagreement between the two scanners (one detected a
         license but the other didn't, or they both detected different
         licenses) a manual inspection of the file occurred.
      
       - In most cases a manual inspection of the information in the file
         resulted in a clear resolution of the license that should apply (and
         which scanner probably needed to revisit its heuristics).
      
       - When it was not immediately clear, the license identifier was
         confirmed with lawyers working with the Linux Foundation.
      
       - If there was any question as to the appropriate license identifier,
         the file was flagged for further research and to be revisited later
         in time.
      
      In total, over 70 hours of logged manual review was done on the
      spreadsheet to determine the SPDX license identifiers to apply to the
      source files by Kate, Philippe, Thomas and, in some cases, confirmation
      by lawyers working with the Linux Foundation.
      
      Kate also obtained a third independent scan of the 4.13 code base from
      FOSSology, and compared selected files where the other two scanners
      disagreed against that SPDX file, to see if there was new insights.  The
      Windriver scanner is based on an older version of FOSSology in part, so
      they are related.
      
      Thomas did random spot checks in about 500 files from the spreadsheets
      for the uapi headers and agreed with SPDX license identifier in the
      files he inspected. For the non-uapi files Thomas did random spot checks
      in about 15000 files.
      
      In initial set of patches against 4.14-rc6, 3 files were found to have
      copy/paste license identifier errors, and have been fixed to reflect the
      correct identifier.
      
      Additionally Philippe spent 10 hours this week doing a detailed manual
      inspection and review of the 12,461 patched files from the initial patch
      version early this week with:
       - a full scancode scan run, collecting the matched texts, detected
         license ids and scores
       - reviewing anything where there was a license detected (about 500+
         files) to ensure that the applied SPDX license was correct
       - reviewing anything where there was no detection but the patch license
         was not GPL-2.0 WITH Linux-syscall-note to ensure that the applied
         SPDX license was correct
      
      This produced a worksheet with 20 files needing minor correction.  This
      worksheet was then exported into 3 different .csv files for the
      different types of files to be modified.
      
      These .csv files were then reviewed by Greg.  Thomas wrote a script to
      parse the csv files and add the proper SPDX tag to the file, in the
      format that the file expected.  This script was further refined by Greg
      based on the output to detect more types of files automatically and to
      distinguish between header and source .c files (which need different
      comment types.)  Finally Greg ran the script using the .csv files to
      generate the patches.
      Reviewed-by: default avatarKate Stewart <kstewart@linuxfoundation.org>
      Reviewed-by: default avatarPhilippe Ombredanne <pombredanne@nexb.com>
      Reviewed-by: default avatarThomas Gleixner <tglx@linutronix.de>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      b2441318
    • Eric Biggers's avatar
      KEYS: trusted: fix writing past end of buffer in trusted_read() · a3c812f7
      Eric Biggers authored
      When calling keyctl_read() on a key of type "trusted", if the
      user-supplied buffer was too small, the kernel ignored the buffer length
      and just wrote past the end of the buffer, potentially corrupting
      userspace memory.  Fix it by instead returning the size required, as per
      the documentation for keyctl_read().
      
      We also don't even fill the buffer at all in this case, as this is
      slightly easier to implement than doing a short read, and either
      behavior appears to be permitted.  It also makes it match the behavior
      of the "encrypted" key type.
      
      Fixes: d00a1c72 ("keys: add new trusted key-type")
      Reported-by: default avatarBen Hutchings <ben@decadent.org.uk>
      Cc: <stable@vger.kernel.org> # v2.6.38+
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarMimi Zohar <zohar@linux.vnet.ibm.com>
      Reviewed-by: default avatarJames Morris <james.l.morris@oracle.com>
      Signed-off-by: default avatarJames Morris <james.l.morris@oracle.com>
      a3c812f7
    • Eric Biggers's avatar
      KEYS: return full count in keyring_read() if buffer is too small · 3239b6f2
      Eric Biggers authored
      Commit e645016a ("KEYS: fix writing past end of user-supplied buffer
      in keyring_read()") made keyring_read() stop corrupting userspace memory
      when the user-supplied buffer is too small.  However it also made the
      return value in that case be the short buffer size rather than the size
      required, yet keyctl_read() is actually documented to return the size
      required.  Therefore, switch it over to the documented behavior.
      
      Note that for now we continue to have it fill the short buffer, since it
      did that before (pre-v3.13) and dump_key_tree_aux() in keyutils arguably
      relies on it.
      
      Fixes: e645016a ("KEYS: fix writing past end of user-supplied buffer in keyring_read()")
      Reported-by: default avatarBen Hutchings <ben@decadent.org.uk>
      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>
      Reviewed-by: default avatarJames Morris <james.l.morris@oracle.com>
      Signed-off-by: default avatarJames Morris <james.l.morris@oracle.com>
      3239b6f2
  22. 18 Oct, 2017 6 commits
    • Eric Biggers's avatar
      KEYS: load key flags and expiry time atomically in proc_keys_show() · ab5c69f0
      Eric Biggers authored
      In proc_keys_show(), the key semaphore is not held, so the key ->flags
      and ->expiry can be changed concurrently.  We therefore should read them
      atomically just once.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      ab5c69f0
    • 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
    • Eric Biggers's avatar
      KEYS: load key flags and expiry time atomically in key_validate() · 1823d475
      Eric Biggers authored
      In key_validate(), load the flags and expiry time once atomically, since
      these can change concurrently if key_validate() is called without the
      key semaphore held.  And we don't want to get inconsistent results if a
      variable is referenced multiple times.  For example, key->expiry was
      referenced in both 'if (key->expiry)' and in 'if (now.tv_sec >=
      key->expiry)', making it theoretically possible to see a spurious
      EKEYEXPIRED while the expiration time was being removed, i.e. set to 0.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      1823d475
    • David Howells's avatar
      KEYS: don't let add_key() update an uninstantiated key · 60ff5b2f
      David Howells authored
      Currently, when passed a key that already exists, add_key() will call the
      key's ->update() method if such exists.  But this is heavily broken in the
      case where the key is uninstantiated because it doesn't call
      __key_instantiate_and_link().  Consequently, it doesn't do most of the
      things that are supposed to happen when the key is instantiated, such as
      setting the instantiation state, clearing KEY_FLAG_USER_CONSTRUCT and
      awakening tasks waiting on it, and incrementing key->user->nikeys.
      
      It also never takes key_construction_mutex, which means that
      ->instantiate() can run concurrently with ->update() on the same key.  In
      the case of the "user" and "logon" key types this causes a memory leak, at
      best.  Maybe even worse, the ->update() methods of the "encrypted" and
      "trusted" key types actually just dereference a NULL pointer when passed an
      uninstantiated key.
      
      Change key_create_or_update() to wait interruptibly for the key to finish
      construction before continuing.
      
      This patch only affects *uninstantiated* keys.  For now we still allow a
      negatively instantiated key to be updated (thereby positively
      instantiating it), although that's broken too (the next patch fixes it)
      and I'm not sure that anyone actually uses that functionality either.
      
      Here is a simple reproducer for the bug using the "encrypted" key type
      (requires CONFIG_ENCRYPTED_KEYS=y), though as noted above the bug
      pertained to more than just the "encrypted" key type:
      
          #include <stdlib.h>
          #include <unistd.h>
          #include <keyutils.h>
      
          int main(void)
          {
              int ringid = keyctl_join_session_keyring(NULL);
      
              if (fork()) {
                  for (;;) {
                      const char payload[] = "update user:foo 32";
      
                      usleep(rand() % 10000);
                      add_key("encrypted", "desc", payload, sizeof(payload), ringid);
                      keyctl_clear(ringid);
                  }
              } else {
                  for (;;)
                      request_key("encrypted", "desc", "callout_info", ringid);
              }
          }
      
      It causes:
      
          BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
          IP: encrypted_update+0xb0/0x170
          PGD 7a178067 P4D 7a178067 PUD 77269067 PMD 0
          PREEMPT SMP
          CPU: 0 PID: 340 Comm: reproduce Tainted: G      D         4.14.0-rc1-00025-g428490e3 #796
          Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
          task: ffff8a467a39a340 task.stack: ffffb15c40770000
          RIP: 0010:encrypted_update+0xb0/0x170
          RSP: 0018:ffffb15c40773de8 EFLAGS: 00010246
          RAX: 0000000000000000 RBX: ffff8a467a275b00 RCX: 0000000000000000
          RDX: 0000000000000005 RSI: ffff8a467a275b14 RDI: ffffffffb742f303
          RBP: ffffb15c40773e20 R08: 0000000000000000 R09: ffff8a467a275b17
          R10: 0000000000000020 R11: 0000000000000000 R12: 0000000000000000
          R13: 0000000000000000 R14: ffff8a4677057180 R15: ffff8a467a275b0f
          FS:  00007f5d7fb08700(0000) GS:ffff8a467f200000(0000) knlGS:0000000000000000
          CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
          CR2: 0000000000000018 CR3: 0000000077262005 CR4: 00000000001606f0
          Call Trace:
           key_create_or_update+0x2bc/0x460
           SyS_add_key+0x10c/0x1d0
           entry_SYSCALL_64_fastpath+0x1f/0xbe
          RIP: 0033:0x7f5d7f211259
          RSP: 002b:00007ffed03904c8 EFLAGS: 00000246 ORIG_RAX: 00000000000000f8
          RAX: ffffffffffffffda RBX: 000000003b2a7955 RCX: 00007f5d7f211259
          RDX: 00000000004009e4 RSI: 00000000004009ff RDI: 0000000000400a04
          RBP: 0000000068db8bad R08: 000000003b2a7955 R09: 0000000000000004
          R10: 000000000000001a R11: 0000000000000246 R12: 0000000000400868
          R13: 00007ffed03905d0 R14: 0000000000000000 R15: 0000000000000000
          Code: 77 28 e8 64 34 1f 00 45 31 c0 31 c9 48 8d 55 c8 48 89 df 48 8d 75 d0 e8 ff f9 ff ff 85 c0 41 89 c4 0f 88 84 00 00 00 4c 8b 7d c8 <49> 8b 75 18 4c 89 ff e8 24 f8 ff ff 85 c0 41 89 c4 78 6d 49 8b
          RIP: encrypted_update+0xb0/0x170 RSP: ffffb15c40773de8
          CR2: 0000000000000018
      
      Cc: <stable@vger.kernel.org> # v2.6.12+
      Reported-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      cc: Eric Biggers <ebiggers@google.com>
      60ff5b2f
    • 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
    • Arnd Bergmann's avatar
      security/keys: BIG_KEY requires CONFIG_CRYPTO · 3cd18d19
      Arnd Bergmann authored
      The recent rework introduced a possible randconfig build failure
      when CONFIG_CRYPTO configured to only allow modules:
      
      security/keys/big_key.o: In function `big_key_crypt':
      big_key.c:(.text+0x29f): undefined reference to `crypto_aead_setkey'
      security/keys/big_key.o: In function `big_key_init':
      big_key.c:(.init.text+0x1a): undefined reference to `crypto_alloc_aead'
      big_key.c:(.init.text+0x45): undefined reference to `crypto_aead_setauthsize'
      big_key.c:(.init.text+0x77): undefined reference to `crypto_destroy_tfm'
      crypto/gcm.o: In function `gcm_hash_crypt_remain_continue':
      gcm.c:(.text+0x167): undefined reference to `crypto_ahash_finup'
      crypto/gcm.o: In function `crypto_gcm_exit_tfm':
      gcm.c:(.text+0x847): undefined reference to `crypto_destroy_tfm'
      
      When we 'select CRYPTO' like the other users, we always get a
      configuration that builds.
      
      Fixes: 428490e3 ("security/keys: rewrite all of big_key crypto")
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      3cd18d19
  23. 12 Oct, 2017 1 commit
    • Eric Biggers's avatar
      KEYS: encrypted: fix dereference of NULL user_key_payload · 13923d08
      Eric Biggers authored
      A key of type "encrypted" references a "master key" which is used to
      encrypt and decrypt the encrypted key's payload.  However, when we
      accessed the master key's payload, we failed to handle the case where
      the master key has been revoked, which sets the payload pointer to NULL.
      Note that request_key() *does* skip revoked keys, but there is still a
      window where the key can be revoked before we acquire its semaphore.
      
      Fix it by checking for a NULL payload, treating it like a key which was
      already revoked at the time it was requested.
      
      This was an issue for master keys of type "user" only.  Master keys can
      also be of type "trusted", but those cannot be revoked.
      
      Fixes: 7e70cb49 ("keys: add new key-type encrypted")
      Reviewed-by: default avatarJames Morris <james.l.morris@oracle.com>
      Cc: <stable@vger.kernel.org>    [v2.6.38+]
      Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
      Cc: David Safford <safford@us.ibm.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      13923d08