Re: [dm-devel] [REGRESSION] dm_crypt essiv ciphers do not use async driver mv-aes-cbc anymore

Re: [dm-devel] [REGRESSION] dm_crypt essiv ciphers do not use async driver mv-aes-cbc anymore

[Eric Biggers]

Hi Yureka,

On Fri, Sep 29, 2023 at 11:08:55PM +0200, Yureka wrote:
> #regzbot introduced: 7bcb2c99f8ed
> 
> I am running the NixOS distribution cross-compiled from x86_64 to a Marvell
> Armada 388 armv7 SoC.
> 
> I am not getting expected speeds when reading/writing on my encrypted hard
> drive with 6.5.5, while it is fast on 5.4.257. Volume is formatted like this:
> `cryptsetup luksFormat -c aes-cbc-essiv:sha256 /dev/sda`.
> 
> Specifically, I tracked this down to the changes to crypto/essiv.c from
> 7bcb2c99f8ed mentioned above. Reverting those changes on top of a 6.5.5 kernel
> provides working (see applicable diff further below).
> 
> I'm *guessing* that this is related to the mv-aes-cbc crypto driver (from the
> marvell-cesa module) being registered as async (according to /proc/crypto),
> and I *suspect* that async drivers are not being used anymore by essiv or
> dm_crypt. Going by the commit description, which sounds more like a refactor,
> this does not seem intentional.

This is actually from commit b8aa7dc5c753 ("crypto: drivers - set the flag
CRYPTO_ALG_ALLOCATES_MEMORY"), which set CRYPTO_ALG_ALLOCATES_MEMORY in
marvell-cesa.  7bcb2c99f8ed is just one of the prerequisite commits.

I understand that the dm-crypt developers did this as an intentional bug fix in
order to prevent dm-crypt from using crypto drivers that are known to cause
deadlocks due to allocating memory during requests.

If you are interested in still being able to use marvell-cesa with dm-crypt, I
believe it would need to be fixed to meet the requirements for not needing
CRYPTO_ALG_ALLOCATES_MEMORY.  I've Cc'ed the maintainers of that driver.

#regzbot introduced: b8aa7dc5c753

- Eric

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [REGRESSION] dm_crypt essiv ciphers do not use async driver mv-aes-cbc anymore

[Linux regression tracking (Thorsten Leemhuis)]

n 30.09.23 00:43, Eric Biggers wrote:
> On Fri, Sep 29, 2023 at 11:08:55PM +0200, Yureka wrote:
>>
>> I am running the NixOS distribution cross-compiled from x86_64 to a Marvell
>> Armada 388 armv7 SoC.
>>
>> I am not getting expected speeds when reading/writing on my encrypted hard
>> drive with 6.5.5, while it is fast on 5.4.257. Volume is formatted like this:
>> `cryptsetup luksFormat -c aes-cbc-essiv:sha256 /dev/sda`.
>>
>> Specifically, I tracked this down to the changes to crypto/essiv.c from
>> 7bcb2c99f8ed mentioned above. Reverting those changes on top of a 6.5.5 kernel
>> provides working (see applicable diff further below).
>>
>> I'm *guessing* that this is related to the mv-aes-cbc crypto driver (from the
>> marvell-cesa module) being registered as async (according to /proc/crypto),
>> and I *suspect* that async drivers are not being used anymore by essiv or
>> dm_crypt. Going by the commit description, which sounds more like a refactor,
>> this does not seem intentional.
> 
> This is actually from commit b8aa7dc5c753 ("crypto: drivers - set the flag
> CRYPTO_ALG_ALLOCATES_MEMORY"), which set CRYPTO_ALG_ALLOCATES_MEMORY in
> marvell-cesa.  7bcb2c99f8ed is just one of the prerequisite commits.
> 
> I understand that the dm-crypt developers did this as an intentional bug fix in
> order to prevent dm-crypt from using crypto drivers that are known to cause
> deadlocks due to allocating memory during requests.
> 
> If you are interested in still being able to use marvell-cesa with dm-crypt, I
> believe it would need to be fixed to meet the requirements for not needing
> CRYPTO_ALG_ALLOCATES_MEMORY.  I've Cc'ed the maintainers of that driver.
> 
> #regzbot introduced: b8aa7dc5c753

BTW: Eric, thx for this.

Boris, Arnaud, Srujana, and Mikulas, could you maybe comment on this? I
understand that this is not some everyday regression due to deadlock
risk, but it nevertheless would be good to get this resolved somehow to
stay in line with our "no regressions" rule.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

Re: [REGRESSION] dm_crypt essiv ciphers do not use async driver mv-aes-cbc anymore

[Mikulas Patocka]

On Wed, 1 Nov 2023, Linux regression tracking (Thorsten Leemhuis) wrote:

> > #regzbot introduced: b8aa7dc5c753
> 
> BTW: Eric, thx for this.
> 
> Boris, Arnaud, Srujana, and Mikulas, could you maybe comment on this? I
> understand that this is not some everyday regression due to deadlock
> risk, but it nevertheless would be good to get this resolved somehow to
> stay in line with our "no regressions" rule.
> 
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

Hi

The driver drivers/crypto/marvell/cesa/cipher.c uses GFP_ATOMIC 
allocations (see mv_cesa_skcipher_dma_req_init). So, it is not really safe 
to use it for dm-crypt.

GFP_ATOMIC allocations may fail anytime (for example, they fill fail if 
the machine receives too many network packets in a short timeframe and 
runs temporarily out of memory). And when the GFP_ATOMIC allocation fails, 
you get a write I/O error and data corruption.

It could be possible to change it to use GFP_NOIO allocations, then we 
would risk deadlock instead of data corruption. The best thing would be to 
convert the driver to use mempools.

Mikulas