1. 09 Jan, 2020 23 commits
    • Eric Biggers's avatar
      crypto: cipher - introduce crypto_cipher_spawn and crypto_grab_cipher() · 0764ac28
      Eric Biggers authored
      Currently, "cipher" (single-block cipher) spawns are usually initialized
      by using crypto_get_attr_alg() to look up the algorithm, then calling
      crypto_init_spawn().  In one case, crypto_grab_spawn() is used directly.
      
      The former way is different from how skcipher, aead, and akcipher spawns
      are initialized (they use crypto_grab_*()), and for no good reason.
      This difference introduces unnecessary complexity.
      
      The crypto_grab_*() functions used to have some problems, like not
      holding a reference to the algorithm and requiring the caller to
      initialize spawn->base.inst.  But those problems are fixed now.
      
      Also, the cipher spawns are not strongly typed; e.g., the API requires
      that the user manually specify the flags CRYPTO_ALG_TYPE_CIPHER and
      CRYPTO_ALG_TYPE_MASK.  Though the "cipher" algorithm type itself isn't
      yet strongly typed, we can start by making the spawns strongly typed.
      
      So, let's introduce a new 'struct crypto_cipher_spawn', and functions
      crypto_grab_cipher() and crypto_drop_cipher() to grab and drop them.
      
      Later patches will convert all cipher spawns to use these, then make
      crypto_spawn_cipher() take 'struct crypto_cipher_spawn' as well, instead
      of a bare 'struct crypto_spawn' as it currently does.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      0764ac28
    • Eric Biggers's avatar
      crypto: ahash - introduce crypto_grab_ahash() · 84a9c938
      Eric Biggers authored
      Currently, ahash spawns are initialized by using ahash_attr_alg() or
      crypto_find_alg() to look up the ahash algorithm, then calling
      crypto_init_ahash_spawn().
      
      This is different from how skcipher, aead, and akcipher spawns are
      initialized (they use crypto_grab_*()), and for no good reason.  This
      difference introduces unnecessary complexity.
      
      The crypto_grab_*() functions used to have some problems, like not
      holding a reference to the algorithm and requiring the caller to
      initialize spawn->base.inst.  But those problems are fixed now.
      
      So, let's introduce crypto_grab_ahash() so that we can convert all
      templates to the same way of initializing their spawns.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      84a9c938
    • Eric Biggers's avatar
      crypto: shash - introduce crypto_grab_shash() · fdfad1ff
      Eric Biggers authored
      Currently, shash spawns are initialized by using shash_attr_alg() or
      crypto_alg_mod_lookup() to look up the shash algorithm, then calling
      crypto_init_shash_spawn().
      
      This is different from how skcipher, aead, and akcipher spawns are
      initialized (they use crypto_grab_*()), and for no good reason.  This
      difference introduces unnecessary complexity.
      
      The crypto_grab_*() functions used to have some problems, like not
      holding a reference to the algorithm and requiring the caller to
      initialize spawn->base.inst.  But those problems are fixed now.
      
      So, let's introduce crypto_grab_shash() so that we can convert all
      templates to the same way of initializing their spawns.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      fdfad1ff
    • Eric Biggers's avatar
      crypto: algapi - pass instance to crypto_grab_spawn() · de95c957
      Eric Biggers authored
      Currently, crypto_spawn::inst is first used temporarily to pass the
      instance to crypto_grab_spawn().  Then crypto_init_spawn() overwrites it
      with crypto_spawn::next, which shares the same union.  Finally,
      crypto_spawn::inst is set again when the instance is registered.
      
      Make this less convoluted by just passing the instance as an argument to
      crypto_grab_spawn() instead.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      de95c957
    • Eric Biggers's avatar
      crypto: akcipher - pass instance to crypto_grab_akcipher() · 73bed26f
      Eric Biggers authored
      Initializing a crypto_akcipher_spawn currently requires:
      
      1. Set spawn->base.inst to point to the instance.
      2. Call crypto_grab_akcipher().
      
      But there's no reason for these steps to be separate, and in fact this
      unneeded complication has caused at least one bug, the one fixed by
      commit 6db43410 ("crypto: adiantum - initialize crypto_spawn::inst")
      
      So just make crypto_grab_akcipher() take the instance as an argument.
      
      To keep the function call from getting too unwieldy due to this extra
      argument, also introduce a 'mask' variable into pkcs1pad_create().
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      73bed26f
    • Eric Biggers's avatar
      crypto: aead - pass instance to crypto_grab_aead() · cd900f0c
      Eric Biggers authored
      Initializing a crypto_aead_spawn currently requires:
      
      1. Set spawn->base.inst to point to the instance.
      2. Call crypto_grab_aead().
      
      But there's no reason for these steps to be separate, and in fact this
      unneeded complication has caused at least one bug, the one fixed by
      commit 6db43410 ("crypto: adiantum - initialize crypto_spawn::inst")
      
      So just make crypto_grab_aead() take the instance as an argument.
      
      To keep the function calls from getting too unwieldy due to this extra
      argument, also introduce a 'mask' variable into the affected places
      which weren't already using one.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      cd900f0c
    • Eric Biggers's avatar
      crypto: skcipher - pass instance to crypto_grab_skcipher() · b9f76ddd
      Eric Biggers authored
      Initializing a crypto_skcipher_spawn currently requires:
      
      1. Set spawn->base.inst to point to the instance.
      2. Call crypto_grab_skcipher().
      
      But there's no reason for these steps to be separate, and in fact this
      unneeded complication has caused at least one bug, the one fixed by
      commit 6db43410 ("crypto: adiantum - initialize crypto_spawn::inst")
      
      So just make crypto_grab_skcipher() take the instance as an argument.
      
      To keep the function calls from getting too unwieldy due to this extra
      argument, also introduce a 'mask' variable into the affected places
      which weren't already using one.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      b9f76ddd
    • Eric Biggers's avatar
      crypto: ahash - make struct ahash_instance be the full size · 77f7e94d
      Eric Biggers authored
      Define struct ahash_instance in a way analogous to struct
      skcipher_instance, struct aead_instance, and struct akcipher_instance,
      where the struct is defined to include both the algorithm structure at
      the beginning and the additional crypto_instance fields at the end.
      
      This is needed to allow allocating ahash instances directly using
      kzalloc(sizeof(*inst) + sizeof(*ictx), ...) in the same way as skcipher,
      aead, and akcipher instances.  In turn, that's needed to make spawns be
      initialized in a consistent way everywhere.
      
      Also take advantage of the addition of the base instance to struct
      ahash_instance by simplifying the ahash_crypto_instance() and
      ahash_instance() functions.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      77f7e94d
    • Eric Biggers's avatar
      crypto: shash - make struct shash_instance be the full size · 1b84e7d0
      Eric Biggers authored
      Define struct shash_instance in a way analogous to struct
      skcipher_instance, struct aead_instance, and struct akcipher_instance,
      where the struct is defined to include both the algorithm structure at
      the beginning and the additional crypto_instance fields at the end.
      
      This is needed to allow allocating shash instances directly using
      kzalloc(sizeof(*inst) + sizeof(*ictx), ...) in the same way as skcipher,
      aead, and akcipher instances.  In turn, that's needed to make spawns be
      initialized in a consistent way everywhere.
      
      Also take advantage of the addition of the base instance to struct
      shash_instance by simplifying the shash_crypto_instance() and
      shash_instance() functions.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      1b84e7d0
    • Eric Biggers's avatar
      crypto: algapi - make crypto_grab_spawn() handle an ERR_PTR() name · ca94e937
      Eric Biggers authored
      To allow further simplifying template ->create() functions, make
      crypto_grab_spawn() handle an ERR_PTR() name by passing back the error.
      
      For most templates, this will allow the result of crypto_attr_alg_name()
      to be passed directly to crypto_grab_*(), rather than first having to
      assign it to a variable [where it can then potentially be misused, as it
      was in the rfc7539 template prior to commit 5e27f38f ("crypto:
      chacha20poly1305 - set cra_name correctly")] and check it for error.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      ca94e937
    • Eric Biggers's avatar
      crypto: algapi - make crypto_drop_spawn() a no-op on uninitialized spawns · ff670627
      Eric Biggers authored
      Make crypto_drop_spawn() do nothing when the spawn hasn't been
      initialized with an algorithm yet.  This will allow simplifying error
      handling in all the template ->create() functions, since on error they
      will be able to just call their usual "free instance" function, rather
      than having to handle dropping just the spawns that have been
      initialized so far.
      
      This does assume the spawn starts out zero-filled, but that's always the
      case since instances are allocated with kzalloc().  And some other code
      already assumes this anyway.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      ff670627
    • Gary R Hook's avatar
      crypto: ccp - Update MAINTAINERS for CCP driver · e91e785e
      Gary R Hook authored
      Remove Gary R Hook as CCP maintainer.
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      e91e785e
    • Christian Lamparter's avatar
      crypto: crypto4xx - use GFP_KERNEL for big allocations · 30a50e44
      Christian Lamparter authored
      The driver should use GFP_KERNEL for the bigger allocation
      during the driver's crypto4xx_probe() and not GFP_ATOMIC in
      my opinion.
      Signed-off-by: default avatarChristian Lamparter <chunkeey@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      30a50e44
    • Christian Lamparter's avatar
      crypto: crypto4xx - reduce memory fragmentation · b87b2c4d
      Christian Lamparter authored
      With recent kernels (>5.2), the driver fails to probe, as the
      allocation of the driver's scatter buffer fails with -ENOMEM.
      
      This happens in crypto4xx_build_sdr(). Where the driver tries
      to get 512KiB (=PPC4XX_SD_BUFFER_SIZE * PPC4XX_NUM_SD) of
      continuous memory. This big chunk is by design, since the driver
      uses this circumstance in the crypto4xx_copy_pkt_to_dst() to
      its advantage:
      "all scatter-buffers are all neatly organized in one big
      continuous ringbuffer; So scatterwalk_map_and_copy() can be
      instructed to copy a range of buffers in one go."
      
      The PowerPC arch does not have support for DMA_CMA. Hence,
      this patch reorganizes the order in which the memory
      allocations are done. Since the driver itself is responsible
      for some of the issues.
      Signed-off-by: default avatarChristian Lamparter <chunkeey@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      b87b2c4d
    • Eric Biggers's avatar
      crypto: remove propagation of CRYPTO_TFM_RES_* flags · af5034e8
      Eric Biggers authored
      The CRYPTO_TFM_RES_* flags were apparently meant as a way to make the
      ->setkey() functions provide more information about errors.  But these
      flags weren't actually being used or tested, and in many cases they
      weren't being set correctly anyway.  So they've now been removed.
      
      Also, if someone ever actually needs to start better distinguishing
      ->setkey() errors (which is somewhat unlikely, as this has been unneeded
      for a long time), we'd be much better off just defining different return
      values, like -EINVAL if the key is invalid for the algorithm vs.
      -EKEYREJECTED if the key was rejected by a policy like "no weak keys".
      That would be much simpler, less error-prone, and easier to test.
      
      So just remove CRYPTO_TFM_RES_MASK and all the unneeded logic that
      propagates these flags around.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      af5034e8
    • Eric Biggers's avatar
      crypto: remove CRYPTO_TFM_RES_WEAK_KEY · c4c4db0d
      Eric Biggers authored
      The CRYPTO_TFM_RES_WEAK_KEY flag was apparently meant as a way to make
      the ->setkey() functions provide more information about errors.
      
      However, no one actually checks for this flag, which makes it pointless.
      There are also no tests that verify that all algorithms actually set (or
      don't set) it correctly.
      
      This is also the last remaining CRYPTO_TFM_RES_* flag, which means that
      it's the only thing still needing all the boilerplate code which
      propagates these flags around from child => parent tfms.
      
      And if someone ever needs to distinguish this error in the future (which
      is somewhat unlikely, as it's been unneeded for a long time), it would
      be much better to just define a new return value like -EKEYREJECTED.
      That would be much simpler, less error-prone, and easier to test.
      
      So just remove this flag.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      c4c4db0d
    • Eric Biggers's avatar
      crypto: remove CRYPTO_TFM_RES_BAD_KEY_LEN · 674f368a
      Eric Biggers authored
      The CRYPTO_TFM_RES_BAD_KEY_LEN flag was apparently meant as a way to
      make the ->setkey() functions provide more information about errors.
      
      However, no one actually checks for this flag, which makes it pointless.
      
      Also, many algorithms fail to set this flag when given a bad length key.
      Reviewing just the generic implementations, this is the case for
      aes-fixed-time, cbcmac, echainiv, nhpoly1305, pcrypt, rfc3686, rfc4309,
      rfc7539, rfc7539esp, salsa20, seqiv, and xcbc.  But there are probably
      many more in arch/*/crypto/ and drivers/crypto/.
      
      Some algorithms can even set this flag when the key is the correct
      length.  For example, authenc and authencesn set it when the key payload
      is malformed in any way (not just a bad length), the atmel-sha and ccree
      drivers can set it if a memory allocation fails, and the chelsio driver
      sets it for bad auth tag lengths, not just bad key lengths.
      
      So even if someone actually wanted to start checking this flag (which
      seems unlikely, since it's been unused for a long time), there would be
      a lot of work needed to get it working correctly.  But it would probably
      be much better to go back to the drawing board and just define different
      return values, like -EINVAL if the key is invalid for the algorithm vs.
      -EKEYREJECTED if the key was rejected by a policy like "no weak keys".
      That would be much simpler, less error-prone, and easier to test.
      
      So just remove this flag.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Reviewed-by: default avatarHoria Geantă <horia.geanta@nxp.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      674f368a
    • Eric Biggers's avatar
      crypto: remove CRYPTO_TFM_RES_BAD_BLOCK_LEN · 5c925e8b
      Eric Biggers authored
      The flag CRYPTO_TFM_RES_BAD_BLOCK_LEN is never checked for, and it's
      only set by one driver.  And even that single driver's use is wrong
      because the driver is setting the flag from ->encrypt() and ->decrypt()
      with no locking, which is unsafe because ->encrypt() and ->decrypt() can
      be executed by many threads in parallel on the same tfm.
      
      Just remove this flag.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      5c925e8b
    • Eric Biggers's avatar
      crypto: remove unused tfm result flags · f9d89b85
      Eric Biggers authored
      The tfm result flags CRYPTO_TFM_RES_BAD_KEY_SCHED and
      CRYPTO_TFM_RES_BAD_FLAGS are never used, so remove them.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      f9d89b85
    • Eric Biggers's avatar
      crypto: atmel-sha - fix error handling when setting hmac key · b529f198
      Eric Biggers authored
      HMAC keys can be of any length, and atmel_sha_hmac_key_set() can only
      fail due to -ENOMEM.  But atmel_sha_hmac_setkey() incorrectly treated
      any error as a "bad key length" error.  Fix it to correctly propagate
      the -ENOMEM error code and not set any tfm result flags.
      
      Fixes: 81d8750b ("crypto: atmel-sha - add support to hmac(shaX)")
      Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
      Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
      Cc: Ludovic Desroches <ludovic.desroches@microchip.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Reviewed-by: default avatarTudor Ambarus <tudor.ambarus@microchip.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      b529f198
    • Eric Biggers's avatar
      crypto: artpec6 - return correct error code for failed setkey() · b828f905
      Eric Biggers authored
      ->setkey() is supposed to retun -EINVAL for invalid key lengths, not -1.
      
      Fixes: a21eb94f ("crypto: axis - add ARTPEC-6/7 crypto accelerator driver")
      Cc: Jesper Nilsson <jesper.nilsson@axis.com>
      Cc: Lars Persson <lars.persson@axis.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Acked-by: default avatarLars Persson <lars.persson@axis.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      b828f905
    • Eric Biggers's avatar
      crypto: chelsio - fix writing tfm flags to wrong place · bd56cea0
      Eric Biggers authored
      The chelsio crypto driver is casting 'struct crypto_aead' directly to
      'struct crypto_tfm', which is incorrect because the crypto_tfm isn't the
      first field of 'struct crypto_aead'.  Consequently, the calls to
      crypto_tfm_set_flags() are modifying some other field in the struct.
      
      Also, the driver is setting CRYPTO_TFM_RES_BAD_KEY_LEN in
      ->setauthsize(), not just in ->setkey().  This is incorrect since this
      flag is for bad key lengths, not for bad authentication tag lengths.
      
      Fix these bugs by removing the broken crypto_tfm_set_flags() calls from
      ->setauthsize() and by fixing them in ->setkey().
      
      Fixes: 324429d7 ("chcr: Support for Chelsio's Crypto Hardware")
      Cc: <stable@vger.kernel.org> # v4.9+
      Cc: Atul Gupta <atul.gupta@chelsio.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      bd56cea0
    • Eric Biggers's avatar
      crypto: skcipher - remove skcipher_walk_aead() · 70ffa8fd
      Eric Biggers authored
      skcipher_walk_aead() is unused and is identical to
      skcipher_walk_aead_encrypt(), so remove it.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      70ffa8fd
  2. 04 Jan, 2020 4 commits
  3. 27 Dec, 2019 13 commits