JDK-6665028 : native code of method j*.text.Bidi.nativeBidiChars is using the contents of a primitive array direct
  • Type: Bug
  • Status: Resolved
  • Resolution: Fixed
  • Component: core-libs
  • Sub-Component: java.text
  • Priority: P3
  • Affected Version: 6u4
  • OS: solaris_7
  • CPU: sparc
  • Submit Date: 2008-02-19
  • Updated Date: 2010-07-29
  • Resolved Date: 2008-03-25
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availabitlity Release.

To download the current JDK release, click here.
Other JDK 6 JDK 7
5.0u19-revFixed 6u10 b20Resolved 7Fixed
Related Reports
Relates :  
Description
It seems that the native code of method java.text.Bidi.nativeBidiChars is using the contents of a primitive array directly after calling to the JNI function ReleasePrimitiveArrayCritical.

Here are some code analysis in detail.

The definition of native method java.text.Bidi.nativeBidiChars,
Java_java_text_Bidi_nativeBidiChars, is in file
j2se/src/share/native/sun/font/bidi/jbidi.c.
For the case that parameter "embs" is not NULL, cEmbs will be set to the
pointer(got by GetPrimitiveArrayCritical) to the contents of byte array
embs.
In function ubidi_setPara, which is defined in file
j2se/src/share/native/sun/font/bidi/ubidi.c, field "levels" of pBiDi
will be set to embeddingLevels, which
is the value of cEmbs. After calling to function ubidi_setPara,
ReleasePrimitiveArrayCritical is called for the byte array embs, however
the value of
cEmbs is still being held by bidi->levels, and will be used later in
function ubidi_getLogicalRun, defined in file
j2se/src/share/native/sun/font/bidi/ubidiln.c.
Since GC is no longer locked after calling to
ReleasePrimitiveArrayCritical, we think it is possible that the contents
of byte array embs is moved to some
place else, and bidi->levels is pointing to the wrong place.

We suggest that the piece of code in function ubidi_setPara in file
j2se/src/share/native/sun/font/bidi/ubidi.c

------------------------------------------------------------------------
-
    /* are explicit levels specified? */
    if(embeddingLevels==NULL) {
        /* no: determine explicit levels according to the (Xn) rules */\
        if(getLevelsMemory(pBiDi, length)) {
            pBiDi->levels=pBiDi->levelsMemory;
            direction=resolveExplicitLevels(pBiDi);
        } else {
            *pErrorCode=U_MEMORY_ALLOCATION_ERROR;
            return;
        }
    } else {
        /* set BN for all explicit codes, check that all levels are
paraLevel..UBIDI_MAX_EXPLICIT_LEVEL */
        pBiDi->levels=embeddingLevels;
        direction=checkExplicitLevels(pBiDi, pErrorCode);
        if(U_FAILURE(*pErrorCode)) {
            return;
        }
    }
------------------------------------------------------------------------
-

might be changed to

------------------------------------------------------------------------
-
    if(getLevelsMemory(pBiDi, length)) {
        pBiDi->levels=pBiDi->levelsMemory;
        /* are explicit levels specified? */
        if(embeddingLevels==NULL) {
            /* no: determine explicit levels according to the (Xn) rules
\ */
            direction=resolveExplicitLevels(pBiDi);
        } else {
            /* set BN for all explicit codes, check that all levels are
paraLevel..UBIDI_MAX_EXPLICIT_LEVEL */
            icu_memcpy(pBiDi->levels, embeddingLevels, length);
            direction=checkExplicitLevels(pBiDi, pErrorCode);
            if(U_FAILURE(*pErrorCode)) {
                return;
            }
        }
    } else {
        *pErrorCode=U_MEMORY_ALLOCATION_ERROR;
        return;
    }
------------------------------------------------------------------------
-

Comments
EVALUATION Yes, we should take the suggested fix.
2008-02-20

SUGGESTED FIX We suggest that the piece of code in function ubidi_setPara in file j2se/src/share/native/sun/font/bidi/ubidi.c ------------------------------------------------------------------------ - /* are explicit levels specified? */ if(embeddingLevels==NULL) { /* no: determine explicit levels according to the (Xn) rules */\ if(getLevelsMemory(pBiDi, length)) { pBiDi->levels=pBiDi->levelsMemory; direction=resolveExplicitLevels(pBiDi); } else { *pErrorCode=U_MEMORY_ALLOCATION_ERROR; return; } } else { /* set BN for all explicit codes, check that all levels are paraLevel..UBIDI_MAX_EXPLICIT_LEVEL */ pBiDi->levels=embeddingLevels; direction=checkExplicitLevels(pBiDi, pErrorCode); if(U_FAILURE(*pErrorCode)) { return; } } ------------------------------------------------------------------------ - might be changed to ------------------------------------------------------------------------ - if(getLevelsMemory(pBiDi, length)) { pBiDi->levels=pBiDi->levelsMemory; /* are explicit levels specified? */ if(embeddingLevels==NULL) { /* no: determine explicit levels according to the (Xn) rules \ */ direction=resolveExplicitLevels(pBiDi); } else { /* set BN for all explicit codes, check that all levels are paraLevel..UBIDI_MAX_EXPLICIT_LEVEL */ icu_memcpy(pBiDi->levels, embeddingLevels, length); direction=checkExplicitLevels(pBiDi, pErrorCode); if(U_FAILURE(*pErrorCode)) { return; } } } else { *pErrorCode=U_MEMORY_ALLOCATION_ERROR; return; } ------------------------------------------------------------------------ - *** (#1 of 1): [ UNSAVED ] ###@###.###
2008-02-19