Messages in this thread Patch in this message |  | | Date | Tue, 7 Apr 2026 18:44:14 +0800 | | From | Xu Yilun <> | | Subject | Re: [PATCH v3] fpga: microchip-spi: add bounds checks in mpf_ops_parse_header() |
| |
> @@ -139,6 +145,12 @@ static int mpf_ops_parse_header(struct fpga_manager *mgr, > bitstream_start = 0; > > while (blocks_num--) { > + if (block_id_offset >= count || > + block_start_offset + sizeof(u32) > count) { > + info->header_size = block_start_offset + sizeof(u32); > + return -EAGAIN; > + } > +
The image header has already been extended up to all look-up table for blocks, is it?
header_size += blocks_num * MPF_LOOKUP_TABLE_RECORD_SIZE; if (header_size > count) { info->header_size = header_size; return -EAGAIN; }
> block_id = *(buf + block_id_offset); > block_start = get_unaligned_le32(buf + block_start_offset); > > @@ -175,6 +187,9 @@ static int mpf_ops_parse_header(struct fpga_manager *mgr, > * to each other. Image header should be extended by now up to where > * actual bitstream starts, so no need for overflow check anymore. > */ > + if (MPF_DATA_SIZE_OFFSET + sizeof(u16) > count) > + return -EINVAL; > +
Do you notice the comments above? IIUC it says all these header info should be before actual bitstream starts, if we could ensure this, we don't need to check the addresses inside the header byte by byte.
I think it is important we understand the structure of the image file first then meaningfully check the boundaries chunk by chunk, rather than byte by byte, which makes code unreadable.
Thanks, Yilun
> components_num = get_unaligned_le16(buf + MPF_DATA_SIZE_OFFSET); > > for (i = 0; i < components_num; i++) { > @@ -183,6 +198,11 @@ static int mpf_ops_parse_header(struct fpga_manager *mgr, > component_size_byte_off = > (i * MPF_BITS_PER_COMPONENT_SIZE) % BITS_PER_BYTE; > > + if (components_size_start + component_size_byte_num < components_size_start || > + components_size_start + component_size_byte_num + > + sizeof(u32) > count) > + return -EINVAL; > + > component_size = get_unaligned_le32(buf + > components_size_start + > component_size_byte_num); > -- > 2.43.0 >
|  |