Merged
fix(drivers): validate bus and chip transfer buffers #1335
liuxingsenAdd on 25 days ago
fix(drivers): validate bus and chip transfer buffers #1335
Merged
Total: 16 files changed+564-60
| @@ -41,6 +41,9 @@ std::vector<uint8_t> Interface_BitIO::Read(uint32_t offset, uint8_t length, uint | |||
| 41 | void Interface_BitIO::Write(uint32_t offset, uint8_t length, uint32_t mask, const std::vector<uint8_t>& indata) | 41 | void Interface_BitIO::Write(uint32_t offset, uint8_t length, uint32_t mask, const std::vector<uint8_t>& indata) |
| 42 | { | 42 | { |
| 43 | ensure_chip_accessible(m_chip, m_access_state, m_interface_chip); | 43 | ensure_chip_accessible(m_chip, m_access_state, m_interface_chip); |
| 44 | + if (length > indata.size()) { | ||
| 45 | + MC_THROW(mc::invalid_arg_exception, "BitIO Write length exceeds input buffer size"); | ||
| 46 | + } | ||
| 44 | std::vector<uint8_t> out_buffer; | 47 | std::vector<uint8_t> out_buffer; |
| 45 | BusLock lock(*m_chip->get_host_bus()); | 48 | BusLock lock(*m_chip->get_host_bus()); |
| 46 | std::string error; | 49 | std::string error; |
| @@ -123,6 +123,10 @@ uint32_t bus_can::write(ACCESS_DATA_S& input) | |||
| 123 | { | 123 | { |
| 124 | ensure_driver_ready(m_can_drv, "write", m_object_name, m_id); | 124 | ensure_driver_ready(m_can_drv, "write", m_object_name, m_id); |
| 125 | 125 | ||
| 126 | + if (input.length > input.in_buffer.size()) { | ||
| 127 | + MC_THROW(mc::invalid_op_exception, "CAN write length exceeds input buffer size"); | ||
| 128 | + } | ||
| 129 | + | ||
| 126 | try { | 130 | try { |
| 127 | m_can_drv->write(input.in_buffer.data(), input.length, input.write_time_out); | 131 | m_can_drv->write(input.in_buffer.data(), input.length, input.write_time_out); |
| 128 | return 0; | 132 | return 0; |
| @@ -110,6 +110,15 @@ void bus_i2c::bus_unlock() | |||
| 110 | 110 | ||
| 111 | std::string_view bus_i2c::format_tx_buf(const ACCESS_DATA_S& data) | 111 | std::string_view bus_i2c::format_tx_buf(const ACCESS_DATA_S& data) |
| 112 | { | 112 | { |
| 113 | + // On reads, data.length describes the reply, not the transmit buffer. | ||
| 114 | + return format_tx_buf(data, data.in_buffer.size()); | ||
| 115 | +} | ||
| 116 | + | ||
| 117 | +std::string_view bus_i2c::format_tx_buf(const ACCESS_DATA_S& data, size_t write_length) | ||
| 118 | +{ | ||
| 119 | + if (write_length > data.in_buffer.size()) { | ||
| 120 | + MC_THROW(mc::invalid_arg_exception, "write length exceeds input buffer size"); | ||
| 121 | + } | ||
| 113 | // 验证偏移量宽度 | 122 | // 验证偏移量宽度 |
| 114 | if (data.offset_width > MAX_OFFSET_WIDTH) { | 123 | if (data.offset_width > MAX_OFFSET_WIDTH) { |
| 115 | MC_THROW(mc::invalid_arg_exception, "offsetWidth must be 0~4"); | 124 | MC_THROW(mc::invalid_arg_exception, "offsetWidth must be 0~4"); |
| @@ -119,7 +128,7 @@ std::string_view bus_i2c::format_tx_buf(const ACCESS_DATA_S& data) | |||
| 119 | m_tx_buffer.clear(); | 128 | m_tx_buffer.clear(); |
| 120 | 129 | ||
| 121 | // 计算总长度：地址(1字节) + 偏移量(offset_width字节) + 数据长度 | 130 | // 计算总长度：地址(1字节) + 偏移量(offset_width字节) + 数据长度 |
| 122 | - size_t total_length = 1 + data.offset_width + data.length; | 131 | + size_t total_length = 1 + data.offset_width + write_length; |
| 123 | m_tx_buffer.reserve(total_length); | 132 | m_tx_buffer.reserve(total_length); |
| 124 | 133 | ||
| 125 | // 添加地址字节 | 134 | // 添加地址字节 |
| @@ -131,8 +140,8 @@ std::string_view bus_i2c::format_tx_buf(const ACCESS_DATA_S& data) | |||
| 131 | } | 140 | } |
| 132 | 141 | ||
| 133 | // 添加数据缓冲区内容 | 142 | // 添加数据缓冲区内容 |
| 134 | - if (!data.in_buffer.empty() && data.length > 0) { | 143 | + if (write_length > 0) { |
| 135 | - m_tx_buffer.append(reinterpret_cast<const char*>(data.in_buffer.data()), data.length); | 144 | + m_tx_buffer.append(reinterpret_cast<const char*>(data.in_buffer.data()), write_length); |
| 136 | } | 145 | } |
| 137 | 146 | ||
| 138 | return std::string_view(m_tx_buffer); | 147 | return std::string_view(m_tx_buffer); |
| @@ -260,7 +269,7 @@ uint32_t bus_i2c::write(ACCESS_DATA_S& data) | |||
| 260 | } | 269 | } |
| 261 | 270 | ||
| 262 | // 准备写入数据 | 271 | // 准备写入数据 |
| 263 | - const std::string_view tx_buf = format_tx_buf(data); | 272 | + const std::string_view tx_buf = format_tx_buf(data, data.length); |
| 264 | 273 | ||
| 265 | int32_t ret = 0; | 274 | int32_t ret = 0; |
| 266 | try { | 275 | try { |
| @@ -81,6 +81,8 @@ public: | |||
| 81 | #endif | 81 | #endif |
| 82 | 82 | ||
| 83 | private: | 83 | private: |
| 84 | + std::string_view format_tx_buf(const ACCESS_DATA_S& data, size_t write_length); | ||
| 85 | + | ||
| 84 | uint8_t m_id = 0; | 86 | uint8_t m_id = 0; |
| 85 | uint32_t m_speed = DEFAULT_SPEED; | 87 | uint32_t m_speed = DEFAULT_SPEED; |
| 86 | uint8_t m_mode = DEFAULT_MODE; | 88 | uint8_t m_mode = DEFAULT_MODE; |
| @@ -504,6 +504,9 @@ int32_t chip_base::data_access(uint32_t mask, uint32_t offset, uint32_t length, | |||
| 504 | ACCESS_DATA_S data = {}; | 504 | ACCESS_DATA_S data = {}; |
| 505 | m_last_access_error.clear(); | 505 | m_last_access_error.clear(); |
| 506 | try { | 506 | try { |
| 507 | + if (optype == CHIP_OP_TYPE_WRITE && length > in_buffer.size()) { | ||
| 508 | + MC_THROW(mc::invalid_arg_exception, "write length exceeds input buffer size"); | ||
| 509 | + } | ||
| 507 | data = pack_access_data(mask, offset, length, in_buffer, out_buffer, optype, protocol_flag); | 510 | data = pack_access_data(mask, offset, length, in_buffer, out_buffer, optype, protocol_flag); |
| 508 | result = input(data); | 511 | result = input(data); |
| 509 | 512 | ||
| @@ -519,7 +522,7 @@ int32_t chip_base::data_access(uint32_t mask, uint32_t offset, uint32_t length, | |||
| 519 | } | 522 | } |
| 520 | 523 | ||
| 521 | // 对于读取操作，将读取的数据移动回原始的 out_buffer | 524 | // 对于读取操作，将读取的数据移动回原始的 out_buffer |
| 522 | - if (optype == CHIP_OP_TYPE_READ && !data.out_buffer.empty()) { | 525 | + if (optype == CHIP_OP_TYPE_READ) { |
L | |||
| 523 | out_buffer = std::move(data.out_buffer); | 526 | out_buffer = std::move(data.out_buffer); |
| 524 | } | 527 | } |
| 525 | } catch (const std::exception& e) { | 528 | } catch (const std::exception& e) { |
| @@ -627,6 +630,15 @@ int32_t chip_base::bit_read(uint32_t offset, uint8_t length, uint32_t mask, std: | |||
| 627 | return ret; | 630 | return ret; |
| 628 | } | 631 | } |
| 629 | 632 | ||
| 633 | + // An empty reply must not be replaced by the caller's preallocated buffer. | ||
| 634 | + const size_t required_length = (length == 0 && mask != 0) ? 1 : length; | ||
| 635 | + if (out_buffer.size() < required_length) { | ||
| 636 | + error = "bit_read response too short"; | ||
| 637 | + record_error_history(error); | ||
| 638 | + out_buffer.clear(); | ||
| 639 | + return HAS_ERR; | ||
| 640 | + } | ||
| 641 | + | ||
| 630 | // 对读取的数据进行掩码处理。 | 642 | // 对读取的数据进行掩码处理。 |
| 631 | // 注意：has_shifted_bit_read=true 的总线（如 LocalBus）已在 HAL 层完成 | 643 | // 注意：has_shifted_bit_read=true 的总线（如 LocalBus）已在 HAL 层完成 |
| 632 | // (value & mask) >> ctz(mask) 的位域提取，返回的是移位后的字段值，此处无需重复处理； | 644 | // (value & mask) >> ctz(mask) 的位域提取，返回的是移位后的字段值，此处无需重复处理； |
| @@ -641,13 +653,16 @@ int32_t chip_base::bit_read(uint32_t offset, uint8_t length, uint32_t mask, std: | |||
| 641 | result_buffer.push_back(chip_base_get_value_with_mask(mask, data_buf[0])); | 653 | result_buffer.push_back(chip_base_get_value_with_mask(mask, data_buf[0])); |
| 642 | break; | 654 | break; |
| 643 | case sizeof(uint16_t): { | 655 | case sizeof(uint16_t): { |
| 644 | - uint16_t value = chip_base_get_value_with_mask(mask, *((uint16_t*)data_buf)); | 656 | + uint16_t raw = static_cast<uint16_t>(data_buf[0]) | (static_cast<uint16_t>(data_buf[1]) << 8); |
| 657 | + uint16_t value = chip_base_get_value_with_mask(mask, raw); | ||
| 645 | result_buffer.push_back(value & 0xFF); | 658 | result_buffer.push_back(value & 0xFF); |
| 646 | result_buffer.push_back(value >> 8); | 659 | result_buffer.push_back(value >> 8); |
| 647 | break; | 660 | break; |
| 648 | } | 661 | } |
| 649 | case sizeof(uint32_t): { | 662 | case sizeof(uint32_t): { |
| 650 | - uint32_t value = chip_base_get_value_with_mask(mask, *((uint32_t*)data_buf)); | 663 | + uint32_t raw = static_cast<uint32_t>(data_buf[0]) | (static_cast<uint32_t>(data_buf[1]) << 8) | |
| 664 | + (static_cast<uint32_t>(data_buf[2]) << 16) | (static_cast<uint32_t>(data_buf[3]) << 24); | ||
| 665 | + uint32_t value = chip_base_get_value_with_mask(mask, raw); | ||
| 651 | result_buffer.push_back(value & 0xFF); | 666 | result_buffer.push_back(value & 0xFF); |
| 652 | result_buffer.push_back((value >> 8) & 0xFF); | 667 | result_buffer.push_back((value >> 8) & 0xFF); |
| 653 | result_buffer.push_back((value >> 16) & 0xFF); | 668 | result_buffer.push_back((value >> 16) & 0xFF); |
| @@ -497,7 +497,10 @@ int32_t chip_canbus_chip::bit_write(uint32_t offset, uint8_t length, uint32_t ma | |||
| 497 | uint8_t cmd = static_cast<uint8_t>((offset >> 16) & 0xFF); | 497 | uint8_t cmd = static_cast<uint8_t>((offset >> 16) & 0xFF); |
| 498 | uint16_t sig_id = static_cast<uint16_t>(offset & 0xFFFF); | 498 | uint16_t sig_id = static_cast<uint16_t>(offset & 0xFFFF); |
| 499 | uint8_t pos = find_data_pos(length); | 499 | uint8_t pos = find_data_pos(length); |
| 500 | - uint8_t pad = pos - kEngcanSigIdLen; | 500 | + if (in_buffer.size() < length) { |
| 501 | + MC_THROW(mc::invalid_arg_exception, "CAN bit_write input buffer too short"); | ||
| 502 | + } | ||
| 503 | + uint8_t pad = pos - kEngcanSigIdLen; | ||
| 501 | 504 | ||
| 502 | // Build CAN ID | 505 | // Build CAN ID |
| 503 | uint32_t can_id = pack_can_id(0, 0x3F, 1, kEngcanCmdBase + cmd, m_address, m_protocol, kExtendedFrameType); | 506 | uint32_t can_id = pack_can_id(0, 0x3F, 1, kEngcanCmdBase + cmd, m_address, m_protocol, kExtendedFrameType); |
| @@ -123,6 +123,11 @@ int32_t chip_eeprom::block_read(uint32_t offset, uint32_t length, std::vector<ui | |||
| 123 | ", length=" + std::to_string(read_length) + ", error=" + err_str; | 123 | ", length=" + std::to_string(read_length) + ", error=" + err_str; |
| 124 | return ret; | 124 | return ret; |
| 125 | } | 125 | } |
| 126 | + if (read_buffer.size() < read_length) { | ||
| 127 | + error = m_object_name + " read failed: short reply at offset=" + std::to_string(read_offset) + | ||
| 128 | + ", length=" + std::to_string(read_length) + ", received=" + std::to_string(read_buffer.size()); | ||
| 129 | + return HAS_ERR; | ||
| 130 | + } | ||
| 126 | out_buffer.insert(out_buffer.end(), read_buffer.begin(), read_buffer.begin() + read_length); | 131 | out_buffer.insert(out_buffer.end(), read_buffer.begin(), read_buffer.begin() + read_length); |
| 127 | } | 132 | } |
| 128 | 133 | ||
| @@ -167,7 +167,12 @@ int32_t chip_smc::bit_read(uint32_t offset, uint8_t size, uint32_t mask, std::ve | |||
| 167 | para--; | 167 | para--; |
| 168 | } | 168 | } |
| 169 | 169 | ||
| 170 | - uint32_t data_length = read_buffer[SMC_READ_LEN_OFFSET] - SMC_EXTRA_LEN; | 170 | + const uint32_t response_length = read_buffer[SMC_READ_LEN_OFFSET]; |
| 171 | + if (response_length < SMC_EXTRA_LEN) { | ||
| 172 | + error = "response error, invalid smc response length"; | ||
| 173 | + return HAS_ERR; | ||
| 174 | + } | ||
| 175 | + const uint32_t data_length = response_length - SMC_EXTRA_LEN; | ||
| 171 | if (para * size + size > data_length) { | 176 | if (para * size + size > data_length) { |
| 172 | error = "request error, error para, para: " + std::to_string(para) + ", size: " + std::to_string(size) + | 177 | error = "request error, error para, para: " + std::to_string(para) + ", size: " + std::to_string(size) + |
| 173 | ", read_len: " + std::to_string(data_length); | 178 | ", read_len: " + std::to_string(data_length); |
| @@ -176,27 +181,14 @@ int32_t chip_smc::bit_read(uint32_t offset, uint8_t size, uint32_t mask, std::ve | |||
| 176 | 181 | ||
| 177 | uint8_t* dataBuf = read_buffer + SMC_READ_DATA_OFFSET + para * size; | 182 | uint8_t* dataBuf = read_buffer + SMC_READ_DATA_OFFSET + para * size; |
| 178 | 183 | ||
| 179 | - switch (size) { | 184 | + // SMC register payloads are little-endian byte sequences. |
| 180 | - case sizeof(uint8_t): | 185 | + uint32_t raw_value = 0; |
| 181 | - out_buffer.push_back(chip_smc_get_value_with_mask(mask, dataBuf[0])); | 186 | + for (uint8_t i = 0; i < size; ++i) { |
| 182 | - break; | 187 | + raw_value |= static_cast<uint32_t>(dataBuf[i]) << (BITS_PER_BYTE * i); |
| 183 | - case sizeof(uint16_t): { | 188 | + } |
| 184 | - uint16_t value = chip_smc_get_value_with_mask(mask, *((uint16_t*)dataBuf)); | 189 | + const uint32_t value = chip_smc_get_value_with_mask(mask, raw_value); |
| 185 | - out_buffer.push_back(value & 0xFF); | 190 | + for (uint8_t i = 0; i < size; ++i) { |
| 186 | - out_buffer.push_back(value >> 8); | 191 | + out_buffer.push_back(static_cast<uint8_t>(value >> (BITS_PER_BYTE * i))); |
| 187 | - break; | ||
| 188 | - } | ||
| 189 | - case sizeof(uint32_t): { | ||
| 190 | - uint32_t value = chip_smc_get_value_with_mask(mask, *((uint32_t*)dataBuf)); | ||
| 191 | - out_buffer.push_back(value & 0xFF); | ||
| 192 | - out_buffer.push_back((value >> 8) & 0xFF); | ||
| 193 | - out_buffer.push_back((value >> 16) & 0xFF); | ||
| 194 | - out_buffer.push_back(value >> 24); | ||
| 195 | - break; | ||
| 196 | - } | ||
| 197 | - default: | ||
| 198 | - // 早期校验已拒绝 size=0/3/>4，此处不可达 | ||
| 199 | - __builtin_unreachable(); | ||
| 200 | } | 192 | } |
| 201 | 193 | ||
| 202 | return HAS_OK; | 194 | return HAS_OK; |
| @@ -214,8 +206,14 @@ int32_t chip_smc::bit_write(uint32_t offset, uint8_t size, uint32_t mask, const | |||
| 214 | return HAS_OK; | 206 | return HAS_OK; |
| 215 | } | 207 | } |
| 216 | 208 | ||
| 217 | - uint32_t para = offset & SMC_PARA_MASK; | 209 | + if (in_buffer.size() < size) { |
| 218 | - if (para == 0 && ((1U << (BITS_PER_BYTE * size)) - 1) == mask) { | 210 | + error = "request error, bit_write input length is shorter than size"; |
| 211 | + return HAS_ERR; | ||
| 212 | + } | ||
| 213 | + | ||
| 214 | + uint32_t para = offset & SMC_PARA_MASK; | ||
| 215 | + const uint32_t full_mask = size == sizeof(uint32_t) ? UINT32_MAX : ((1U << (BITS_PER_BYTE * size)) - 1U); | ||
| 216 | + if (para == 0 && full_mask == mask) { | ||
| 219 | // mask 全为1时不需要读取硬件值, 直接写入硬件 | 217 | // mask 全为1时不需要读取硬件值, 直接写入硬件 |
| 220 | return write(offset, size, in_buffer.data(), error); | 218 | return write(offset, size, in_buffer.data(), error); |
| 221 | } | 219 | } |
| @@ -230,28 +228,27 @@ int32_t chip_smc::bit_write(uint32_t offset, uint8_t size, uint32_t mask, const | |||
| 230 | return ret; | 228 | return ret; |
| 231 | } | 229 | } |
| 232 | 230 | ||
| 233 | - uint32_t data_length = read_buffer[SMC_READ_LEN_OFFSET] - 1; | 231 | + const uint32_t response_length = read_buffer[SMC_READ_LEN_OFFSET]; |
| 232 | + if (response_length < SMC_EXTRA_LEN) { | ||
| 233 | + error = "response error, invalid smc response length"; | ||
| 234 | + return HAS_ERR; | ||
| 235 | + } | ||
| 236 | + const uint32_t data_length = response_length - SMC_EXTRA_LEN; | ||
| 234 | if (para * size + size > data_length) { | 237 | if (para * size + size > data_length) { |
| 235 | error = "bit_write para size overflow"; | 238 | error = "bit_write para size overflow"; |
| 236 | return HAS_ERR; | 239 | return HAS_ERR; |
| 237 | } | 240 | } |
| 238 | 241 | ||
| 239 | - uint8_t* dataBuf = read_buffer + SMC_READ_DATA_OFFSET + para * size; | 242 | + uint8_t* dataBuf = read_buffer + SMC_READ_DATA_OFFSET + para * size; |
| 240 | - switch (size) { | 243 | + uint32_t original = 0; |
| 241 | - case sizeof(uint8_t): | 244 | + uint32_t input = 0; |
| 242 | - *((uint8_t*)dataBuf) = chip_smc_set_value_with_mask(mask, *((uint8_t*)dataBuf), in_buffer[0]); | 245 | + for (uint8_t i = 0; i < size; ++i) { |
| 243 | - break; | 246 | + original |= static_cast<uint32_t>(dataBuf[i]) << (BITS_PER_BYTE * i); |
| 244 | - case sizeof(uint16_t): | 247 | + input |= static_cast<uint32_t>(in_buffer[i]) << (BITS_PER_BYTE * i); |
| 245 | - *((uint16_t*)dataBuf) = | 248 | + } |
| 246 | - chip_smc_set_value_with_mask(mask, *((uint16_t*)dataBuf), *((uint16_t*)in_buffer.data())); | 249 | + const uint32_t value = chip_smc_set_value_with_mask(mask, original, input); |
| 247 | - break; | 250 | + for (uint8_t i = 0; i < size; ++i) { |
| 248 | - case sizeof(uint32_t): | 251 | + dataBuf[i] = static_cast<uint8_t>(value >> (BITS_PER_BYTE * i)); |
| 249 | - *((uint32_t*)dataBuf) = | ||
| 250 | - chip_smc_set_value_with_mask(mask, *((uint32_t*)dataBuf), *((uint32_t*)in_buffer.data())); | ||
| 251 | - break; | ||
| 252 | - default: | ||
| 253 | - // 早期校验已拒绝 size=0/3/>4，此处不可达 | ||
| 254 | - __builtin_unreachable(); | ||
| 255 | } | 252 | } |
| 256 | 253 | ||
| 257 | return write(offset, size, dataBuf, error); | 254 | return write(offset, size, dataBuf, error); |
| @@ -446,11 +443,16 @@ uint32_t chip_smc::read_data(uint32_t read_length, uint8_t* out_buffer, uint8_t | |||
| 446 | return HAS_ERR; | 443 | return HAS_ERR; |
| 447 | } | 444 | } |
| 448 | 445 | ||
| 449 | - if (out_buffer_vec.at(SMC_RESP_RAW_CC_OFFSET) != SMC_CC_SUCCESS) { | 446 | + if (out_buffer_vec.size() <= SMC_RESP_RAW_CC_OFFSET) { |
| 450 | - if (out_buffer_vec.at(SMC_RESP_RAW_CC_OFFSET) == SMC_CC_NOT_READY) { | 447 | + error = "response error, smc response length is too short"; |
| 448 | + return HAS_ERR; | ||
| 449 | + } | ||
| 450 | + | ||
| 451 | + if (out_buffer_vec[SMC_RESP_RAW_CC_OFFSET] != SMC_CC_SUCCESS) { | ||
| 452 | + if (out_buffer_vec[SMC_RESP_RAW_CC_OFFSET] == SMC_CC_NOT_READY) { | ||
| 451 | continue; | 453 | continue; |
| 452 | } else { | 454 | } else { |
| 453 | - m_last_smc_cc = out_buffer_vec.at(SMC_RESP_RAW_CC_OFFSET); | 455 | + m_last_smc_cc = out_buffer_vec[SMC_RESP_RAW_CC_OFFSET]; |
| 454 | error = get_object_name() + " Smc read_data : request error, " + | 456 | error = get_object_name() + " Smc read_data : request error, " + |
| 455 | std::string(smc_cc_to_string(m_last_smc_cc)) + | 457 | std::string(smc_cc_to_string(m_last_smc_cc)) + |
| 456 | " , SMC completion code:" + std::to_string(m_last_smc_cc); | 458 | " , SMC completion code:" + std::to_string(m_last_smc_cc); |
| @@ -459,12 +461,23 @@ uint32_t chip_smc::read_data(uint32_t read_length, uint8_t* out_buffer, uint8_t | |||
| 459 | } | 461 | } |
| 460 | } | 462 | } |
| 461 | 463 | ||
| 462 | - if (static_cast<uint32_t>(out_buffer_vec.at(SMC_RESP_RAW_READ_LEN_OFFSET) + SMC_MIN_READ_LEN) > read_length) { | 464 | + const uint32_t response_length = out_buffer_vec[SMC_RESP_RAW_READ_LEN_OFFSET]; |
| 463 | - read_length = out_buffer_vec.at(SMC_RESP_RAW_READ_LEN_OFFSET) + SMC_MIN_READ_LEN; | 465 | + if (response_length < SMC_EXTRA_LEN) { |
| 466 | + error = "response error, invalid smc response length"; | ||
| 467 | + return HAS_ERR; | ||
| 468 | + } | ||
| 469 | + const uint32_t frame_length = response_length + SMC_MIN_READ_LEN; | ||
| 470 | + if (frame_length > read_length) { | ||
| 471 | + read_length = frame_length; | ||
| 464 | continue; | 472 | continue; |
| 465 | } | 473 | } |
| 466 | 474 | ||
| 467 | - out_buffer_vec.resize(out_buffer_vec.at(SMC_RESP_RAW_READ_LEN_OFFSET) + SMC_MIN_READ_LEN); | 475 | + if (out_buffer_vec.size() < frame_length) { |
| 476 | + error = "response error, smc response length exceeds received bytes"; | ||
| 477 | + return HAS_ERR; | ||
| 478 | + } | ||
| 479 | + | ||
| 480 | + out_buffer_vec.resize(frame_length); | ||
| 468 | 481 | ||
| 469 | // 倒序插入, 拼凑原始响应数据计算响应crc | 482 | // 倒序插入, 拼凑原始响应数据计算响应crc |
| 470 | out_buffer_vec.insert(out_buffer_vec.begin(), m_address | 1); | 483 | out_buffer_vec.insert(out_buffer_vec.begin(), m_address | 1); |
| @@ -473,8 +486,7 @@ uint32_t chip_smc::read_data(uint32_t read_length, uint8_t* out_buffer, uint8_t | |||
| 473 | 486 | ||
| 474 | // 构建crc_input时去掉最后一个字节的响应crc | 487 | // 构建crc_input时去掉最后一个字节的响应crc |
| 475 | std::string crc_input(out_buffer_vec.begin(), out_buffer_vec.end() - 1); | 488 | std::string crc_input(out_buffer_vec.begin(), out_buffer_vec.end() - 1); |
| 476 | - uint8_t real_length = out_buffer_vec.at(SMC_READ_LEN_OFFSET); | 489 | + uint8_t crc8 = mc::crypto::CRC8().calculate(crc_input); |
| 477 | - uint8_t crc8 = mc::crypto::CRC8().calculate(crc_input); | ||
| 478 | if (crc8 != out_buffer_vec.back()) { | 490 | if (crc8 != out_buffer_vec.back()) { |
| 479 | error = | 491 | error = |
| 480 | "crc mismatch, crc8: " + std::to_string(crc8) + ", expected: " + std::to_string(out_buffer_vec.back()); | 492 | "crc mismatch, crc8: " + std::to_string(crc8) + ", expected: " + std::to_string(out_buffer_vec.back()); |
| @@ -623,7 +635,13 @@ uint32_t chip_smc::forward_read(uint32_t length, std::vector<uint8_t>& data_out) | |||
| 623 | return ret; | 635 | return ret; |
| 624 | } | 636 | } |
| 625 | 637 | ||
| 626 | - uint32_t actual_length = std::min(length, static_cast<uint32_t>(out_buffer[SMC_READ_LEN_OFFSET] - 1U)); | 638 | + const uint32_t response_length = out_buffer[SMC_READ_LEN_OFFSET]; |
| 639 | + if (response_length < SMC_EXTRA_LEN || | ||
| 640 | + response_length - SMC_EXTRA_LEN > SMC_MAX_BUFFER_LEN - SMC_READ_DATA_OFFSET) { | ||
| 641 | + elog("forward_read failed: invalid smc response length"); | ||
| 642 | + return HAS_ERR; | ||
| 643 | + } | ||
| 644 | + const uint32_t actual_length = std::min(length, response_length - SMC_EXTRA_LEN); | ||
| 627 | data_out.resize(actual_length); | 645 | data_out.resize(actual_length); |
| 628 | for (uint32_t i = 0; i < actual_length; i++) { | 646 | for (uint32_t i = 0; i < actual_length; i++) { |
| 629 | data_out[i] = out_buffer[i + SMC_READ_DATA_OFFSET]; | 647 | data_out[i] = out_buffer[i + SMC_READ_DATA_OFFSET]; |
| @@ -685,10 +703,20 @@ int32_t chip_smc::forward_by_front_chip(ACCESS_DATA_S& data, const std::string& | |||
| 685 | return forward_read_mctp(data.length, data.out_buffer, error); | 703 | return forward_read_mctp(data.length, data.out_buffer, error); |
| 686 | } | 704 | } |
| 687 | 705 | ||
| 706 | + if (data.offset_width > sizeof(data.offset)) { | ||
| 707 | + elog("forward request has invalid offset width: ${width}", ("width", data.offset_width)); | ||
| 708 | + return HAS_ERR; | ||
| 709 | + } | ||
| 710 | + | ||
| 688 | if (data.optype == CHIP_OP_TYPE_WRITE) { | 711 | if (data.optype == CHIP_OP_TYPE_WRITE) { |
| 689 | uint8_t buffer[BUFFER_LEN] = {0}; | 712 | uint8_t buffer[BUFFER_LEN] = {0}; |
| 690 | uint8_t chip_type = is_smc ? FORWARD_TYPE_SMC : 0x00; | 713 | uint8_t chip_type = is_smc ? FORWARD_TYPE_SMC : 0x00; |
| 691 | 714 | ||
| 715 | + if (data.length > data.in_buffer.size() || data.length > BUFFER_LEN - data.offset_width) { | ||
| 716 | + elog("forward write length exceeds input or buffer capacity"); | ||
| 717 | + return HAS_ERR; | ||
| 718 | + } | ||
| 719 | + | ||
| 692 | uint32_t offset = calc_forward_opcode(data.addr, data.length, chip_type, bus_id); | 720 | uint32_t offset = calc_forward_opcode(data.addr, data.length, chip_type, bus_id); |
| 693 | uint32_t buffer_offset = 0; | 721 | uint32_t buffer_offset = 0; |
| 694 | for (uint32_t i = data.offset_width; i > 0; i--) { | 722 | for (uint32_t i = data.offset_width; i > 0; i--) { |
| @@ -109,6 +109,56 @@ ASAN 检查的分配边界，因此不能把此次行为回归写成已复现分 | |||
| 109 | 109 | ||
| 110 | 主题1、2、4、5、7仍会改变异常输入处理、数值解码或启停/销毁时序；没有单独升版不等于只有测试变化。主题8、9为测试和辅助统计工具，主题10为独立本地构建入口，其变化不修改正式服务接口或正式构建 recipe。 | 110 | 主题1、2、4、5、7仍会改变异常输入处理、数值解码或启停/销毁时序；没有单独升版不等于只有测试变化。主题8、9为测试和辅助统计工具，主题10为独立本地构建入口，其变化不修改正式服务接口或正式构建 recipe。 |
| 111 | 111 | ||
| 112 | +### EEPROM 多块读取：先检查实际回复长度再复制 | ||
| 113 | + | ||
| 114 | +对应 [!1335](https://gitcode.com/openUBMC/component_drivers/merge_requests/1335)，生产代码在 | ||
| 115 | +[chip_eeprom::block_read](../drivers/internal/chip/chip_eeprom/eeprom.cpp)，回归用例在 | ||
| 116 | +[test_chip_eeprom.cpp](drivers/internal/chip/test_chip_eeprom.cpp)。 | ||
| 117 | + | ||
| 118 | +通用 `chip_base::data_access` 现在把空读回复也传回调用方，避免预分配空间中的零值或旧数据 | ||
| 119 | +被误当作实际读取结果。EEPROM 多块循环原先只判断总线返回码，却没有检查每块回复的实际长度： | ||
| 120 | + | ||
| 121 | +```cpp | ||
| 122 | +// 修改前：data_access 返回 HAS_OK 后，直接按请求长度拼接。 | ||
| 123 | +out_buffer.insert(out_buffer.end(), read_buffer.begin(), read_buffer.begin() + read_length); | ||
| 124 | + | ||
| 125 | +// 修改后：在构造复制范围前拒绝空回复和非空短回复。 | ||
| 126 | +if (read_buffer.size() < read_length) { | ||
| 127 | + error = m_object_name + " read failed: short reply at offset=" + std::to_string(read_offset) + | ||
| 128 | + ", length=" + std::to_string(read_length) + ", received=" + std::to_string(read_buffer.size()); | ||
| 129 | + return HAS_ERR; | ||
| 130 | +} | ||
| 131 | +out_buffer.insert(out_buffer.end(), read_buffer.begin(), read_buffer.begin() + read_length); | ||
| 132 | +``` | ||
| 133 | + | ||
| 134 | +`std::vector<uint8_t>` 是 C++ 标准库的可变长字节数组；可以用 C 中的 | ||
| 135 | +`struct { uint8_t *data; size_t size; size_t capacity; }` 理解其存储信息。 | ||
| 136 | +`size()` 是有效字节数，不是分配容量。`data_access` 的 | ||
| 137 | +`out_buffer = std::move(data.out_buffer)` 把回复数组的存储所有权和有效长度交给调用方， | ||
| 138 | +不是向原先固定大小的数组中复制；因此先前预分配了 32 字节，不代表返回后仍有 32 个有效字节。 | ||
| 139 | +这是 `std` 标准容器的移动语义，与项目的 `mc` 智能指针无关。 | ||
| 140 | + | ||
| 141 | +`begin()` 可类比指向首字节的 C 指针；`begin() + read_length` 是复制结束位置， | ||
| 142 | +`insert` 使用左闭右开的范围追加数据。若只收到 3 字节，却构造 4 字节范围，效果类似 | ||
| 143 | +让 C 的 `memcpy` 从有效长度为 3 的缓冲区读取 4 字节；容量恰好够大也不能使这个范围合法。 | ||
| 144 | +这属于未定义行为，外层 `try/catch` 不能保证接住。新增检查相当于 C 中先判断 | ||
| 145 | +`received < requested` 再返回错误，确保复制路径只接收足够长的回复。 | ||
| 146 | + | ||
| 147 | +失败时保留此前完整块，当前短块不追加，也不再读取后续块；调用方必须检查 `HAS_ERR`， | ||
| 148 | +不能把已完成的前缀当作完整结果。错误文本包含当前块的偏移、请求长度和实际长度，便于定位设备回复。 | ||
| 149 | +本次防护针对多块拼接的固定长度复制；单块路径仍直接传回回复，由已有上层解析器检查其长度。 | ||
| 150 | + | ||
| 151 | +`FirstMiddleAndLast/chip_eeprom_short_fragment.RejectsIncompleteReplyBeforeAppending` 的六组参数 | ||
| 152 | +覆盖 100 字节请求中首块、中间块、末块的空回复，以及分别少 1 字节的回复： | ||
| 153 | +`(offset, received) = (0,0), (0,31), (32,0), (32,31), (96,0), (96,3)`。 | ||
| 154 | +测试沿真实 `block_read → data_access` 路径执行，仅以既有总线替身代替硬件；同时断言错误返回、 | ||
| 155 | +停止后续读请求、输出仅保留完整前缀和错误上下文。已有成功用例验证整块、末块和非对齐起点的正常数据。 | ||
| 156 | + | ||
| 157 | +2026-10-14 修复前验证中，末块短回复用例明确失败：实际返回 `HAS_OK`，并追加了不完整块； | ||
| 158 | +首块空回复在 GDB 下停于 `block_read → vector::_M_range_insert → memcpy` 的 SIGSEGV。 | ||
| 159 | +调试器自身退出码为 0 不能作为测试通过依据。本地使用普通宿主构建，未加入 sanitizer 插桩； | ||
| 160 | +修复后执行结果以对应提交的 PR 自验证报告及原始日志为准，不据此宣称已完成目标板或全仓 ASan 验证。 | ||
| 161 | + | ||
| 112 | ### 从 C 的资源清理顺序理解生命周期修改 | 162 | ### 从 C 的资源清理顺序理解生命周期修改 |
| 113 | 163 | ||
| 114 | 以下片段用于解释上述 PR，不能理解为本文所在分支已经包含其他九项源码。 | 164 | 以下片段用于解释上述 PR，不能理解为本文所在分支已经包含其他九项源码。 |
| @@ -2499,6 +2499,26 @@ TEST_F(InterfaceBitIOTest, Read_success_without_interface_chip_no_callback) | |||
| 2499 | 2499 | ||
| 2500 | // ===== Write: bit_write 成功路径 ===== | 2500 | // ===== Write: bit_write 成功路径 ===== |
| 2501 | 2501 | ||
| 2502 | +TEST_F(InterfaceBitIOTest, memory_safety_write_rejects_short_input) | ||
| 2503 | +{ | ||
| 2504 | + auto chip = std::make_shared<mock_chip_for_bitio>(); | ||
| 2505 | + m_bitio->m_chip = chip.get(); | ||
| 2506 | + for (uint8_t length : {2, 4, 255}) { | ||
| 2507 | + SCOPED_TRACE(+length); | ||
| 2508 | + EXPECT_THROW(m_bitio->Write(0x20, length, 0xffffffffU, {0x5a}), mc::invalid_arg_exception); | ||
| 2509 | + EXPECT_EQ(chip->get_last_write_offset(), 0U); | ||
| 2510 | + } | ||
| 2511 | +} | ||
| 2512 | + | ||
| 2513 | +TEST_F(InterfaceBitIOTest, memory_safety_write_preserves_prefix_length) | ||
| 2514 | +{ | ||
| 2515 | + auto chip = std::make_shared<mock_chip_for_bitio>(); | ||
| 2516 | + m_bitio->m_chip = chip.get(); | ||
| 2517 | + EXPECT_NO_THROW(m_bitio->Write(0x20, 1, 0xff, {0x34, 0x56})); | ||
| 2518 | + EXPECT_EQ(chip->get_last_write_offset(), 0x20U); | ||
| 2519 | + EXPECT_EQ(chip->get_last_write_length(), 1U); | ||
| 2520 | +} | ||
| 2521 | + | ||
| 2502 | TEST_F(InterfaceBitIOTest, Write_success) | 2522 | TEST_F(InterfaceBitIOTest, Write_success) |
| 2503 | { | 2523 | { |
| 2504 | auto mock_chip = std::make_shared<mock_chip_for_bitio>(); | 2524 | auto mock_chip = std::make_shared<mock_chip_for_bitio>(); |
| @@ -386,4 +386,30 @@ TEST_F(BusCanTest, extend_read_invalid_read_length_throws) | |||
| 386 | } | 386 | } |
| 387 | } | 387 | } |
| 388 | 388 | ||
| 389 | +TEST_F(BusCanTest, memory_safety_write_rejects_oversized_source_length) | ||
| 390 | +{ | ||
| 391 | + mc::mutable_dict csr; | ||
| 392 | + ASSERT_TRUE(m_bus.init(csr)); | ||
| 393 | + for (uint32_t length : {2U, 4U, 255U}) { | ||
| 394 | + SCOPED_TRACE(length); | ||
| 395 | + ACCESS_DATA_S data{}; | ||
| 396 | + data.in_buffer = {0x5a}; | ||
| 397 | + data.length = length; | ||
| 398 | + EXPECT_THROW(m_bus.write(data), mc::invalid_op_exception); | ||
| 399 | + EXPECT_EQ(DRIVER_CANBUS::test_state().last_write_len, 0U); | ||
| 400 | + DRIVER_CANBUS::reset_test_state(); | ||
| 401 | + } | ||
| 402 | +} | ||
| 403 | + | ||
| 404 | +TEST_F(BusCanTest, memory_safety_write_preserves_explicit_prefix_length) | ||
| 405 | +{ | ||
| 406 | + mc::mutable_dict csr; | ||
| 407 | + ASSERT_TRUE(m_bus.init(csr)); | ||
| 408 | + ACCESS_DATA_S data{}; | ||
| 409 | + data.in_buffer = {0x34, 0x56}; | ||
| 410 | + data.length = 1; | ||
| 411 | + EXPECT_EQ(m_bus.write(data), HAS_OK); | ||
| 412 | + EXPECT_EQ(DRIVER_CANBUS::test_state().last_write_len, 1U); | ||
| 413 | +} | ||
| 414 | + | ||
| 389 | } // namespace | 415 | } // namespace |
| @@ -619,4 +619,49 @@ TEST_F(bus_i2c_test, init_skips_iocfg_when_multiio_missing) | |||
| 619 | EXPECT_EQ(DRIVER_COMM::comm_test_state().iocfg_config_count, 0); | 619 | EXPECT_EQ(DRIVER_COMM::comm_test_state().iocfg_config_count, 0); |
| 620 | } | 620 | } |
| 621 | 621 | ||
| 622 | +// The transmit source length is independent of the requested reply length. | ||
| 623 | +TEST_F(bus_i2c_test, memory_safety_read_uses_actual_input_size) | ||
| 624 | +{ | ||
| 625 | + ACCESS_DATA_S data{}; | ||
| 626 | + data.addr = 0x70; | ||
| 627 | + data.offset_width = 1; | ||
| 628 | + data.offset = 0x12; | ||
| 629 | + data.in_buffer = {0x34, 0x56}; | ||
| 630 | + for (uint32_t read_length : {0U, 1U, 2U, 4U, 255U}) { | ||
| 631 | + SCOPED_TRACE(read_length); | ||
| 632 | + data.length = read_length; | ||
| 633 | + EXPECT_EQ(std::string(m_bus.format_tx_buf(data)), std::string("\x70\x12\x34\x56", 4)); | ||
| 634 | + } | ||
| 635 | + data.in_buffer.clear(); | ||
| 636 | + EXPECT_EQ(std::string(m_bus.format_tx_buf(data)), std::string("\x70\x12", 2)); | ||
| 637 | +} | ||
| 638 | + | ||
| 639 | +TEST_F(bus_i2c_test, memory_safety_write_rejects_oversized_source_length) | ||
| 640 | +{ | ||
| 641 | + mc::mutable_dict csr; | ||
| 642 | + ASSERT_TRUE(m_bus.init(csr)); | ||
| 643 | + for (uint32_t length : {2U, 4U, 255U}) { | ||
| 644 | + SCOPED_TRACE(length); | ||
| 645 | + ACCESS_DATA_S data{}; | ||
| 646 | + data.addr = 0x70; | ||
| 647 | + data.length = length; | ||
| 648 | + data.in_buffer = {0x5a}; | ||
| 649 | + EXPECT_THROW(m_bus.write(data), mc::invalid_arg_exception); | ||
| 650 | + } | ||
| 651 | +} | ||
| 652 | + | ||
| 653 | +TEST_F(bus_i2c_test, memory_safety_write_preserves_explicit_prefix_length) | ||
| 654 | +{ | ||
| 655 | + mc::mutable_dict csr; | ||
| 656 | + ASSERT_TRUE(m_bus.init(csr)); | ||
| 657 | + ACCESS_DATA_S data{}; | ||
| 658 | + data.addr = 0x70; | ||
| 659 | + data.offset_width = 1; | ||
| 660 | + data.offset = 0x12; | ||
| 661 | + data.length = 1; | ||
| 662 | + data.in_buffer = {0x34, 0x56}; | ||
| 663 | + ASSERT_EQ(m_bus.write(data), HAS_OK); | ||
| 664 | + EXPECT_EQ(m_bus.m_tx_buffer, std::string("\x70\x12\x34", 3)); | ||
| 665 | +} | ||
| 666 | + | ||
| 622 | } // namespace | 667 | } // namespace |
| @@ -895,6 +895,20 @@ TEST_F(chip_canbus_chip_can_bus_test, bit_read_wrong_bus_type_returns_err) | |||
| 895 | // bit_write (offset: bits 31-16 = cmd, bits 15-0 = sig_id) | 895 | // bit_write (offset: bits 31-16 = cmd, bits 15-0 = sig_id) |
| 896 | // =========================================================================== | 896 | // =========================================================================== |
| 897 | 897 | ||
| 898 | +TEST_F(chip_canbus_chip_can_bus_test, memory_safety_bit_write_rejects_short_input) | ||
| 899 | +{ | ||
| 900 | + auto bus = attach_can_bus(); | ||
| 901 | + for (uint8_t length : {1, 2, 4, 6}) { | ||
| 902 | + for (size_t size = 0; size < length; ++size) { | ||
| 903 | + SCOPED_TRACE(::testing::Message() << +length << "/" << size); | ||
| 904 | + std::vector<uint8_t> input(size, 0x5a), output; | ||
| 905 | + std::string error; | ||
| 906 | + EXPECT_EQ(m_chip->bit_write(0x00011234, length, 0xff, input, output, error), HAS_ERR); | ||
| 907 | + EXPECT_FALSE(error.empty()); | ||
| 908 | + } | ||
| 909 | + } | ||
| 910 | +} | ||
| 911 | + | ||
| 898 | TEST_F(chip_canbus_chip_can_bus_test, bit_write_length2_success) | 912 | TEST_F(chip_canbus_chip_can_bus_test, bit_write_length2_success) |
| 899 | { | 913 | { |
| 900 | auto bus = attach_can_bus(); | 914 | auto bus = attach_can_bus(); |
| @@ -1067,4 +1067,60 @@ TEST_F(chip_topo_path_test, topo_path_different_addr_are_different) | |||
| 1067 | EXPECT_NE(m_chip1->get_topo_path(), m_chip3->get_topo_path()); | 1067 | EXPECT_NE(m_chip1->get_topo_path(), m_chip3->get_topo_path()); |
| 1068 | } | 1068 | } |
| 1069 | 1069 | ||
| 1070 | +TEST_F(chip_base_test, memory_safety_bit_read_rejects_every_short_reply) | ||
| 1071 | +{ | ||
| 1072 | + m_chip->set_left_bus(m_bus); | ||
| 1073 | + m_chip->set_host_bus(m_bus); | ||
| 1074 | + for (uint8_t width : {1, 2, 4}) { | ||
| 1075 | + for (size_t received = 0; received < width; ++received) { | ||
| 1076 | + for (uint32_t mask : {0U, 0xffffffffU}) { | ||
| 1077 | + SCOPED_TRACE(::testing::Message() << +width << "/" << received << "/" << mask); | ||
| 1078 | + m_bus->add_read_payload(std::vector<uint8_t>(received, 0x5a)); | ||
| 1079 | + std::vector<uint8_t> input; | ||
| 1080 | + // A preallocated caller buffer must not disguise an empty reply. | ||
| 1081 | + std::vector<uint8_t> output(width, 0xff); | ||
| 1082 | + std::string error; | ||
| 1083 | + EXPECT_EQ(m_chip->bit_read(0x10, width, mask, input, output, error), HAS_ERR); | ||
| 1084 | + EXPECT_FALSE(error.empty()); | ||
| 1085 | + } | ||
| 1086 | + } | ||
| 1087 | + } | ||
| 1088 | +} | ||
| 1089 | + | ||
| 1090 | +TEST_F(chip_base_test, memory_safety_shifted_bus_preserves_value_and_rejects_short_reply) | ||
| 1091 | +{ | ||
| 1092 | + m_chip->set_left_bus(m_bus); | ||
| 1093 | + m_chip->set_host_bus(m_bus); | ||
| 1094 | + m_bus->has_shifted_bit_read = true; | ||
| 1095 | + std::vector<uint8_t> input, output; | ||
| 1096 | + std::string error; | ||
| 1097 | + | ||
| 1098 | + // LocalBus has already extracted (register & 0xf0) >> 4. | ||
| 1099 | + m_bus->add_read_payload({3, 0, 0, 0}); | ||
| 1100 | + ASSERT_EQ(m_chip->bit_read(0x10, 4, 0xf0, input, output, error), HAS_OK); | ||
| 1101 | + EXPECT_EQ(output, (std::vector<uint8_t>{3, 0, 0, 0})); | ||
| 1102 | + | ||
| 1103 | + // A shifted response must still contain the requested number of bytes. | ||
| 1104 | + m_bus->add_read_payload({3}); | ||
| 1105 | + EXPECT_EQ(m_chip->bit_read(0x10, 4, 0xf0, input, output, error), HAS_ERR); | ||
| 1106 | + EXPECT_TRUE(output.empty()); | ||
| 1107 | + EXPECT_FALSE(error.empty()); | ||
| 1108 | +} | ||
| 1109 | + | ||
| 1110 | +TEST_F(chip_base_test, memory_safety_bit_write_rejects_short_input_before_bus) | ||
| 1111 | +{ | ||
| 1112 | + m_chip->set_left_bus(m_bus); | ||
| 1113 | + m_chip->set_host_bus(m_bus); | ||
| 1114 | + for (uint8_t width : {1, 2, 4}) { | ||
| 1115 | + for (size_t provided = 0; provided < width; ++provided) { | ||
| 1116 | + SCOPED_TRACE(::testing::Message() << +width << "/" << provided); | ||
| 1117 | + std::vector<uint8_t> input(provided, 0x5a), output; | ||
| 1118 | + std::string error; | ||
| 1119 | + EXPECT_EQ(m_chip->bit_write(0x10, width, 0xffffffffU, input, output, error), HAS_ERR); | ||
| 1120 | + EXPECT_FALSE(error.empty()); | ||
| 1121 | + EXPECT_EQ(m_bus->m_write_call_count, 0); | ||
| 1122 | + } | ||
| 1123 | + } | ||
| 1124 | +} | ||
| 1125 | + | ||
| 1070 | } // namespace | 1126 | } // namespace |
| @@ -25,6 +25,7 @@ | |||
| 25 | #include <limits> | 25 | #include <limits> |
| 26 | #include <memory> | 26 | #include <memory> |
| 27 | #include <string> | 27 | #include <string> |
| 28 | +#include <utility> | ||
| 28 | #include <vector> | 29 | #include <vector> |
| 29 | 30 | ||
| 30 | #define private public | 31 | #define private public |
| @@ -511,6 +512,33 @@ TEST(chip_eeprom_block_read, MultiFragmentMidLoopFailure) | |||
| 511 | EXPECT_FALSE(err.empty()); | 512 | EXPECT_FALSE(err.empty()); |
| 512 | } | 513 | } |
| 513 | 514 | ||
| 515 | +class chip_eeprom_short_fragment : public testing::TestWithParam<std::pair<uint32_t, uint32_t>> {}; | ||
| 516 | + | ||
| 517 | +TEST_P(chip_eeprom_short_fragment, RejectsIncompleteReplyBeforeAppending) | ||
| 518 | +{ | ||
| 519 | + const auto [bad_offset, reply_length] = GetParam(); | ||
| 520 | + const uint32_t requested_length = bad_offset == 96 ? 4 : 32; | ||
| 521 | + auto chip = make_chip(); | ||
| 522 | + // The image ends within this fragment. The real data_access path receives HAS_OK | ||
| 523 | + // with an empty or short reply; any earlier fragments contain complete data. | ||
| 524 | + auto bus = attach_bus(chip, std::vector<uint8_t>(bad_offset + reply_length, 0xA5)); | ||
| 525 | + std::vector<uint8_t> in; | ||
| 526 | + std::vector<uint8_t> out; | ||
| 527 | + std::string error; | ||
| 528 | + | ||
| 529 | + EXPECT_EQ(chip->block_read(0, 100, in, out, error), HAS_ERR); | ||
| 530 | + EXPECT_EQ(bus->read_calls, bad_offset / 32 + 1); | ||
| 531 | + EXPECT_EQ(out, std::vector<uint8_t>(bad_offset, 0xA5)); | ||
| 532 | + EXPECT_NE(error.find("short reply"), std::string::npos); | ||
| 533 | + EXPECT_NE(error.find("offset=" + std::to_string(bad_offset)), std::string::npos); | ||
| 534 | + EXPECT_NE(error.find("length=" + std::to_string(requested_length)), std::string::npos); | ||
| 535 | + EXPECT_NE(error.find("received=" + std::to_string(reply_length)), std::string::npos); | ||
| 536 | +} | ||
| 537 | + | ||
| 538 | +INSTANTIATE_TEST_SUITE_P(FirstMiddleAndLast, chip_eeprom_short_fragment, | ||
| 539 | + testing::Values(std::make_pair(0U, 0U), std::make_pair(0U, 31U), std::make_pair(32U, 0U), | ||
| 540 | + std::make_pair(32U, 31U), std::make_pair(96U, 0U), std::make_pair(96U, 3U))); | ||
| 541 | + | ||
| 514 | // ============================================================================ | 542 | // ============================================================================ |
| 515 | // 测试组三：block_write | 543 | // 测试组三：block_write |
| 516 | // ============================================================================ | 544 | // ============================================================================ |
| @@ -320,12 +320,13 @@ static std::vector<uint8_t> smc_read_frame(uint8_t addr, uint8_t cmd, const std: | |||
| 320 | class chip_smc_exposed : public dev::chip_smc { | 320 | class chip_smc_exposed : public dev::chip_smc { |
| 321 | public: | 321 | public: |
| 322 | using dev::chip_smc::forward_by_front_chip; | 322 | using dev::chip_smc::forward_by_front_chip; |
| 323 | + using dev::chip_smc::forward_read; | ||
| 323 | }; | 324 | }; |
| 324 | 325 | ||
| 325 | -static std::shared_ptr<smc_test_bus> init_exposed_chip(chip_smc_exposed& chip) | 326 | +static std::shared_ptr<smc_test_bus> init_exposed_chip(chip_smc_exposed& chip, uint8_t address = 0x10) |
| 326 | { | 327 | { |
| 327 | mc::mutable_dict csr; | 328 | mc::mutable_dict csr; |
| 328 | - csr["Address"] = static_cast<uint32_t>(0x10); | 329 | + csr["Address"] = static_cast<uint32_t>(address); |
| 329 | csr["OffsetWidth"] = static_cast<uint8_t>(1); | 330 | csr["OffsetWidth"] = static_cast<uint8_t>(1); |
| 330 | csr["AddrWidth"] = static_cast<uint8_t>(1); | 331 | csr["AddrWidth"] = static_cast<uint8_t>(1); |
| 331 | csr["object_name"] = std::string("smc_test"); | 332 | csr["object_name"] = std::string("smc_test"); |
| @@ -408,6 +409,32 @@ TEST_F(chip_smc_test, bit_read_para_overflow_error) | |||
| 408 | EXPECT_NE(error.find("para"), std::string::npos); | 409 | EXPECT_NE(error.find("para"), std::string::npos); |
| 409 | } | 410 | } |
| 410 | 411 | ||
| 412 | +TEST_F(chip_smc_test, bit_read_rejects_zero_response_length_before_offset) | ||
| 413 | +{ | ||
| 414 | + auto bus = bind_test_bus(); | ||
| 415 | + bus->enqueue_read_payload({0x00, 0x00}); | ||
| 416 | + | ||
| 417 | + std::vector<uint8_t> in, out; | ||
| 418 | + std::string error; | ||
| 419 | + EXPECT_EQ(m_chip.bit_read(0x21FF, 1, 0xFF, in, out, error), HAS_ERR); | ||
| 420 | + EXPECT_NE(error.find("length"), std::string::npos); | ||
| 421 | + EXPECT_TRUE(out.empty()); | ||
| 422 | +} | ||
| 423 | + | ||
| 424 | +TEST_F(chip_smc_test, bit_read_rejects_truncated_response) | ||
| 425 | +{ | ||
| 426 | + auto bus = bind_test_bus(); | ||
| 427 | + // The old parser padded the missing data/CRC with zero; that padded | ||
| 428 | + // frame has CRC8 == 0 and was accepted despite missing received bytes. | ||
| 429 | + bus->enqueue_read_payload({0x03, 0x00, 0x1C}); | ||
| 430 | + | ||
| 431 | + std::vector<uint8_t> in, out; | ||
| 432 | + std::string error; | ||
| 433 | + EXPECT_EQ(m_chip.bit_read(0x2100, 1, 0xFF, in, out, error), HAS_ERR); | ||
| 434 | + EXPECT_NE(error.find("length"), std::string::npos); | ||
| 435 | + EXPECT_TRUE(out.empty()); | ||
| 436 | +} | ||
| 437 | + | ||
| 411 | TEST_F(chip_smc_test, bit_read_uint16) | 438 | TEST_F(chip_smc_test, bit_read_uint16) |
| 412 | { | 439 | { |
| 413 | auto bus = bind_test_bus(); | 440 | auto bus = bind_test_bus(); |
| @@ -430,6 +457,21 @@ TEST_F(chip_smc_test, bit_read_uint32) | |||
| 430 | EXPECT_EQ(out, (std::vector<uint8_t>{0x78, 0x56, 0x34, 0x12})); | 457 | EXPECT_EQ(out, (std::vector<uint8_t>{0x78, 0x56, 0x34, 0x12})); |
| 431 | } | 458 | } |
| 432 | 459 | ||
| 460 | +TEST_F(chip_smc_test, bit_read_accepts_last_byte_of_maximum_payload) | ||
| 461 | +{ | ||
| 462 | + auto bus = bind_test_bus(); | ||
| 463 | + std::vector<uint8_t> payload(254, 0x00); | ||
| 464 | + payload.back() = 0xA5; | ||
| 465 | + auto frame = smc_read_frame(0x10, 0x21, payload); | ||
| 466 | + bus->enqueue_read_payload(frame); | ||
| 467 | + bus->enqueue_read_payload(std::move(frame)); | ||
| 468 | + | ||
| 469 | + std::vector<uint8_t> in, out; | ||
| 470 | + std::string error; | ||
| 471 | + EXPECT_EQ(m_chip.bit_read(0x21FE, 1, 0xFF, in, out, error), HAS_OK); | ||
| 472 | + EXPECT_EQ(out, (std::vector<uint8_t>{0xA5})); | ||
| 473 | +} | ||
| 474 | + | ||
| 433 | // ---- bit_write：读-改-写分支 ---- | 475 | // ---- bit_write：读-改-写分支 ---- |
| 434 | 476 | ||
| 435 | TEST_F(chip_smc_test, bit_write_rejects_invalid_lengths) | 477 | TEST_F(chip_smc_test, bit_write_rejects_invalid_lengths) |
| @@ -442,6 +484,60 @@ TEST_F(chip_smc_test, bit_write_rejects_invalid_lengths) | |||
| 442 | } | 484 | } |
| 443 | } | 485 | } |
| 444 | 486 | ||
| 487 | +TEST_F(chip_smc_test, bit_write_rejects_short_input_before_bus_access) | ||
| 488 | +{ | ||
| 489 | + auto bus = bind_test_bus(); | ||
| 490 | + for (uint8_t width : {1, 2, 4}) { | ||
| 491 | + for (size_t supplied = 0; supplied < width; ++supplied) { | ||
| 492 | + for (uint32_t mask : {0x0FU, 0xFFFFFFFFU}) { | ||
| 493 | + SCOPED_TRACE(::testing::Message() << +width << "/" << supplied << "/" << mask); | ||
| 494 | + std::vector<uint8_t> in(supplied, 0x12), out; | ||
| 495 | + std::string error; | ||
| 496 | + EXPECT_EQ(m_chip.bit_write(0x2100, width, mask, in, out, error), HAS_ERR); | ||
| 497 | + EXPECT_NE(error.find("input"), std::string::npos); | ||
| 498 | + } | ||
| 499 | + } | ||
| 500 | + } | ||
| 501 | + EXPECT_EQ(bus->write_call_count(), 0); | ||
| 502 | + EXPECT_EQ(bus->read_call_count(), 0); | ||
| 503 | +} | ||
| 504 | + | ||
| 505 | +TEST_F(chip_smc_test, bit_write_full_uint32_mask_writes_without_read) | ||
| 506 | +{ | ||
| 507 | + auto bus = bind_test_bus(); | ||
| 508 | + std::vector<uint8_t> in = {0x78, 0x56, 0x34, 0x12}; | ||
| 509 | + std::vector<uint8_t> out; | ||
| 510 | + std::string error; | ||
| 511 | + EXPECT_EQ(m_chip.bit_write(0x2100, 4, 0xFFFFFFFF, in, out, error), HAS_OK); | ||
| 512 | + EXPECT_EQ(bus->read_call_count(), 0); | ||
| 513 | + ASSERT_GE(bus->last_write_buffer().size(), 9U); | ||
| 514 | + const auto& frame = bus->last_write_buffer(); | ||
| 515 | + EXPECT_EQ((std::vector<uint8_t>(frame.end() - 5, frame.end() - 1)), in); | ||
| 516 | +} | ||
| 517 | + | ||
| 518 | +TEST_F(chip_smc_test, bit_write_rejects_zero_response_length_before_offset) | ||
| 519 | +{ | ||
| 520 | + auto bus = bind_test_bus(); | ||
| 521 | + bus->enqueue_read_payload({0x00, 0x00}); | ||
| 522 | + | ||
| 523 | + std::vector<uint8_t> in = {0x12}; | ||
| 524 | + std::vector<uint8_t> out; | ||
| 525 | + std::string error; | ||
| 526 | + EXPECT_EQ(m_chip.bit_write(0x21FF, 1, 0x0F, in, out, error), HAS_ERR); | ||
| 527 | + EXPECT_NE(error.find("length"), std::string::npos); | ||
| 528 | +} | ||
| 529 | + | ||
| 530 | +TEST_F(chip_smc_test, bit_write_rejects_truncated_response_with_valid_padded_crc) | ||
| 531 | +{ | ||
| 532 | + auto bus = bind_test_bus(); | ||
| 533 | + bus->enqueue_read_payload({0x03, 0x00, 0x1C}); | ||
| 534 | + std::vector<uint8_t> input = {0x12}, output; | ||
| 535 | + std::string error; | ||
| 536 | + EXPECT_EQ(m_chip.bit_write(0x2100, 1, 0x0F, input, output, error), HAS_ERR); | ||
| 537 | + // One command starts the read; no register write may follow the short reply. | ||
| 538 | + EXPECT_EQ(bus->write_call_count(), 1); | ||
| 539 | +} | ||
| 540 | + | ||
| 445 | TEST_F(chip_smc_test, bit_write_read_modify_write_byte) | 541 | TEST_F(chip_smc_test, bit_write_read_modify_write_byte) |
| 446 | { | 542 | { |
| 447 | auto bus = bind_test_bus(); | 543 | auto bus = bind_test_bus(); |
| @@ -816,6 +912,106 @@ TEST(chip_smc_forward, forward_by_front_chip_smc_write) | |||
| 816 | EXPECT_EQ(chip.forward_by_front_chip(d, "smc", 1), HAS_OK); | 912 | EXPECT_EQ(chip.forward_by_front_chip(d, "smc", 1), HAS_OK); |
| 817 | } | 913 | } |
| 818 | 914 | ||
| 915 | +TEST(chip_smc_forward, forward_by_front_chip_rejects_length_larger_than_input) | ||
| 916 | +{ | ||
| 917 | + chip_smc_exposed chip; | ||
| 918 | + auto bus = init_exposed_chip(chip); | ||
| 919 | + | ||
| 920 | + ACCESS_DATA_S d{}; | ||
| 921 | + d.optype = CHIP_OP_TYPE_WRITE; | ||
| 922 | + d.addr = 0x60; | ||
| 923 | + d.offset = 0x20; | ||
| 924 | + d.length = 4; | ||
| 925 | + d.offset_width = 1; | ||
| 926 | + d.in_buffer = {0x11}; | ||
| 927 | + | ||
| 928 | + EXPECT_EQ(chip.forward_by_front_chip(d, "smc", 1), HAS_ERR); | ||
| 929 | + EXPECT_EQ(bus->write_call_count(), 0); | ||
| 930 | +} | ||
| 931 | + | ||
| 932 | +TEST(chip_smc_forward, forward_by_front_chip_rejects_oversized_offset_width) | ||
| 933 | +{ | ||
| 934 | + chip_smc_exposed chip; | ||
| 935 | + auto bus = init_exposed_chip(chip); | ||
| 936 | + | ||
| 937 | + ACCESS_DATA_S d{}; | ||
| 938 | + d.optype = CHIP_OP_TYPE_WRITE; | ||
| 939 | + d.addr = 0x60; | ||
| 940 | + d.offset = 0x20; | ||
| 941 | + d.length = 1; | ||
| 942 | + d.in_buffer = {0x11}; | ||
| 943 | + for (uint8_t width : {5, 251, 255}) { | ||
| 944 | + for (auto operation : {CHIP_OP_TYPE_READ, CHIP_OP_TYPE_WRITE}) { | ||
| 945 | + SCOPED_TRACE(::testing::Message() << +width << "/" << operation); | ||
| 946 | + d.offset_width = width; | ||
| 947 | + d.optype = operation; | ||
| 948 | + EXPECT_EQ(chip.forward_by_front_chip(d, "eeprom", 1), HAS_ERR); | ||
| 949 | + } | ||
| 950 | + } | ||
| 951 | + EXPECT_EQ(bus->write_call_count(), 0); | ||
| 952 | + EXPECT_EQ(bus->read_call_count(), 0); | ||
| 953 | +} | ||
| 954 | + | ||
| 955 | +TEST(chip_smc_forward, forward_write_capacity_boundary_with_offset) | ||
| 956 | +{ | ||
| 957 | + for (uint8_t width : {0, 1, 4}) { | ||
| 958 | + for (uint32_t extra : {0U, 1U}) { | ||
| 959 | + SCOPED_TRACE(::testing::Message() << +width << "/" << extra); | ||
| 960 | + chip_smc_exposed chip; | ||
| 961 | + auto bus = init_exposed_chip(chip); | ||
| 962 | + ACCESS_DATA_S d{}; | ||
| 963 | + d.optype = CHIP_OP_TYPE_WRITE; | ||
| 964 | + d.addr = 0x60; | ||
| 965 | + d.offset = 0x12345678; | ||
| 966 | + d.offset_width = width; | ||
| 967 | + d.length = 250 - width + extra; | ||
| 968 | + d.in_buffer.assign(d.length, 0xAB); | ||
| 969 | + EXPECT_EQ(chip.forward_by_front_chip(d, "smc", 1), extra ? HAS_ERR : HAS_OK); | ||
| 970 | + EXPECT_EQ(bus->write_call_count(), extra ? 0 : 1); | ||
| 971 | + } | ||
| 972 | + } | ||
| 973 | +} | ||
| 974 | + | ||
| 975 | +TEST(chip_smc_forward, forward_read_rejects_zero_response_length) | ||
| 976 | +{ | ||
| 977 | + chip_smc_exposed chip; | ||
| 978 | + auto bus = init_exposed_chip(chip, 0x88); | ||
| 979 | + // CRC8([0x88, 0x31, 0x89, 0x00]) is zero, so the legacy parser accepts | ||
| 980 | + // this malformed frame and exposes the length subtraction bug. | ||
| 981 | + bus->enqueue_read_payload({0x00, 0x00}); | ||
| 982 | + | ||
| 983 | + std::vector<uint8_t> out; | ||
| 984 | + EXPECT_EQ(chip.forward_read(1, out), HAS_ERR); | ||
| 985 | + EXPECT_TRUE(out.empty()); | ||
| 986 | +} | ||
| 987 | + | ||
| 988 | +TEST(chip_smc_forward, forward_read_rejects_truncated_response) | ||
| 989 | +{ | ||
| 990 | + chip_smc_exposed chip; | ||
| 991 | + auto bus = init_exposed_chip(chip); | ||
| 992 | + bus->enqueue_read_payload({0x03, 0x00, 0xAB}); | ||
| 993 | + | ||
| 994 | + std::vector<uint8_t> out; | ||
| 995 | + EXPECT_EQ(chip.forward_read(1, out), HAS_ERR); | ||
| 996 | + EXPECT_TRUE(out.empty()); | ||
| 997 | +} | ||
| 998 | + | ||
| 999 | +TEST(chip_smc_forward, forward_read_accepts_maximum_payload) | ||
| 1000 | +{ | ||
| 1001 | + chip_smc_exposed chip; | ||
| 1002 | + auto bus = init_exposed_chip(chip); | ||
| 1003 | + std::vector<uint8_t> payload(254, 0x00); | ||
| 1004 | + payload.front() = 0x11; | ||
| 1005 | + payload.back() = 0xEE; | ||
| 1006 | + auto frame = smc_read_frame(0x10, 0x31, payload); | ||
| 1007 | + bus->enqueue_read_payload(frame); | ||
| 1008 | + bus->enqueue_read_payload(std::move(frame)); | ||
| 1009 | + | ||
| 1010 | + std::vector<uint8_t> out; | ||
| 1011 | + EXPECT_EQ(chip.forward_read(255, out), HAS_OK); | ||
| 1012 | + EXPECT_EQ(out, payload); | ||
| 1013 | +} | ||
| 1014 | + | ||
| 819 | TEST(chip_smc_forward, forward_by_front_chip_smc_write_memcpy_fail) | 1015 | TEST(chip_smc_forward, forward_by_front_chip_smc_write_memcpy_fail) |
| 820 | { | 1016 | { |
| 821 | chip_smc_exposed chip; | 1017 | chip_smc_exposed chip; |
⚠️ 警告：空回复现在会清空调用方缓冲，
chip_eeprom::block_read的多块读会因此越界读去掉
!data.out_buffer.empty()之后，总线返回HAS_OK但回复为空时，调用方预分配的out_buffer会被替换成空数组。这让bit_read能识别出空回复，parse_eeprom_header_from_buffer/parse_eeprom_sign_from_buffer也都先检查了size()，这两处都没问题。但
drivers/internal/chip/chip_eeprom/eeprom.cpp的多块读循环没有长度检查：std::vector<uint8_t> read_buffer(next_read_length); // 预分配 ... ret = data_access(0, read_offset, read_length, in_buffer, read_buffer, CHIP_OP_TYPE_READ); ... out_buffer.insert(out_buffer.end(), read_buffer.begin(), read_buffer.begin() + read_length); // :126read_length字节，读到的是 0 或上一块的残留数据。数据是错的，但不会越界。read_buffer变为空，begin() + read_length超出末尾，insert发生堆越界读。read_length短的情况，主线原本就会越界。本 PR 的主题正是“总线与缓冲区边界”，建议一并修复，例如：
if (read_buffer.size() < read_length) { error = m_object_name + " read failed: short reply at offset=" + std::to_string(read_offset); return HAS_ERR; }并在
test_chip_eeprom中补一个“多块读中某一块返回空或短回复”的用例。我检查了其余读调用方：SPI Flash 有!resp.empty()判断，SMC 的 MCTP 解包有size()判断，CAN 和block_read只是把缓冲原样返回，都不受影响。