From 6a8a68cd7707e75729ca377ee4d692401dc2ccbe Mon Sep 17 00:00:00 2001 From: Philip Craig Date: Thu, 4 Jul 2024 12:00:49 +1000 Subject: [PATCH] read/line: check for overflow when advancing the address Addresses in line sequences should only increase. The `addr2line` crate relies on this for its binary search. --- src/read/line.rs | 137 ++++++++++++++++++++++++++++++++-------------- src/write/line.rs | 5 +- 2 files changed, 99 insertions(+), 43 deletions(-) diff --git a/src/read/line.rs b/src/read/line.rs index c4d2e28c..dc4a089e 100644 --- a/src/read/line.rs +++ b/src/read/line.rs @@ -240,7 +240,7 @@ where Err(err) => return Err(err), Ok(None) => return Ok(None), Ok(Some(instruction)) => { - if self.row.execute(instruction, &mut self.program) { + if self.row.execute(instruction, &mut self.program)? { if self.row.tombstone { // Perform any reset that was required for the tombstone row. // Normally this is done when `next_row` is called again, but for @@ -631,7 +631,7 @@ pub type LineNumberRow = LineRow; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct LineRow { tombstone: bool, - address: Wrapping, + address: u64, op_index: Wrapping, file: u64, line: Wrapping, @@ -652,7 +652,7 @@ impl LineRow { // "At the beginning of each sequence within a line number program, the // state of the registers is:" -- Section 6.2.2 tombstone: false, - address: Wrapping(0), + address: 0, op_index: Wrapping(0), file: 1, line: Wrapping(1), @@ -676,7 +676,7 @@ impl LineRow { /// generated by the compiler." #[inline] pub fn address(&self) -> u64 { - self.address.0 + self.address } /// > An unsigned integer representing the index of an operation within a VLIW @@ -800,21 +800,21 @@ impl LineRow { &mut self, instruction: LineInstruction, program: &mut Program, - ) -> bool + ) -> Result where Program: LineProgram, R: Reader, { - match instruction { + Ok(match instruction { LineInstruction::Special(opcode) => { - self.exec_special_opcode(opcode, program.header()); + self.exec_special_opcode(opcode, program.header())?; true } LineInstruction::Copy => true, LineInstruction::AdvancePc(operation_advance) => { - self.apply_operation_advance(operation_advance, program.header()); + self.apply_operation_advance(operation_advance, program.header())?; false } @@ -846,13 +846,18 @@ impl LineRow { LineInstruction::ConstAddPc => { let adjusted = self.adjust_opcode(255, program.header()); let operation_advance = adjusted / program.header().line_encoding.line_range; - self.apply_operation_advance(u64::from(operation_advance), program.header()); + self.apply_operation_advance(u64::from(operation_advance), program.header())?; false } LineInstruction::FixedAddPc(operand) => { - self.address += Wrapping(u64::from(operand)); - self.op_index.0 = 0; + if !self.tombstone { + self.address = self + .address + .checked_add(u64::from(operand)) + .ok_or(Error::AddressOverflow)?; + self.op_index.0 = 0; + } false } @@ -879,8 +884,13 @@ impl LineRow { LineInstruction::SetAddress(address) => { let tombstone_address = !0 >> (64 - program.header().encoding.address_size * 8); self.tombstone = address == tombstone_address; - self.address.0 = address; - self.op_index.0 = 0; + if !self.tombstone { + if address < self.address { + return Err(Error::InvalidAddressRange); + } + self.address = address; + self.op_index.0 = 0; + } false } @@ -899,7 +909,7 @@ impl LineRow { | LineInstruction::UnknownStandard1(_, _) | LineInstruction::UnknownStandardN(_, _) | LineInstruction::UnknownExtended(_, _) => false, - } + }) } /// Perform any reset that was required after copying the previous row. @@ -940,7 +950,11 @@ impl LineRow { &mut self, operation_advance: u64, header: &LineProgramHeader, - ) { + ) -> Result<()> { + if self.tombstone { + return Ok(()); + } + let operation_advance = Wrapping(operation_advance); let minimum_instruction_length = u64::from(header.line_encoding.minimum_instruction_length); @@ -950,15 +964,20 @@ impl LineRow { u64::from(header.line_encoding.maximum_operations_per_instruction); let maximum_operations_per_instruction = Wrapping(maximum_operations_per_instruction); - if maximum_operations_per_instruction.0 == 1 { - self.address += minimum_instruction_length * operation_advance; + let address_advance = if maximum_operations_per_instruction.0 == 1 { self.op_index.0 = 0; + minimum_instruction_length * operation_advance } else { let op_index_with_advance = self.op_index + operation_advance; - self.address += minimum_instruction_length - * (op_index_with_advance / maximum_operations_per_instruction); self.op_index = op_index_with_advance % maximum_operations_per_instruction; - } + minimum_instruction_length + * (op_index_with_advance / maximum_operations_per_instruction) + }; + self.address = self + .address + .checked_add(address_advance.0) + .ok_or(Error::AddressOverflow)?; + Ok(()) } #[inline] @@ -967,7 +986,11 @@ impl LineRow { } /// Section 6.2.5.1 - fn exec_special_opcode(&mut self, opcode: u8, header: &LineProgramHeader) { + fn exec_special_opcode( + &mut self, + opcode: u8, + header: &LineProgramHeader, + ) -> Result<()> { let adjusted_opcode = self.adjust_opcode(opcode, header); let line_range = header.line_encoding.line_range; @@ -979,7 +1002,8 @@ impl LineRow { self.apply_line_advance(line_base + i64::from(line_advance)); // Step 2 - self.apply_operation_advance(u64::from(operation_advance), header); + self.apply_operation_advance(u64::from(operation_advance), header)?; + Ok(()) } } @@ -2480,7 +2504,7 @@ mod tests { let mut program = IncompleteLineProgram { header }; let is_new_row = registers.execute(opcode, &mut program); - assert_eq!(is_new_row, expect_new_row); + assert_eq!(is_new_row, Ok(expect_new_row)); assert_eq!(registers, expected_registers); } @@ -2533,7 +2557,7 @@ mod tests { let opcode = LineInstruction::Special(52); let mut expected_registers = initial_registers; - expected_registers.address.0 += 3; + expected_registers.address += 3; assert_exec_opcode(header, initial_registers, opcode, expected_registers, true); } @@ -2547,7 +2571,7 @@ mod tests { let opcode = LineInstruction::Special(55); let mut expected_registers = initial_registers; - expected_registers.address.0 += 3; + expected_registers.address += 3; expected_registers.line.0 += 3; assert_exec_opcode(header, initial_registers, opcode, expected_registers, true); @@ -2563,7 +2587,7 @@ mod tests { let opcode = LineInstruction::Special(49); let mut expected_registers = initial_registers; - expected_registers.address.0 += 3; + expected_registers.address += 3; expected_registers.line.0 -= 3; assert_exec_opcode(header, initial_registers, opcode, expected_registers, true); @@ -2592,7 +2616,7 @@ mod tests { let header = make_test_header(EndianSlice::new(&[], LittleEndian)); let mut initial_registers = LineRow::new(&header); - initial_registers.address.0 = 1337; + initial_registers.address = 1337; initial_registers.line.0 = 42; let opcode = LineInstruction::Copy; @@ -2609,7 +2633,7 @@ mod tests { let opcode = LineInstruction::AdvancePc(42); let mut expected_registers = initial_registers; - expected_registers.address.0 += 42; + expected_registers.address += 42; assert_exec_opcode(header, initial_registers, opcode, expected_registers, false); } @@ -2617,15 +2641,12 @@ mod tests { #[test] fn test_exec_advance_pc_overflow() { let header = make_test_header(EndianSlice::new(&[], LittleEndian)); + let mut registers = LineRow::new(&header); + registers.address = u64::MAX; let opcode = LineInstruction::AdvancePc(42); - - let mut initial_registers = LineRow::new(&header); - initial_registers.address.0 = u64::MAX; - - let mut expected_registers = initial_registers; - expected_registers.address.0 = 41; - - assert_exec_opcode(header, initial_registers, opcode, expected_registers, false); + let mut program = IncompleteLineProgram { header }; + let result = registers.execute(opcode, &mut program); + assert_eq!(result, Err(Error::AddressOverflow)); } #[test] @@ -2758,11 +2779,22 @@ mod tests { let opcode = LineInstruction::ConstAddPc; let mut expected_registers = initial_registers; - expected_registers.address.0 += 20; + expected_registers.address += 20; assert_exec_opcode(header, initial_registers, opcode, expected_registers, false); } + #[test] + fn test_exec_const_add_pc_overflow() { + let header = make_test_header(EndianSlice::new(&[], LittleEndian)); + let mut registers = LineRow::new(&header); + registers.address = u64::MAX; + let opcode = LineInstruction::ConstAddPc; + let mut program = IncompleteLineProgram { header }; + let result = registers.execute(opcode, &mut program); + assert_eq!(result, Err(Error::AddressOverflow)); + } + #[test] fn test_exec_fixed_add_pc() { let header = make_test_header(EndianSlice::new(&[], LittleEndian)); @@ -2773,12 +2805,24 @@ mod tests { let opcode = LineInstruction::FixedAddPc(10); let mut expected_registers = initial_registers; - expected_registers.address.0 += 10; + expected_registers.address += 10; expected_registers.op_index.0 = 0; assert_exec_opcode(header, initial_registers, opcode, expected_registers, false); } + #[test] + fn test_exec_fixed_add_pc_overflow() { + let header = make_test_header(EndianSlice::new(&[], LittleEndian)); + let mut registers = LineRow::new(&header); + registers.address = u64::MAX; + registers.op_index.0 = 1; + let opcode = LineInstruction::FixedAddPc(10); + let mut program = IncompleteLineProgram { header }; + let result = registers.execute(opcode, &mut program); + assert_eq!(result, Err(Error::AddressOverflow)); + } + #[test] fn test_exec_set_prologue_end() { let header = make_test_header(EndianSlice::new(&[], LittleEndian)); @@ -2855,7 +2899,7 @@ mod tests { let opcode = LineInstruction::SetAddress(3030); let mut expected_registers = initial_registers; - expected_registers.address.0 = 3030; + expected_registers.address = 3030; assert_exec_opcode(header, initial_registers, opcode, expected_registers, false); } @@ -2868,11 +2912,22 @@ mod tests { let mut expected_registers = initial_registers; expected_registers.tombstone = true; - expected_registers.address.0 = !0; assert_exec_opcode(header, initial_registers, opcode, expected_registers, false); } + #[test] + fn test_exec_set_address_backwards() { + let header = make_test_header(EndianSlice::new(&[], LittleEndian)); + let mut registers = LineRow::new(&header); + registers.address = 1; + let opcode = LineInstruction::SetAddress(0); + + let mut program = IncompleteLineProgram { header }; + let result = registers.execute(opcode, &mut program); + assert_eq!(result, Err(Error::InvalidAddressRange)); + } + #[test] fn test_exec_define_file() { let mut program = make_test_program(EndianSlice::new(&[], LittleEndian)); @@ -2888,7 +2943,7 @@ mod tests { }; let opcode = LineInstruction::DefineFile(file); - let is_new_row = row.execute(opcode, &mut program); + let is_new_row = row.execute(opcode, &mut program).unwrap(); assert!(!is_new_row); assert_eq!(Some(&file), program.header().file_names.last()); diff --git a/src/write/line.rs b/src/write/line.rs index 02a507ff..7c47db81 100644 --- a/src/write/line.rs +++ b/src/write/line.rs @@ -1126,13 +1126,14 @@ mod convert { Some(val) => address = Some(val), None => return Err(ConvertError::InvalidAddress), } - from_row.execute(read::LineInstruction::SetAddress(0), &mut from_program); + from_row + .execute(read::LineInstruction::SetAddress(0), &mut from_program)?; } read::LineInstruction::DefineFile(_) => { return Err(ConvertError::UnsupportedLineInstruction); } _ => { - if from_row.execute(instruction, &mut from_program) { + if from_row.execute(instruction, &mut from_program)? { if !program.in_sequence() { program.begin_sequence(address); address = None;