Messages in this thread |  | | Date | Thu, 2 Apr 2026 17:16:56 +0100 | | From | Conor Dooley <> | | Subject | Re: [PATCH v2] fpga: microchip-spi: add bounds checks in mpf_ops_parse_header() |
| |
On Thu, Apr 02, 2026 at 09:37:52AM -0600, Sebastian Alba Vives wrote: > From: Sebastian Josue Alba Vives <sebasjosue84@gmail.com> > > mpf_ops_parse_header() reads several fields from the bitstream file > and uses them as offsets and sizes without validating them against the > buffer size, leading to multiple out-of-bounds read vulnerabilities: > > 1. There is no check that count is large enough to read header_size > at MPF_HEADER_SIZE_OFFSET (24). Add a minimum count check. > > 2. When header_size (u8 from file) is 0, the expression > *(buf + header_size - 1) reads one byte before the buffer. > Return -EINVAL since retrying with a larger buffer cannot fix > a zero header_size. > > 3. In the block lookup loop, block_id_offset and block_start_offset > advance by MPF_LOOKUP_TABLE_RECORD_SIZE (9) each iteration with > blocks_num (u8) controlling the count. With a small buffer, these > offsets exceed count, causing OOB reads via get_unaligned_le32(). > Return -EAGAIN since a larger buffer may resolve the issue. > > 4. components_size_start (from file) and component_size_byte_num > (derived from components_num, u16 from file) are used as offsets > into buf without validation, allowing arbitrary OOB reads. > > Add bounds checks for all four cases. > > Fixes: 5f8d4a9008307 ("fpga: microchip-spi: add Microchip MPF FPGA manager") > Cc: stable@vger.kernel.org > Signed-off-by: Sebastian Alba Vives <sebasjosue84@gmail.com> > --- > drivers/fpga/microchip-spi.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/fpga/microchip-spi.c b/drivers/fpga/microchip-spi.c > index 6134cea..00fa2d6 100644 > --- a/drivers/fpga/microchip-spi.c > +++ b/drivers/fpga/microchip-spi.c > @@ -115,7 +115,13 @@ static int mpf_ops_parse_header(struct fpga_manager *mgr, > return -EINVAL; > } > > + if (count < MPF_HEADER_SIZE_OFFSET + 1) > + return -EINVAL; > + > header_size = *(buf + MPF_HEADER_SIZE_OFFSET); > + if (!header_size) > + return -EINVAL; > + > if (header_size > count) { > info->header_size = header_size; > return -EAGAIN; > @@ -139,6 +145,10 @@ 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) > + return -EAGAIN; > + > block_id = *(buf + block_id_offset); > block_start = get_unaligned_le32(buf + block_start_offset); > > @@ -183,6 +193,10 @@ 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 + > + sizeof(u32) > count) > + return -EINVAL;
I didn't mention it explicitly, but it kinda follows from the other comment, do we not just want to ask for a bigger buffer here too?
> + > component_size = get_unaligned_le32(buf + > components_size_start + > component_size_byte_num); > -- > 2.43.0 > [unhandled content-type:application/pgp-signature] |  |