RFR: 8261522: [PPC64] AES intrinsics write beyond the destination array

[Martin Doerr]

I'd like to replace the read-modify-write implementation from aescrypt_encryptBlock / aescrypt_decryptBlock stubs. IT can cause severe problems (see bug description).

-------------

Commit messages:
 - 8261522: [PPC64] AES intrinsics write beyond the destination array

Changes: https://git.openjdk.java.net/jdk/pull/2514/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2514&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261522
  Stats: 48 lines in 1 file changed: 10 ins; 16 del; 22 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2514.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2514/head:pull/2514

PR: https://git.openjdk.java.net/jdk/pull/2514

[Ziviani]

On Wed, 10 Feb 2021 17:05:44 GMT, Martin Doerr <[hidden email]> wrote:

> I'd like to replace the read-modify-write implementation from aescrypt_encryptBlock / aescrypt_decryptBlock stubs. IT can cause severe problems (see bug description).

Very good. +1

Tested on P8, P9, and P10. With this patch I don't reproduce the problem. (tested LE only)

-------------

Marked as reviewed by [hidden email] (no known OpenJDK username).

PR: https://git.openjdk.java.net/jdk/pull/2514

[Martin Doerr]

> I'd like to replace the read-modify-write implementation from aescrypt_encryptBlock / aescrypt_decryptBlock stubs. It can cause severe problems (see bug description).

Martin Doerr has updated the pull request incrementally with one additional commit since the last revision:

  Add comment and key length assertions. Minor improvement.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2514/files
  - new: https://git.openjdk.java.net/jdk/pull/2514/files/d933c2f7..725dd8c7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2514&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2514&range=00-01

  Stats: 42 lines in 1 file changed: 26 ins; 8 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2514.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2514/head:pull/2514

PR: https://git.openjdk.java.net/jdk/pull/2514

[Ziviani]

On Mon, 15 Feb 2021 19:47:54 GMT, Martin Doerr <[hidden email]> wrote:

>> I'd like to replace the read-modify-write implementation from aescrypt_encryptBlock / aescrypt_decryptBlock stubs. It can cause severe problems (see bug description).
>
> Martin Doerr has updated the pull request incrementally with one additional commit since the last revision:
>
>   Add comment and key length assertions. Minor improvement.

+1

-------------

Marked as reviewed by [hidden email] (no known OpenJDK username).

PR: https://git.openjdk.java.net/jdk/pull/2514

[Lutz Schmidt]

On Mon, 15 Feb 2021 19:47:54 GMT, Martin Doerr <[hidden email]> wrote:

>> I'd like to replace the read-modify-write implementation from aescrypt_encryptBlock / aescrypt_decryptBlock stubs. It can cause severe problems (see bug description).
>
> Martin Doerr has updated the pull request incrementally with one additional commit since the last revision:
>
>   Add comment and key length assertions. Minor improvement.

Hi Martin,
the changes look good to me. Thanks for tracking that down and fixing it. I know that wasn't trivial.

One minor thing - it made me believe for a second I had missed something: in line 2631, the reference should be to vRet, not vsRet. Same is true further down in decryptBlock.

Lutz

-------------

Marked as reviewed by lucy (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2514

[Martin Doerr]

> I'd like to replace the read-modify-write implementation from aescrypt_encryptBlock / aescrypt_decryptBlock stubs. It can cause severe problems (see bug description).

Martin Doerr has updated the pull request incrementally with one additional commit since the last revision:

  Fix typo in comment.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2514/files
  - new: https://git.openjdk.java.net/jdk/pull/2514/files/725dd8c7..69188df5

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2514&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2514&range=01-02

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2514.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2514/head:pull/2514

PR: https://git.openjdk.java.net/jdk/pull/2514

[Martin Doerr]

On Tue, 16 Feb 2021 13:56:19 GMT, Lutz Schmidt <[hidden email]> wrote:

>> Martin Doerr has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Add comment and key length assertions. Minor improvement.
>
> Hi Martin,
> the changes look good to me. Thanks for tracking that down and fixing it. I know that wasn't trivial.
>
> One minor thing - it made me believe for a second I had missed something: in line 2631, the reference should be to vRet, not vsRet. Same is true further down in decryptBlock.
>
> Lutz

Thanks a lot for the reviews!
I've fixed the typo in the comments as suggested by Lutz.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2514

[Martin Doerr]

On Wed, 10 Feb 2021 17:05:44 GMT, Martin Doerr <[hidden email]> wrote:

> I'd like to replace the read-modify-write implementation from aescrypt_encryptBlock / aescrypt_decryptBlock stubs. It can cause severe problems (see bug description).

This pull request has now been integrated.

Changeset: 05d59556
Author:    Martin Doerr <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/05d59556
Stats:     76 lines in 1 file changed: 32 ins; 20 del; 24 mod

8261522: [PPC64] AES intrinsics write beyond the destination array

Reviewed-by: lucy

-------------

PR: https://git.openjdk.java.net/jdk/pull/2514