On 7/13/2020 7:01 PM, Eric Biggers wrote: > On Mon, Jul 13, 2020 at 06:49:00PM +0300, Horia Geantă wrote: >> On 7/1/2020 7:52 AM, Eric Biggers wrote: >>> From: Mikulas Patocka <mpatocka@xxxxxxxxxx> >>> >>> Set the flag CRYPTO_ALG_ALLOCATES_MEMORY in the crypto drivers that >>> allocate memory. >>> >> Quite a few drivers are impacted. >> >> I wonder what's the proper way to address the memory allocation. >> >> Herbert mentioned setting up reqsize: >> https://lore.kernel.org/linux-crypto/20200610010450.GA6449@xxxxxxxxxxxxxxxxxxx/ >> >> I see at least two hurdles in converting the drivers to using reqsize: >> >> 1. Some drivers allocate the memory using GFP_DMA >> >> reqsize does not allow drivers to control gfp allocation flags. >> >> I've tried converting talitos driver (to use reqsize) at some point, >> and in the process adding a generic CRYPTO_TFM_REQ_DMA flag: >> https://lore.kernel.org/linux-crypto/54FD8D3B.5040409@xxxxxxxxxxxxx >> https://lore.kernel.org/linux-crypto/1426266882-31626-1-git-send-email-horia.geanta@xxxxxxxxxxxxx >> >> The flag was supposed to be transparent for the user, >> however there were users that open-coded the request allocation, >> for example esp_alloc_tmp() in net/ipv4/esp4.c. >> At that time, Dave NACK-ed the change: >> https://lore.kernel.org/linux-crypto/1426266922-31679-1-git-send-email-horia.geanta@xxxxxxxxxxxxx >> >> >> 2. Memory requirements cannot be determined / are not known >> at request allocation time >> >> An analysis for talitos driver is here: >> https://lore.kernel.org/linux-crypto/54F8235B.5080301@xxxxxxxxxxxxx >> >> In general, drivers would be forced to ask more memory than needed, >> to handle the "worst-case". >> Logic will be needed to fail in case the "worst-case" isn't correctly estimated. >> >> However, this is still problematic. >> >> For example, a driver could set up reqsize to accommodate for 32 S/G entries >> (in the HW S/G table). In case a dm-crypt encryption request would require more, >> then driver's .encrypt callback would fail, possibly with -ENOMEM, >> since there's not enough pre-allocated memory. >> This brings us back to the same problem we're trying to solve, >> since in this case the driver would be forced to either fail immediately or >> to allocate memory at .encrypt/.decrypt time. >> > > We have to place restrictions on what cases > !(flags & CRYPTO_ALG_ALLOCATES_MEMORY) applies to anyway; see the patch that > introduces it. If needed we could add more restrictions, like limit the number > of scatterlist elements. If we did that, the driver could allocate memory if > the number of scatterlist elements is large, without having to set > CRYPTO_ALG_ALLOCATES_MEMORY. > This sounds reasonable. > Also, have you considered using a mempool? A mempool allows allocations without > a possibility of failure, at the cost of pre-allocations. > Thanks for the suggestion. Would this be safe for all cases, e.g. IPsec - where .encrypt/.decrypt callbacks execute in (soft)IRQ context? kernel-doc for mempool_alloc() mentions it could fail when called from "IRQ context". Thanks, Horia
