linux-kernel - Re: [dm-devel] [PATCH 2/3] crypto: api - adjust meaning of CRYPTO_ALG_ALLOCATES

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 7 Jul 2023 12:41:19 +0200 (CEST)
From:   Mikulas Patocka <mpatocka@...hat.com>
To:     Eric Biggers <ebiggers@...nel.org>
cc:     Giovanni Cabiddu <giovanni.cabiddu@...el.com>,
        Fiona Trahe <fiona.trahe@...el.com>,
        herbert@...dor.apana.org.au, horia.geanta@....com,
        pankaj.gupta@....com, gaurav.jain@....com, heinzm@...hat.com,
        snitzer@...nel.org, linux-kernel@...r.kernel.org,
        qat-linux@...el.com, iuliana.prodan@....com, dm-devel@...hat.com,
        meenakshi.aggarwal@....com, linux-crypto@...r.kernel.org,
        davem@...emloft.net, agk@...hat.com, V.Sethi@....com
Subject: Re: [dm-devel] [PATCH 2/3] crypto: api - adjust meaning of
 CRYPTO_ALG_ALLOCATES_MEMORY



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