RFR(S): 8067648: JVM crashes reproducable with GCM cipher suites in GCTR doFinal
zoltan.majo at oracle.com
Mon Apr 13 11:51:43 UTC 2015
please review the following patch.
Problem: On architectures with hardware support for AES operations, the
Java version (the version in the JDK sources) of the
com.sun.crypto.provides.AESCrypt::encryptBlock(byte, int, byte, int)
method is replaced with an intrinsic that uses the CPU's AES instructions.
The Java version of encryptBlock operates on arrays of size
AES_BLOCK_SIZE=16 and it consequently performs a number of "implicit"
checks (e.g., null checks and range checks) as required by the Java VM
specification. The intrinsified version of encryptBlock, however, does
not perform any of these checks.
Omitting checks results in a VM crash if invalid parameters (e.g., a
null pointer, as reported in the current case) are passed to the method.
Solution: The failure reported in the current issue appears in the
com.sun.crypto.provider.GCTR class that calls the intrinsified version
of encryptBlock. None of the methods of the class are accessible from
packages other than com.sun.crypto.provider. So, after private a
discussion with John Rose, Vladimir Kozlov, and Roland Westrelin, I
propose to solve this problem on the Java-level.
The GCTR::counter field is supposed to be initialized with an array of
size AES_BLOCK_SIZE so that it is safe to call encryptBlock. The
'counter' field is never supposed to become NULL during the lifetime of
a GCTR object (so that encryptBlock can be always called safely).
The GCTR class supports saving and restoring the value of the 'counter'
field (via the save() and restore() methods). For saving/restoring, the
class uses the 'counterSave' field as temporary storage.
It is also possible to reset the a GCTR object to its initial state by
calling reset(). Reset sets both the 'counter' and 'counterSave' fields
to their initial values.
If a call to the method reset() is followed by a call to restore(), the
field 'counter' is not restored to its original value, but it becomes
NULL. This is an invalid state, because a GCTR object should always
contain a valid 'counter' array. This problem has been also described
(in part) by Chris Ellis.
This patch proposes to restore the contents of 'counter' from
'counterSave' only if some data has been saved into 'counterSave' before
(i.e., counterSave is not NULL). The patch also adds a check to the
constructor of GCTR to verify if the length of 'counter' is
AES_BLOCK_SIZE. (I checked and JDK code uses this class only with arrays
of size AES_BLOCK_SIZE, but it is good if the required size is
documented and enforced by GCTR.)
The array to store the output of the encryptBlock method (the third
parameter) should be also of length AES_BLOCK_SIZE. That is ensured by
the GCTR class (both in the doFinal and update methods). The input and
output offsets (the second and fourth parameters) are 0, as required by
- JPRT (both with 9 and 8u), all tests in the testsets hotspot pass;
- JTREG tests in jdk_security[1-4] executed locally with the sources
built with --enable-openjdk-only; all tests that pass without the patch
pass with the patch as well;
- failure reported in 8067648 can be reproduced with 8u, failure is not
triggered with patch applied.
Thank you and best regards,
More information about the hotspot-compiler-dev