diff mbox series

[2/3] crypto: api - adjust meaning of CRYPTO_ALG_ALLOCATES_MEMORY

Message ID 20230705164009.58351-3-giovanni.cabiddu@intel.com
State New
Headers show
Series crypto: adjust meaning of CRYPTO_ALG_ALLOCATES_MEMORY | expand

Commit Message

Cabiddu, Giovanni July 5, 2023, 4:40 p.m. UTC 
The CRYPTO_ALG_ALLOCATES_MEMORY flag doesn't allow to distinguish
between implementations which don't allocate memory for scatterlists
with 4 or less entries (the typical case for dm-crypt) and those that
do.
The flag's meaning is adjusted based on the ML discussion below.

This patch removes the need to set the flag if the implementation can
handle scatterlists up to 4 entries without allocating memory.
The documentation is updated accordingly, with an extra clarification
regarding sleeping.

Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
Suggested-by: Eric Biggers <ebiggers@google.com>
Link: https://lore.kernel.org/linux-crypto/20200722072932.GA27544@gondor.apana.org.au/
Link: https://lore.kernel.org/linux-crypto/20230523165503.GA864814@google.com/
Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Reviewed-by: Fiona Trahe <fiona.trahe@intel.com>
---
 include/linux/crypto.h | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Mikulas Patocka July 7, 2023, 10:41 a.m. UTC | #1 
On Wed, 5 Jul 2023, Eric Biggers wrote:

> On Wed, Jul 05, 2023 at 05:40:08PM +0100, Giovanni Cabiddu wrote:
> 
> > Algorithms that do not set this flag will guarantee
> 
> "will guarantee" => "guarantee"
>  
> > that memory is not allocated during request processing, except in
> > the avoidable exception cases described below.
> 
> "avoidable exception cases" => "exception cases"
> 
> Whether they are avoidable depends on the user.
> 
> > * Users can request an algorithm with this flag unset if they can't handle
> > * memory allocation failures or sleeping during request processing.
> 
> Why add the "sleeping during request processing" part?  Isn't that controlled on
> a per-request basis by CRYPTO_TFM_REQ_MAY_SLEEP which is a separate thing?
> 
> > * They should also follow the constraints below.
> 
> "should" => "must"
> 
> > + *	- The input and output scatterlists must have no more than 4 entries.
> > + *	  If the scatterlists contain more than 4 entries, the algorithm may
> > + *	  allocate memory.
> 
> "If the scatterlists contains" => "If either scatterlist contains"
> 
> Otherwise it is unclear whether this is talking about the length of each
> scatterlist individually, or the sum of their lengths.
> 
> - Eric

Hi

I wouldn't change the meaning of CRYPTO_ALG_ALLOCATES_MEMORY (because 
people will forget about this subtle change anyway).

Also note that dm-integrity allocates arbitrarily large sg-lists when 
encrypting the journal, so if you change the meaning of 
CRYPTO_ALG_ALLOCATES_MEMORY, there would be no flag left for dm-integrity 
to test.

I would introduce a new flag, something like 
CRYPTO_ALG_ALLOCATES_MEMORY_FOR_5_OR_MORE_SG_ENTRIES. dm-crypt can then 
filter the algorithms based on this flag - and the rest of the kernel code 
may stay unchanged.

Mikulas
diff mbox series

Patch

diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 31f6fee0c36c..15884790a3d0 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -87,8 +87,13 @@ 
 
 /*
  * The algorithm may allocate memory during request processing, i.e. during
- * encryption, decryption, or hashing.  Users can request an algorithm with this
- * flag unset if they can't handle memory allocation failures.
+ * encryption, decryption, or hashing. Algorithms that do not set this flag will
+ * guarantee that memory is not allocated during request processing, except in
+ * the avoidable exception cases described below.
+ *
+ * Users can request an algorithm with this flag unset if they can't handle
+ * memory allocation failures or sleeping during request processing. They should
+ * also follow the constraints below.
  *
  * This flag is currently only implemented for algorithms of type "skcipher",
  * "aead", "ahash", "shash", and "cipher".  Algorithms of other types might not
@@ -102,6 +107,9 @@ 
  *	- If the data were to be divided into chunks of size
  *	  crypto_skcipher_walksize() (with any remainder going at the end), no
  *	  chunk can cross a page boundary or a scatterlist element boundary.
+ *	- The input and output scatterlists must have no more than 4 entries.
+ *	  If the scatterlists contain more than 4 entries, the algorithm may
+ *	  allocate memory.
  *    aead:
  *	- The IV buffer and all scatterlist elements must be aligned to the
  *	  algorithm's alignmask.
@@ -110,10 +118,16 @@ 
  *	- If the plaintext/ciphertext were to be divided into chunks of size
  *	  crypto_aead_walksize() (with the remainder going at the end), no chunk
  *	  can cross a page boundary or a scatterlist element boundary.
+ *	- The input and output scatterlists must have no more than 4 entries.
+ *	  If the scatterlists contain more than 4 entries, the algorithm may
+ *	  allocate memory.
  *    ahash:
  *	- The result buffer must be aligned to the algorithm's alignmask.
  *	- crypto_ahash_finup() must not be used unless the algorithm implements
  *	  ->finup() natively.
+ *	- The input and output scatterlists must have no more than 4 entries.
+ *	  If the scatterlists contain more than 4 entries, the algorithm may
+ *	  allocate memory.
  */
 #define CRYPTO_ALG_ALLOCATES_MEMORY	0x00010000