| Message ID | 20230705164009.58351-3-giovanni.cabiddu@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: adjust meaning of CRYPTO_ALG_ALLOCATES_MEMORY | expand |
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
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 --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