8352637: Enhance bytecode verification

Reviewed-by: fferrari, andrew
Backport-of: 974f4da2e53ebde8b06224f4ba0b80aa74c5f434
This commit is contained in:
Jan Kratochvil 2025-09-10 18:12:18 -07:00 committed by Andrew John Hughes
parent 5cffbcb034
commit 3273033153
5 changed files with 74 additions and 31 deletions

View File

@ -122,8 +122,16 @@ bool StackMapTable::match_stackmap(
}
void StackMapTable::check_jump_target(
StackMapFrame* frame, int32_t target, TRAPS) const {
StackMapFrame* frame, int bci, int offset, TRAPS) const {
ErrorContext ctx;
// Jump targets must be within the method and the method size is limited. See JVMS 4.11
int min_offset = -1 * max_method_code_size;
if (offset < min_offset || offset > max_method_code_size) {
frame->verifier()->verify_error(ErrorContext::bad_stackmap(bci, frame),
"Illegal target of jump or branch (bci %d + offset %d)", bci, offset);
return;
}
int target = bci + offset;
bool match = match_stackmap(
frame, target, true, false, &ctx, CHECK_VERIFY(frame->verifier()));
if (!match || (target < 0 || target >= _code_length)) {

View File

@ -86,7 +86,7 @@ class StackMapTable : public StackObj {
// Check jump instructions. Make sure there are no uninitialized
// instances on backward branch.
void check_jump_target(StackMapFrame* frame, int32_t target, TRAPS) const;
void check_jump_target(StackMapFrame* frame, int bci, int offset, TRAPS) const;
// The following methods are only used inside this class.

View File

@ -668,7 +668,6 @@ void ClassVerifier::verify_method(methodHandle m, TRAPS) {
// Merge with the next instruction
{
u2 index;
int target;
VerificationType type, type2;
VerificationType atype;
@ -1480,9 +1479,8 @@ void ClassVerifier::verify_method(methodHandle m, TRAPS) {
case Bytecodes::_ifle:
current_frame.pop_stack(
VerificationType::integer_type(), CHECK_VERIFY(this));
target = bcs.dest();
stackmap_table.check_jump_target(
&current_frame, target, CHECK_VERIFY(this));
&current_frame, bcs.bci(), bcs.get_offset_s2(), CHECK_VERIFY(this));
no_control_flow = false; break;
case Bytecodes::_if_acmpeq :
case Bytecodes::_if_acmpne :
@ -1493,19 +1491,16 @@ void ClassVerifier::verify_method(methodHandle m, TRAPS) {
case Bytecodes::_ifnonnull :
current_frame.pop_stack(
VerificationType::reference_check(), CHECK_VERIFY(this));
target = bcs.dest();
stackmap_table.check_jump_target
(&current_frame, target, CHECK_VERIFY(this));
(&current_frame, bcs.bci(), bcs.get_offset_s2(), CHECK_VERIFY(this));
no_control_flow = false; break;
case Bytecodes::_goto :
target = bcs.dest();
stackmap_table.check_jump_target(
&current_frame, target, CHECK_VERIFY(this));
&current_frame, bcs.bci(), bcs.get_offset_s2(), CHECK_VERIFY(this));
no_control_flow = true; break;
case Bytecodes::_goto_w :
target = bcs.dest_w();
stackmap_table.check_jump_target(
&current_frame, target, CHECK_VERIFY(this));
&current_frame, bcs.bci(), bcs.get_offset_s4(), CHECK_VERIFY(this));
no_control_flow = true; break;
case Bytecodes::_tableswitch :
case Bytecodes::_lookupswitch :
@ -2107,15 +2102,14 @@ void ClassVerifier::verify_switch(
}
}
}
int target = bci + default_offset;
stackmap_table->check_jump_target(current_frame, target, CHECK_VERIFY(this));
stackmap_table->check_jump_target(current_frame, bci, default_offset, CHECK_VERIFY(this));
for (int i = 0; i < keys; i++) {
// Because check_jump_target() may safepoint, the bytecode could have
// moved, which means 'aligned_bcp' is no good and needs to be recalculated.
aligned_bcp = (address)round_to((intptr_t)(bcs->bcp() + 1), jintSize);
target = bci + (jint)Bytes::get_Java_u4(aligned_bcp+(3+i*delta)*jintSize);
int offset = (jint)Bytes::get_Java_u4(aligned_bcp+(3+i*delta)*jintSize);
stackmap_table->check_jump_target(
current_frame, target, CHECK_VERIFY(this));
current_frame, bci, offset, CHECK_VERIFY(this));
}
NOT_PRODUCT(aligned_bcp = NULL); // no longer valid at this point
}
@ -2376,8 +2370,13 @@ bool ClassVerifier::ends_in_athrow(u4 start_bc_offset) {
break;
case Bytecodes::_goto:
case Bytecodes::_goto_w:
target = (opcode == Bytecodes::_goto ? bcs.dest() : bcs.dest_w());
case Bytecodes::_goto_w: {
int offset = (opcode == Bytecodes::_goto ? bcs.get_offset_s2() : bcs.get_offset_s4());
int min_offset = -1 * max_method_code_size;
// Check offset for overflow
if (offset < min_offset || offset > max_method_code_size) return false;
target = bci + offset;
if (visited_branches->contains(bci)) {
if (bci_stack->is_empty()) {
if (handler_stack->is_empty()) {
@ -2398,6 +2397,7 @@ bool ClassVerifier::ends_in_athrow(u4 start_bc_offset) {
visited_branches->append(bci);
}
break;
}
// Check that all switch alternatives end in 'athrow' bytecodes. Since it
// is difficult to determine where each switch alternative ends, parse
@ -2434,7 +2434,10 @@ bool ClassVerifier::ends_in_athrow(u4 start_bc_offset) {
// Push the switch alternatives onto the stack.
for (int i = 0; i < keys; i++) {
u4 target = bci + (jint)Bytes::get_Java_u4(aligned_bcp+(3+i*delta)*jintSize);
int min_offset = -1 * max_method_code_size;
int offset = (jint)Bytes::get_Java_u4(aligned_bcp+(3+i*delta)*jintSize);
if (offset < min_offset || offset > max_method_code_size) return false;
u4 target = bci + offset;
if (target > code_length) return false;
bci_stack->push(target);
}

View File

@ -121,8 +121,23 @@ class BaseBytecodeStream: StackObj {
void set_next_bci(int bci) { assert(0 <= bci && bci <= method()->code_size(), "illegal bci"); _next_bci = bci; }
// Bytecode-specific attributes
int dest() const { return bci() + bytecode().get_offset_s2(raw_code()); }
int dest_w() const { return bci() + bytecode().get_offset_s4(raw_code()); }
int get_offset_s2() const { return bytecode().get_offset_s2(raw_code()); }
int get_offset_s4() const { return bytecode().get_offset_s4(raw_code()); }
// These methods are not safe to use before or during verification as they may
// have large offsets and cause overflows
int dest() const {
int min_offset = -1 * max_method_code_size;
int offset = bytecode().get_offset_s2(raw_code());
guarantee(offset >= min_offset && offset <= max_method_code_size, "must be");
return bci() + offset;
}
int dest_w() const {
int min_offset = -1 * max_method_code_size;
int offset = bytecode().get_offset_s4(raw_code());
guarantee(offset >= min_offset && offset <= max_method_code_size, "must be");
return bci() + offset;
}
// One-byte indices.
int get_index_u1() const { assert_raw_index_size(1); return *(jubyte*)(bcp()+1); }

View File

@ -396,7 +396,8 @@ static jboolean is_superclass(context_type *, fullinfo_type);
static void initialize_exception_table(context_type *);
static int instruction_length(unsigned char *iptr, unsigned char *end);
static jboolean isLegalTarget(context_type *, int offset);
static jboolean isLegalOffset(context_type *, int bci, int offset);
static jboolean isLegalTarget(context_type *, int target);
static void verify_constant_pool_type(context_type *, int, unsigned);
static void initialize_dataflow(context_type *);
@ -1161,9 +1162,9 @@ verify_opcode_operands(context_type *context, unsigned int inumber, int offset)
case JVM_OPC_goto: {
/* Set the ->operand to be the instruction number of the target. */
int jump = (((signed char)(code[offset+1])) << 8) + code[offset+2];
int target = offset + jump;
if (!isLegalTarget(context, target))
if (!isLegalOffset(context, offset, jump))
CCerror(context, "Illegal target of jump or branch");
int target = offset + jump;
this_idata->operand.i = code_data[target];
break;
}
@ -1177,9 +1178,9 @@ verify_opcode_operands(context_type *context, unsigned int inumber, int offset)
int jump = (((signed char)(code[offset+1])) << 24) +
(code[offset+2] << 16) + (code[offset+3] << 8) +
(code[offset + 4]);
int target = offset + jump;
if (!isLegalTarget(context, target))
if (!isLegalOffset(context, offset, jump))
CCerror(context, "Illegal target of jump or branch");
int target = offset + jump;
this_idata->operand.i = code_data[target];
break;
}
@ -1218,13 +1219,16 @@ verify_opcode_operands(context_type *context, unsigned int inumber, int offset)
}
}
saved_operand = NEW(int, keys + 2);
if (!isLegalTarget(context, offset + _ck_ntohl(lpc[0])))
int jump = _ck_ntohl(lpc[0]);
if (!isLegalOffset(context, offset, jump))
CCerror(context, "Illegal default target in switch");
saved_operand[keys + 1] = code_data[offset + _ck_ntohl(lpc[0])];
int target = offset + jump;
saved_operand[keys + 1] = code_data[target];
for (k = keys, lptr = &lpc[3]; --k >= 0; lptr += delta) {
int target = offset + _ck_ntohl(lptr[0]);
if (!isLegalTarget(context, target))
jump = _ck_ntohl(lptr[0]);
if (!isLegalOffset(context, offset, jump))
CCerror(context, "Illegal branch in tableswitch");
target = offset + jump;
saved_operand[k + 1] = code_data[target];
}
saved_operand[0] = keys + 1; /* number of successors */
@ -1746,11 +1750,24 @@ static int instruction_length(unsigned char *iptr, unsigned char *end)
/* Given the target of a branch, make sure that it's a legal target. */
static jboolean
isLegalTarget(context_type *context, int offset)
isLegalTarget(context_type *context, int target)
{
int code_length = context->code_length;
int *code_data = context->code_data;
return (offset >= 0 && offset < code_length && code_data[offset] >= 0);
return (target >= 0 && target < code_length && code_data[target] >= 0);
}
/* Given a bci and offset, make sure the offset is valid and the target is legal */
static jboolean
isLegalOffset(context_type *context, int bci, int offset)
{
int code_length = context->code_length;
int *code_data = context->code_data;
int max_offset = 65535; // JVMS 4.11
int min_offset = -65535;
if (offset < min_offset || offset > max_offset) return JNI_FALSE;
int target = bci + offset;
return (target >= 0 && target < code_length && code_data[target] >= 0);
}