RFR: JDK-8277029: JMM GetDiagnosticXXXInfo APIs should verify output array sizes

[David Holmes]

On Fri, 12 Nov 2021 08:25:04 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> jmm_GetDiagnosticCommandArgumentsInfo and jmm_GetDiagnosticCommandInfo are used to query the hotspot about diagnostic commands. They provide output arrays for the information:
> 
> 
> void jmm_GetDiagnosticCommandArgumentsInfo(JNIEnv *env,
>           jstring command, dcmdArgInfo* infoArray)
> 
> 
> but array size is implicitly assumed to be known to both caller and callee. Caller and callee negotiate those sizes in prior steps, but things can go wrong. E.g. I recently hunted a bug where `DCmd::number_arguments()` was off - did not reflect the real number of its jcmd parameters - which led to a hidden memory overwriter.
> 
> Thankfully, JDK-8264565 rewrote the dcmd framework to deal with this particular issue (The VM I analyzed was older). Still, it would be good if we had additional safety measures here.
> 
> -------------
> 
> Testing:
> - manual tests with artificially induced error in one dcmd for debug, release
> - GHAs (which include tier1 serviceability jcmd tests which use JMX and exercise these APIs)

Hi Thomas,

Sorry but only half of this makes sense to me.

Cheers,
David

src/hotspot/share/services/management.cpp line 1968:

> 1966: 
> 1967: JVM_ENTRY(void, jmm_GetDiagnosticCommandInfo(JNIEnv *env, jobjectArray cmds,
> 1968:           dcmdInfo* infoArray, jint count))

I do not see the point of the change in this case. This doesn't check for a mismatch between an "external" and "internal" value but between two external values. If you don't trust the caller to size infoArray correctly then how can you trust them to pass in the right "count" ?

src/hotspot/share/services/management.cpp line 2022:

> 2020: 
> 2021: JVM_ENTRY(void, jmm_GetDiagnosticCommandArgumentsInfo(JNIEnv *env,
> 2022:           jstring command, dcmdArgInfo* infoArray, jint count))

This one makes sense because you verify that the external expectation about the arg count matches the actual internal representation.

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

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