8299817: [s390] AES-CTR mode intrinsic fails with multiple short update() calls

[RealLucy]

This PR addresses an issue in the AES-CTR mode intrinsic on s390. When a message is ciphered in multiple, small (< 16 bytes) segments, the result is incorrect.

This is not just a band-aid fix. The issue was taken as a chance to restructure the code. though still complicated, It is now easier to read and (hopefully) understand.

Except for the new jetreg test, the changes are purely s390. There are no side effects on other platforms. Issue-specific tests pass. Other tests are in progress. I will update this PR once they are complete.

Reviews and comments are very much appreciated.

@backwaterred could you please run some "official" s390 tests? Thanks.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8299817: [s390] AES-CTR mode intrinsic fails with multiple short update() calls

Reviewers

• Matthias Baesken (@MBaesken - Reviewer) ⚠️ Review applies to ca40de84
• @offamitkumar (no known openjdk.org user name / role) ⚠️ Review applies to 0c8ced1e
• Martin Doerr (@TheRealMDoerr - Reviewer) ⚠️ Review applies to 0c8ced1e

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/11967/head:pull/11967
$ git checkout pull/11967

Update a local copy of the PR:
$ git checkout pull/11967
$ git pull https://git.openjdk.org/jdk pull/11967/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 11967

View PR using the GUI difftool:
$ git pr show -t 11967

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/11967.diff

[bridgekeeper]

👋 Welcome back lucy! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@RealLucy The following labels will be automatically applied to this pull request:

• hotspot
• security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 04: Full - Incremental (6de4aef7)
• 03: Full (0c8ced1e)
• 02: Full - Incremental (ca40de84)
• 01: Full - Incremental (33722f27)
• 00: Full (7af2c3db)

[offamitkumar]

Hi @RealLucy , Sorry for kept you waiting. I've run tests over s390/Z machine and everything seems fine (Test failures I'm seeing were already there even before this PR).

But please let me know if anything specific you want me to test, as of now I've run tier1 test in fast & slow debug build for this PR.

[RealLucy]

Thank you for testing, Amit.
In addition to that, an identical version of the code runs in our internal tests for our commercial product. Everything is fine there as well.

[MBaesken]

Hi Lutz, is the JIT_TIMER still needed ?

[MBaesken]

Some small typos found

nesessary -> necessary
sace extended SP -> save extended SP

Why do you clear memory only in the ASSERT case here ?

src/hotspot/cpu/s390/stubGenerator_s390.cpp

#ifdef ASSERT
__ clear_mem(Address(Z_SP, (intptr_t)8), resize_len - 8);
#endif

[RealLucy]

Typos repaired, JIT_TIMER references removed.
The allocated stack space is cleared for debugging purposes only.
Thanks for having a look, @MBaesken

[openjdk]

@RealLucy This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8299817: [s390] AES-CTR mode intrinsic fails with multiple short update() calls

Reviewed-by: mbaesken, mdoerr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 97 new commits pushed to the master branch:

• a2c5a4a: 8302732: sun/net/www/http/HttpClient/MultiThreadTest.java still failing intermittently
• db217c9: 8292583: Comment for ciArrayKlass::make is wrong
• 2613b94: 8302149: Speed up compiler/jsr292/methodHandleExceptions/TestAMEnotNPE.java
• d2660a6: 8303045: Remove RegionNode::LoopStatus::NeverIrreducibleEntry assert with wrong assumption
• 1794f49: 8302681: [IR Framework] Only allow cpuFeatures from a verified list
• a43931b: 8303102: jcmd: ManagementAgent.status truncates the text longer than O_BUFLEN
• 2fb1e3b: 8302268: Prefer ArrayList to LinkedList in XEmbeddedFramePeer
• 3a9f491: 8301964: Expensive fillInStackTrace operation in HttpURLConnection.getLastModified when no last-modified in response
• 1dbd18a: 8302120: Prefer ArrayList to LinkedList in AggregatePainter
• 2e78d83: 8302899: Executors.newSingleThreadExecutor can use Cleaner to shutdown executor
• ... and 87 more: https://git.openjdk.org/jdk/compare/98716e2b251c5e86e840116d0c70e2bb07993a10...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@RealLucy this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8299817
git fetch https://git.openjdk.org/jdk master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

[RealLucy]

Being an s390-only PR, failures on arm build and linux-x85 tests are unrelated.

[offamitkumar]

Build and Testing done for tier1 in fastdebug, slowdebug and release build on z15/s390x machine.

LGTM

[TheRealMDoerr]

Not a detailed review, but I couldn't spot anything bad.

[RealLucy]

Build and test failures seem unrelated to this s390-only fix.

[RealLucy]

/integrate

All SAP-internal testing is fine.

[openjdk]

Going to push as commit e144783.
Since your change was applied there have been 115 commits pushed to the master branch:

• 5feb13b: 8301701: java/net/DatagramSocket/DatagramSocketMulticasting.java should be hardened
• 1e3c9fd: 8026369: javac potentially ambiguous overload warning needs an improved scheme
• 14a014d: 8302124: HotSpot Style Guide should permit noreturn attribute
• bca60f4: 8303249: JFR: Incorrect description of dumponexit
• f7f1036: 8303068: Memory leak in DwarfFile::LineNumberProgram::run_line_number_program
• 784f7b1: 8293667: Align jlink's --compress option with jmod's --compress option
• 54603aa: 8303208: JFR: 'jfr print' displays incorrect timestamps
• 4c169d2: 8303253: Remove unnecessary calls to super() in java.time value based classes
• b527edd: 8292914: Lambda proxies have unstable names
• 42330d2: 8029370: (fc) FileChannel javadoc not clear for cases where position == size
• ... and 105 more: https://git.openjdk.org/jdk/compare/98716e2b251c5e86e840116d0c70e2bb07993a10...master

Your commit was automatically rebased without conflicts.

[openjdk]

@RealLucy Pushed as commit e144783.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.