RFR (M): 8161224: CONSTANT_NameAndType_info permits references to illegal names and descriptors
Dmitry Dmitriev
dmitry.dmitriev at oracle.com
Tue Sep 6 20:29:40 UTC 2016
Looks good!
Thanks,
Dmitry
----- Original Message -----
From: rachel.protacio at oracle.com
To: dmitry.dmitriev at oracle.com
Cc: hotspot-runtime-dev at openjdk.java.net
Sent: Tuesday, September 6, 2016 11:23:11 PM GMT +03:00 Iraq
Subject: Re: RFR (M): 8161224: CONSTANT_NameAndType_info permits references to illegal names and descriptors
That sounds fair to me. How's this?
@@ -4843,19 +4840,28 @@
}
}
else {
- // 4900761: For class version > 48, any unicode is allowed in class name.
+ // Skip leading 'L' and ignore first appearance of ';'
length--;
signature++;
- while (length > 0 && signature[0] != ';') {
- if (signature[0] == '.') {
- classfile_parse_error("Class name contains illegal character '.' in descriptor in class
file %s", CHECK_0);
- }
- length--;
- signature++;
- }
- if (signature[0] == ';') { return signature + 1; }
- }
-
+ char* c = strchr((char*) signature, ';');
+ // Format check signature
+ if (c != NULL) {
+ ResourceMark rm(THREAD);
+ int newlen = c - (char*) signature;
+ char* sig = NEW_RESOURCE_ARRAY(char, newlen + 1);
+ strncpy(sig, signature, newlen);
+ sig[newlen] = '\0';
+
+ bool legal = verify_unqualified_name(sig, newlen, LegalClass);
+ if (!legal) {
+ classfile_parse_error("Class name contains illegal character "
+ "in descriptor in class file %s",
+ CHECK_0);
+ return NULL;
+ }
+ return signature + newlen + 1;
+ }
+ }
return NULL;
}
case JVM_SIGNATURE_ARRAY:
It builds fine and passes the jck tests in question.
Rachel
On 9/6/2016 4:14 PM, Dmitry Dmitriev wrote:
Hi Rachel,
Small question about resource mark: on line 4850 you allocate new resource array which is used only in verify_unqualified_name() call. Do we need resource mark in this block?
Thanks,
Dmitry
----- Original Message -----
From: rachel.protacio at oracle.com To: hotspot-runtime-dev at openjdk.java.net Sent: Tuesday, September 6, 2016 11:01:45 PM GMT +03:00 Iraq
Subject: Re: RFR (M): 8161224: CONSTANT_NameAndType_info permits references to illegal names and descriptors
Hi Harold,
Thanks for the review! I've removed the resource mark. If no further
comments in the next hour, will commit.
Rachel
On 9/6/2016 3:29 PM, harold seigel wrote:
Hi Rachel,
The change looks good. Just one nit:
No resource mark is needed before calling classfile_parse_error() at
line 4857.
(No new RFR needed.)
Thanks, Harold
On 9/1/2016 5:30 PM, Rachel Protacio wrote:
Hello!
Please review this fix, which addresses a few issues related to
incomplete format checking with NameAndType names and signatures.
First, the code that should have been format checking the strings in
later classfile versions was in fact just checking for periods, so
I've rewritten it to call verify_unqualified_name(). Second, the
checks were (depending on the version) only performed when referenced
through Methodref/Fieldref/InterfaceMethodref/InvokeDynamic, meaning
that non-referenced NameAndType bytecodes did not get checked like
they were supposed to. My change enforces the spec in both aspects.
To summarize:
The existing code had:
- strict checks for pre-5.0
- incomplete/non-spec-compliant checks for 5.0-and-later
- no checks for un-referenced NameAndType (A) names and (B)
6.0-and-earlier signatures.
My change has:
- the same strict checks for pre-5.0
- complete/spec-compliant checks for 5.0-and-later
- all checks moved to the NameAndType section so all names and
signatures will be checked regardless of whether NameAndType is
referenced.
Bug: https://bugs.openjdk.java.net/browse/JDK-8161224 Open webrev: http://cr.openjdk.java.net/~rprotacio/8161224.00/ Testing: The jck tests which had been failing for this bug now pass,
along with all other jck vm tests. Also tested with JPRT and RBT
hotspot_all and noncolo tests.
A compatibility request has been approved for this change.
Thank you,
Rachel
More information about the hotspot-runtime-dev
mailing list