ICP: AES-GCM: Unify gcm_init_ctx() and gmac_init_ctx()

[AttilaFueloep]

Motivation and Context

gmac_init_ctx() duplicates most of the code in gcm_int_ctx() while it just needs to set its own IV length and AAD tag length.

Description

Introduce gcm_init_ctx_impl() which handles the GCM and GMAC differences while reusing the duplicated code.

While here, constify the IV and AAD pointers passed to gcm_init{,_avx}().

How Has This Been Tested?

Builds, included in another PR which passes testing.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[robn]

The only difference I see is that previously, gmac_init_ctx would always do the key schedule byteswap check, and never do the MOVBE check, whereas gcm_init_ctx would do both checks only when AVX is selected by the cycle. Now, they both do it the gcm_init_ctx way.

I'm not sure if that was intentional or not, but it is a difference.

(It does seem like both checks should always be done, ie, doesn't matter how we decide to use AVX, but if we do, we should consider all the details).

[AttilaFueloep]

Thanks, well spotted, this is very helpful feedback.

Only gcm_init_ctx() should do the MOVBE check since GMAC just calls gcm_init() and gcm_{en,de}crypt_final(). Both functions don't call into MOVBE/BSWAP code paths so there's no need to toggle between them in the GMAC case. I changed the code to skip the toggle in the GMAC case. Toggling gcm_avx_can_use_movbe makes only sense for the cycle implementation, so the current location is correct.

Not checking for a byte swapped key schedule if we are using the AVX implementation (as opposed to the cycle implementation) is a major bug with the potential of pool corruption. I could easily produce a panic by setting icp_gcm_impl to avx and icp_aes_impl to generic and starting some crypto i/o. Thanks for catching that! It was there from day one since I implemented the AVX support. Fortunately this isn't something one would do intentionally so hopefully no one has been bitten by it yet.

In summary, the key schedule byte swap check should be done for both, the cycle and the AVX implementation. In the cycle case we can silently fallback to a non-AVX implementation but for the AVX case we should print a warning in addition to the fallback at least.

The proper fix of course would be to deny setting the GCM implementation to one which won't work with the AES one and vice versa. Unfortunately this is a bit tricky since it would require synchronizing the routines setting the AES and GCM implementation. Not sure if it's worth the effort for such an edge case.

[AttilaFueloep]

Added fixes, rebased to master and force pushed to get a clean test run.

[behlendorf]

@AttilaFueloep rebasing one more time should sort of the GitHub Actions CI infrastructure issue so you can get a clean test run.

[AttilaFueloep]

Rebased. I wonder if we have the equivalent of pr_err_once(), that is log a message to syslog only on the first call, not always. Otherwise I'd say it's better to not log at all than flooding the syslog file. Since the ICP is used by the macOS and Windows ports as well, we can't use the Linux function here.

[AttilaFueloep]

The make failures seem to be related to #14527. The zloop failure (ztest_dmu_snapshot_hold()) semms to be unrelated as well.

[AttilaFueloep]

Once #14567 is merged I can drop d6eca66, rebase and squash. It should be ready for merging then.

[behlendorf]

@AttilaFueloep merged #14567, I'll get this merged once you rebase.

[AttilaFueloep]

Rebased, squashed and updated the commit message.