Re: RFR: 8308549: Classfile API should fail to generate over-sized Code attribute [v2]
On Wed, 24 May 2023 12:57:59 GMT, Adam Sotona wrote:
>> src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java
>> line 314:
>>
>>> 312:
>>> 313: int codeLength = curPc();
>>> 314: if (codeLength >= 65536) {
>>
>> Hello Adam, looking at the JVM spec, section 4.7.3
>> https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-4.7.3,
>> it states:
>>
>>> The value of code_length must be greater than zero (as the code array must
>>> not be empty) and less than 65536.
>>
>> Do you think this check then should also verify (and throw) if the
>> codeLength <= 0?
>
> Right, DirectCodeBuilder can be triggered empty, thanks.
Thank you Adam for the update, looks good to me.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14100#discussion_r1204181523
Re: RFR: 8308549: Classfile API should fail to generate over-sized Code attribute [v2]
> Classfile API allowed to generate Code attribute exceeding the 65k limit. No > exception has been thrown during class generation and the class failed > verification later during class loading. > This patch adds Code size limit check throwing IllegalArgumentException. > The patch also adds similar check for constant pool size limit to avoid > generation class file with corrupted constant pool. > Two new tests are added to check response on oversized Code attribute and > constant pool. > `VerifierImpl` is extended to check Code attribute size as a part of class > verification process. > > Please review. > > Thanks, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: added check for empty Code + test - Changes: - all: https://git.openjdk.org/jdk/pull/14100/files - new: https://git.openjdk.org/jdk/pull/14100/files/32c00386..d4b21a22 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14100&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14100&range=00-01 Stats: 12 lines in 4 files changed: 10 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/14100.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14100/head:pull/14100 PR: https://git.openjdk.org/jdk/pull/14100
Re: RFR: 8308549: Classfile API should fail to generate over-sized Code attribute
On Wed, 24 May 2023 12:04:05 GMT, Jaikiran Pai wrote:
>> Classfile API allowed to generate Code attribute exceeding the 65k limit. No
>> exception has been thrown during class generation and the class failed
>> verification later during class loading.
>> This patch adds Code size limit check throwing IllegalArgumentException.
>> The patch also adds similar check for constant pool size limit to avoid
>> generation class file with corrupted constant pool.
>> Two new tests are added to check response on oversized Code attribute and
>> constant pool.
>> `VerifierImpl` is extended to check Code attribute size as a part of class
>> verification process.
>>
>> Please review.
>>
>> Thanks,
>> Adam
>
> src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java
> line 314:
>
>> 312:
>> 313: int codeLength = curPc();
>> 314: if (codeLength >= 65536) {
>
> Hello Adam, looking at the JVM spec, section 4.7.3
> https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-4.7.3,
> it states:
>
>> The value of code_length must be greater than zero (as the code array must
>> not be empty) and less than 65536.
>
> Do you think this check then should also verify (and throw) if the codeLength
> <= 0?
Right, DirectCodeBuilder can be triggered empty, thanks.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14100#discussion_r1204085835
Re: RFR: 8308549: Classfile API should fail to generate over-sized Code attribute
On Tue, 23 May 2023 12:54:20 GMT, Adam Sotona wrote:
> Classfile API allowed to generate Code attribute exceeding the 65k limit. No
> exception has been thrown during class generation and the class failed
> verification later during class loading.
> This patch adds Code size limit check throwing IllegalArgumentException.
> The patch also adds similar check for constant pool size limit to avoid
> generation class file with corrupted constant pool.
> Two new tests are added to check response on oversized Code attribute and
> constant pool.
> `VerifierImpl` is extended to check Code attribute size as a part of class
> verification process.
>
> Please review.
>
> Thanks,
> Adam
src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java
line 314:
> 312:
> 313: int codeLength = curPc();
> 314: if (codeLength >= 65536) {
Hello Adam, looking at the JVM spec, section 4.7.3
https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-4.7.3, it
states:
> The value of code_length must be greater than zero (as the code array must
> not be empty) and less than 65536.
Do you think this check then should also verify (and throw) if the codeLength
<= 0?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14100#discussion_r1203993221
Re: RFR: 8308549: Classfile API should fail to generate over-sized Code attribute
On Tue, 23 May 2023 15:13:04 GMT, Chen Liang wrote: > On a side note, does Classfile API reject methods with too many slots > (locals) (MethodTypeDesc can represent parameter lists with over 255 slots) > or stack (operand)? Classfile API does not perform any extra verifications of MethodTypeDesc and 255 slots is limit for method parameters. Code attribute maxLocals and maxStack have limit of 65535, however it is not checked yet. - PR Comment: https://git.openjdk.org/jdk/pull/14100#issuecomment-1560933070
Re: RFR: 8308549: Classfile API should fail to generate over-sized Code attribute
On Tue, 23 May 2023 12:54:20 GMT, Adam Sotona wrote: > Classfile API allowed to generate Code attribute exceeding the 65k limit. No > exception has been thrown during class generation and the class failed > verification later during class loading. > This patch adds Code size limit check throwing IllegalArgumentException. > The patch also adds similar check for constant pool size limit to avoid > generation class file with corrupted constant pool. > Two new tests are added to check response on oversized Code attribute and > constant pool. > `VerifierImpl` is extended to check Code attribute size as a part of class > verification process. > > Please review. > > Thanks, > Adam This looks ok to me. - Marked as reviewed by mchung (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14100#pullrequestreview-1440411354
Re: RFR: 8308549: Classfile API should fail to generate over-sized Code attribute
On Tue, 23 May 2023 12:54:20 GMT, Adam Sotona wrote: > Classfile API allowed to generate Code attribute exceeding the 65k limit. No > exception has been thrown during class generation and the class failed > verification later during class loading. > This patch adds Code size limit check throwing IllegalArgumentException. > The patch also adds similar check for constant pool size limit to avoid > generation class file with corrupted constant pool. > Two new tests are added to check response on oversized Code attribute and > constant pool. > `VerifierImpl` is extended to check Code attribute size as a part of class > verification process. > > Please review. > > Thanks, > Adam On a side note, does Classfile API reject methods with too many slots (locals) (MethodTypeDesc can represent parameter lists with over 255 slots) or stack (operand)? - PR Comment: https://git.openjdk.org/jdk/pull/14100#issuecomment-1559642335
RFR: 8308549: Classfile API should fail to generate over-sized Code attribute
Classfile API allowed to generate Code attribute exceeding the 65k limit. No exception has been thrown during class generation and the class failed verification later during class loading. This patch adds Code size limit check throwing IllegalArgumentException. The patch also adds similar check for constant pool size limit to avoid generation class file with corrupted constant pool. Two new tests are added to check response on oversized Code attribute and constant pool. `VerifierImpl` is extended to check Code attribute size as a part of class verification process. Please review. Thanks, Adam - Commit messages: - 8308549: Classfile API should fail to generate over-sized Code attribute Changes: https://git.openjdk.org/jdk/pull/14100/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14100&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8308549 Stats: 40 lines in 4 files changed: 35 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/14100.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14100/head:pull/14100 PR: https://git.openjdk.org/jdk/pull/14100
