[Bionic,SRU,0/4] Fix build for crypto/testmgr.o
mbox series

Message ID 20190823220825.25608-1-connor.kuehl@canonical.com
Headers show
Series
  • Fix build for crypto/testmgr.o
Related show

Message

Connor Kuehl Aug. 23, 2019, 10:08 p.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1841264

[Impact]

Ubuntu commit aae817ffb114 "crypto: testmgr - add AES-CFB tests" added new test cases
to the crypto self-tests. This patch is referring to structure members that don't exist
in Bionic because the large mainline clean up patch 92a4c9fef34c crypto: "testmgr -
eliminate redundant decryption test vectors" has not been backported.

As a result, Bionic will fail to build if the crypto self tests are enabled in the
kernel config. This build failure was noticed when building a derivative kernel.

This mainline patch is a massive refactoring, complete with the updated structure
definitions that will resolve the build failure:

92a4c9fef34c "crypto: testmgr - eliminate redundant decryption test vectors"

Furthermore, once the pre-requisite patch is backported, the CFB module also needs
to be backported as the new tests added by Ubuntu commit aae817ffb114 "crypto:
testmgr - add AES-CFB tests" will attempt to load that module at runtime to execute its tests.

The primary argument for the inclusion of this backport would be that we would not have to
amend future crypto test case additions as they come in via stable update sync to use the "older"
code structure.

An alternative would be to revert aae817ffb114 "crypto: testmgr - add AES-CFB tests" or we can
amend it to use the "older" code structure.

[Testing]

After backporting the pre-requisite refactor patch and the CFB module that the new tests depend
on, I compiled and booted into the kernel and ran the Crypto test suite by loading the "tcrypt"
module. I compared the test suite results with a Bionic kernel that does NOT have this backport
and confirmed they both emitted the same test results from the crypto self tests:

[ 15.536400] alg: hash: Failed to load transform for hmac(crc32): -2
[ 15.552214] alg: cprng: Failed to load transform for ansi_cprng: -2
[ 15.590773] tcrypt: one or more tests failed!

No new test failures have been added as a result of this backport.

[Regression Potential]

There is a chance for breakage within the test suite because the primary patch that resolves this
build failure, 92a4c9fef34c "crypto: testmgr - eliminate redundant decryption test vectors" is not
practical to review.

However, this backport was accomplished with the very same awk script supplied by the original patch
author and the results between one kernel and a modified kernel are consistent.

Comments

Marcelo Henrique Cerri Aug. 29, 2019, 2:33 p.m. UTC | #1
Hi, Connor. Let's proceed with the revert to avoid potential
regressions, but I would like to keep your backport at hand if we
need it in the future.

Thank you for all the work analyzing and backporting that!

On Fri, Aug 23, 2019 at 03:08:21PM -0700, Connor Kuehl wrote:
> BugLink: https://bugs.launchpad.net/bugs/1841264
> 
> [Impact]
> 
> Ubuntu commit aae817ffb114 "crypto: testmgr - add AES-CFB tests" added new test cases
> to the crypto self-tests. This patch is referring to structure members that don't exist
> in Bionic because the large mainline clean up patch 92a4c9fef34c crypto: "testmgr -
> eliminate redundant decryption test vectors" has not been backported.
> 
> As a result, Bionic will fail to build if the crypto self tests are enabled in the
> kernel config. This build failure was noticed when building a derivative kernel.
> 
> This mainline patch is a massive refactoring, complete with the updated structure
> definitions that will resolve the build failure:
> 
> 92a4c9fef34c "crypto: testmgr - eliminate redundant decryption test vectors"
> 
> Furthermore, once the pre-requisite patch is backported, the CFB module also needs
> to be backported as the new tests added by Ubuntu commit aae817ffb114 "crypto:
> testmgr - add AES-CFB tests" will attempt to load that module at runtime to execute its tests.
> 
> The primary argument for the inclusion of this backport would be that we would not have to
> amend future crypto test case additions as they come in via stable update sync to use the "older"
> code structure.
> 
> An alternative would be to revert aae817ffb114 "crypto: testmgr - add AES-CFB tests" or we can
> amend it to use the "older" code structure.
> 
> [Testing]
> 
> After backporting the pre-requisite refactor patch and the CFB module that the new tests depend
> on, I compiled and booted into the kernel and ran the Crypto test suite by loading the "tcrypt"
> module. I compared the test suite results with a Bionic kernel that does NOT have this backport
> and confirmed they both emitted the same test results from the crypto self tests:
> 
> [ 15.536400] alg: hash: Failed to load transform for hmac(crc32): -2
> [ 15.552214] alg: cprng: Failed to load transform for ansi_cprng: -2
> [ 15.590773] tcrypt: one or more tests failed!
> 
> No new test failures have been added as a result of this backport.
> 
> [Regression Potential]
> 
> There is a chance for breakage within the test suite because the primary patch that resolves this
> build failure, 92a4c9fef34c "crypto: testmgr - eliminate redundant decryption test vectors" is not
> practical to review.
> 
> However, this backport was accomplished with the very same awk script supplied by the original patch
> author and the results between one kernel and a modified kernel are consistent.