dm-verity requires WQ_UNBOUND with mxs-dcp

dm-verity requires WQ_UNBOUND with mxs-dcp

[Sven]

Hi,

I'm seeing a problem using dm-verity with the sha256-dcp driver 
(mxs-dcp). Specifically, I'm seeing corruption errors a la

device-mapper: verity: 179:2: metadata block 76823 is corrupted

This is a regression that has started to appear with commit

c25da5b7baf1d dm verity: stop using WQ_UNBOUND for verify_wq

Reverting that commit while keeping an identical rootfs fixes my issue 
and I don't get any warnings anymore. I'm using an i.MX6ULL SoC which 
only has one CPU core so I don't really understand how WQ_UNBOUND can 
have an affect here. However, there might be side effects that I'm not 
aware of.

It might be worth noting that I'm using the dm-verity device as my root 
device without initramfs. I'm seeing this on both v6.6.23 and today's 
master (v6.10-rc1-13-g2bfcfd584ff5c).

Does anyone have ideas, how WQ_UNBOUND could be required here?

Thanks,
Sven

Re: dm-verity requires WQ_UNBOUND with mxs-dcp

[Eric Biggers]

On Tue, May 28, 2024 at 10:58:33AM +0200, Sven wrote:
> Hi,
> 
> I'm seeing a problem using dm-verity with the sha256-dcp driver (mxs-dcp).
> Specifically, I'm seeing corruption errors a la
> 
> device-mapper: verity: 179:2: metadata block 76823 is corrupted
> 
> This is a regression that has started to appear with commit
> 
> c25da5b7baf1d dm verity: stop using WQ_UNBOUND for verify_wq
> 
> Reverting that commit while keeping an identical rootfs fixes my issue and I
> don't get any warnings anymore. I'm using an i.MX6ULL SoC which only has one
> CPU core so I don't really understand how WQ_UNBOUND can have an affect
> here. However, there might be side effects that I'm not aware of.
> 
> It might be worth noting that I'm using the dm-verity device as my root
> device without initramfs. I'm seeing this on both v6.6.23 and today's master
> (v6.10-rc1-13-g2bfcfd584ff5c).
> 
> Does anyone have ideas, how WQ_UNBOUND could be required here?
> 

Does this happen with software SHA-256, or is it specific to the sha256-dcp
driver?  Does this driver pass all the crypto self-tests?

- Eric

Re: dm-verity requires WQ_UNBOUND with mxs-dcp

[Sven]

Hi Eric,

> Does this happen with software SHA-256, or is it specific to the sha256-dcp
> driver?

This is specific to the sha256-dcp driver. It works without issue with 
the software sha256 implementation.

> Does this driver pass all the crypto self-tests?

Not sure. I don't have experience with those tests. I'll do some digging.

Best regards,
Sven

Re: dm-verity requires WQ_UNBOUND with mxs-dcp

[Sven]

Do you happen to have some more infos on the self-tests you mentioned? 
Can you point me at documentation, I coulnd't really find much useful stuff.

Thanks

On 5/28/24 9:42 PM, Eric Biggers wrote:
> Does this driver pass all the crypto self-tests?

Re: dm-verity requires WQ_UNBOUND with mxs-dcp

[Eric Biggers]

On Thu, Jun 06, 2024 at 03:12:26PM +0200, Sven wrote:
> Do you happen to have some more infos on the self-tests you mentioned? Can
> you point me at documentation, I coulnd't really find much useful stuff.
> 
> Thanks
> 
> On 5/28/24 9:42 PM, Eric Biggers wrote:
> > Does this driver pass all the crypto self-tests?

The crypto self-tests can be enabled by unsetting
CONFIG_CRYPTO_MANAGER_DISABLE_TESTS, and preferably setting
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS in order to enable the full set of tests.

They might catch something, though they don't test parallel usage of drivers
which is where it sounds like the issue here might be here.

In any case the bug here is likely to be in the mxs-dcp driver, especially
considering that drivers tend to be much lower quality than the software crypto
(some driver developers apparently don't even bother to run the tests).

- Eric

Re: dm-verity requires WQ_UNBOUND with mxs-dcp

[Sven]

The driver complains during self-testing, however, those errors seem to 
be thrown during aes testing not sha256 testing:

# dmesg | grep -iE 'crypt|alg:'
[    0.316442] alg: extra crypto tests enabled.  This is intended for 
developer use only.
[    1.055440] mxs-dcp 2280000.crypto: Invalid block size!
[    1.060927] mxs-dcp 2280000.crypto: Invalid block size!
[    1.164247] mxs-dcp 2280000.crypto: Invalid block size!
[    1.169591] mxs-dcp 2280000.crypto: Invalid block size!
[    1.204738] mxs-dcp 2280000.crypto: Invalid block size!
[    1.210093] mxs-dcp 2280000.crypto: Invalid block size!
[    1.371876] mxs-dcp 2280000.crypto: Invalid block size!
[    1.377280] mxs-dcp 2280000.crypto: Invalid block size!
[    1.394093] mxs-dcp 2280000.crypto: Invalid block size!
[    1.399432] mxs-dcp 2280000.crypto: Invalid block size!
[    1.431509] mxs-dcp 2280000.crypto: Invalid block size!
[    1.436904] mxs-dcp 2280000.crypto: Invalid block size!

The sha256-dcp self-test is reported as "passed".

I have tried to reproduce the issue by forcing many concurrent sha256 
operations from userspace using AF_ALG but I haven't been able to see 
any issues. Of course, this goes through the userspace crypto API, so it 
might not be the same as the kernel using it directly.

Any tips on how to narrow in on this issue are appreciated :)

Best regards,
Sven

On 6/7/24 7:22 AM, Eric Biggers wrote:
> On Thu, Jun 06, 2024 at 03:12:26PM +0200, Sven wrote:
>> Do you happen to have some more infos on the self-tests you mentioned? Can
>> you point me at documentation, I coulnd't really find much useful stuff.
>>
>> Thanks
>>
>> On 5/28/24 9:42 PM, Eric Biggers wrote:
>>> Does this driver pass all the crypto self-tests?
> 
> The crypto self-tests can be enabled by unsetting
> CONFIG_CRYPTO_MANAGER_DISABLE_TESTS, and preferably setting
> CONFIG_CRYPTO_MANAGER_EXTRA_TESTS in order to enable the full set of tests.
> 
> They might catch something, though they don't test parallel usage of drivers
> which is where it sounds like the issue here might be here.
> 
> In any case the bug here is likely to be in the mxs-dcp driver, especially
> considering that drivers tend to be much lower quality than the software crypto
> (some driver developers apparently don't even bother to run the tests).
> 
> - Eric