* [PATCH v4 2/3] fpga: dfl-afu: validate DMA mapping length in afu_dma_map_region()
2026-04-07 17:22 [PATCH v4 1/3] fpga: dfl: add bounds check in dfh_get_param_size() Sebastian Alba Vives
@ 2026-04-07 17:22 ` Sebastian Alba Vives
2026-04-07 17:22 ` [PATCH v4 3/3] fpga: microchip-spi: fix OOB read in mpf_ops_parse_header() Sebastian Alba Vives
2026-05-04 11:37 ` [PATCH v4 1/3] fpga: dfl: add bounds check in dfh_get_param_size() Xu Yilun
2 siblings, 0 replies; 5+ messages in thread
From: Sebastian Alba Vives @ 2026-04-07 17:22 UTC (permalink / raw)
To: yilun.xu, gregkh
Cc: linux-fpga, conor.dooley, mdf, linux-kernel, stable,
Sebastian Alba Vives
afu_ioctl_dma_map() accepts a 64-bit length from userspace via
DFL_FPGA_PORT_DMA_MAP ioctl without an upper bound check. The value
is passed to afu_dma_pin_pages() where npages is derived as
length >> PAGE_SHIFT and passed to pin_user_pages_fast() which takes
int nr_pages, causing implicit truncation if length is very large.
Validate map.length at the ioctl entry point before calling
afu_dma_map_region(), rejecting values whose page count exceeds
INT_MAX.
Signed-off-by: Sebastian Alba Vives <sebasjosue84@gmail.com>
---
Changes in v4:
- Resubmit as full series per maintainer request.
Changes in v3:
- Move validation to afu_ioctl_dma_map() at the ioctl entry point,
before crossing the userspace/kernel boundary, instead of deep in
afu_dma_pin_pages(). Suggested by Greg Kroah-Hartman.
Changes in v2:
- Added cap at INT_MAX in afu_dma_pin_pages() (superseded by v3).
---
drivers/fpga/dfl-afu-main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index 3bf8e73..097a97e 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -723,6 +723,9 @@ afu_ioctl_dma_map(struct dfl_feature_dev_data *fdata, void __user *arg)
if (map.argsz < minsz || map.flags)
return -EINVAL;
+ if (map.length >> PAGE_SHIFT > (u64)INT_MAX)
+ return -EINVAL;
+
ret = afu_dma_map_region(fdata, map.user_addr, map.length, &map.iova);
if (ret)
return ret;
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v4 3/3] fpga: microchip-spi: fix OOB read in mpf_ops_parse_header()
2026-04-07 17:22 [PATCH v4 1/3] fpga: dfl: add bounds check in dfh_get_param_size() Sebastian Alba Vives
2026-04-07 17:22 ` [PATCH v4 2/3] fpga: dfl-afu: validate DMA mapping length in afu_dma_map_region() Sebastian Alba Vives
@ 2026-04-07 17:22 ` Sebastian Alba Vives
2026-05-04 9:03 ` Xu Yilun
2026-05-04 11:37 ` [PATCH v4 1/3] fpga: dfl: add bounds check in dfh_get_param_size() Xu Yilun
2 siblings, 1 reply; 5+ messages in thread
From: Sebastian Alba Vives @ 2026-04-07 17:22 UTC (permalink / raw)
To: yilun.xu, gregkh
Cc: linux-fpga, conor.dooley, mdf, linux-kernel, stable,
Sebastian Alba Vives
mpf_ops_parse_header() reads header_size at MPF_HEADER_SIZE_OFFSET (24)
without first checking that count is large enough, leading to an
out-of-bounds read if the buffer is smaller than 25 bytes.
Additionally, when header_size is zero, the expression
*(buf + header_size - 1) reads one byte before the buffer start.
Since a zero header_size cannot be resolved by providing a larger
buffer, return -EINVAL instead of falling through.
Fixes: 5f8d4a9008307 ("fpga: microchip-spi: add Microchip MPF FPGA manager")
Cc: stable@vger.kernel.org
Signed-off-by: Sebastian Alba Vives <sebasjosue84@gmail.com>
---
Changes in v4:
- Reduce to two minimal fixes only: minimum count check before
reading header_size, and -EINVAL for zero header_size.
Drop redundant block loop checks — the pre-loop bounds extension
already ensures all block offsets are within count.
Suggested by Xu Yilun.
Changes in v3:
- Add overflow check for 32-bit in component size loop.
Changes in v2:
- Return -EINVAL for header_size == 0, -EAGAIN in block loop,
add count check before MPF_HEADER_SIZE_OFFSET read.
---
drivers/fpga/microchip-spi.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/fpga/microchip-spi.c b/drivers/fpga/microchip-spi.c
index 6134cea..dca1a5d 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;
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v4 3/3] fpga: microchip-spi: fix OOB read in mpf_ops_parse_header()
2026-04-07 17:22 ` [PATCH v4 3/3] fpga: microchip-spi: fix OOB read in mpf_ops_parse_header() Sebastian Alba Vives
@ 2026-05-04 9:03 ` Xu Yilun
0 siblings, 0 replies; 5+ messages in thread
From: Xu Yilun @ 2026-05-04 9:03 UTC (permalink / raw)
To: Sebastian Alba Vives
Cc: gregkh, linux-fpga, conor.dooley, mdf, linux-kernel, stable
On Tue, Apr 07, 2026 at 11:22:17AM -0600, Sebastian Alba Vives wrote:
> mpf_ops_parse_header() reads header_size at MPF_HEADER_SIZE_OFFSET (24)
> without first checking that count is large enough, leading to an
> out-of-bounds read if the buffer is smaller than 25 bytes.
>
> Additionally, when header_size is zero, the expression
> *(buf + header_size - 1) reads one byte before the buffer start.
> Since a zero header_size cannot be resolved by providing a larger
> buffer, return -EINVAL instead of falling through.
>
> Fixes: 5f8d4a9008307 ("fpga: microchip-spi: add Microchip MPF FPGA manager")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sebastian Alba Vives <sebasjosue84@gmail.com>
> ---
> Changes in v4:
> - Reduce to two minimal fixes only: minimum count check before
> reading header_size, and -EINVAL for zero header_size.
> Drop redundant block loop checks — the pre-loop bounds extension
> already ensures all block offsets are within count.
> Suggested by Xu Yilun.
> Changes in v3:
> - Add overflow check for 32-bit in component size loop.
> Changes in v2:
> - Return -EINVAL for header_size == 0, -EAGAIN in block loop,
> add count check before MPF_HEADER_SIZE_OFFSET read.
> ---
> drivers/fpga/microchip-spi.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/fpga/microchip-spi.c b/drivers/fpga/microchip-spi.c
> index 6134cea..dca1a5d 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;
Why do you think the fpga-mgr core would provide a buffer that is not
good for low-lever driver to start with? There should be contract
between the core and the driver, if you've found anyone breaks the
contract, fix the breaker, not to add defensive code here.
Thanks,
Yilun
> +
> header_size = *(buf + MPF_HEADER_SIZE_OFFSET);
> + if (!header_size)
> + return -EINVAL;
> +
> if (header_size > count) {
> info->header_size = header_size;
> return -EAGAIN;
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/3] fpga: dfl: add bounds check in dfh_get_param_size()
2026-04-07 17:22 [PATCH v4 1/3] fpga: dfl: add bounds check in dfh_get_param_size() Sebastian Alba Vives
2026-04-07 17:22 ` [PATCH v4 2/3] fpga: dfl-afu: validate DMA mapping length in afu_dma_map_region() Sebastian Alba Vives
2026-04-07 17:22 ` [PATCH v4 3/3] fpga: microchip-spi: fix OOB read in mpf_ops_parse_header() Sebastian Alba Vives
@ 2026-05-04 11:37 ` Xu Yilun
2 siblings, 0 replies; 5+ messages in thread
From: Xu Yilun @ 2026-05-04 11:37 UTC (permalink / raw)
To: Sebastian Alba Vives
Cc: gregkh, linux-fpga, conor.dooley, mdf, linux-kernel, stable
On Tue, Apr 07, 2026 at 11:22:15AM -0600, Sebastian Alba Vives wrote:
> dfh_get_param_size() can return a parameter size larger than the feature
> region because the loop bounds check is evaluated before incrementing
> size. If the EOP (End of Parameters) bit is set in the same iteration,
> the inflated size is returned without re-validation against max.
>
> This can cause create_feature_instance() to call memcpy_fromio() with a
> size exceeding the ioremap'd region when a malicious FPGA device provides
> crafted DFHv1 parameter headers.
>
> Add a bounds check after the size increment to ensure the accumulated
> size never exceeds the feature boundary.
>
> Fixes: a80a4b2b2e4f ("fpga: dfl: add support for DFHv1")
> Signed-off-by: Sebastian Alba Vives <sebasjosue84@gmail.com>
> ---
> Changes in v4:
> - Resubmit as full series per maintainer request.
> Changes in v2:
> - Use (size > max) instead of (size + DFHv1_PARAM_HDR > max).
> The previous check unnecessarily guarded against the next parameter
> header, which is not relevant at this point in the loop.
> Suggested by Xu Yilun.
> ---
> drivers/fpga/dfl.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index 4087a36..81d7a68 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -1132,7 +1132,8 @@ static int dfh_get_param_size(void __iomem *dfh_base, resource_size_t max)
> return -EINVAL;
>
> size += next * sizeof(u64);
> -
> + if (size > max)
> + return -EINVAL;
I'd prefer a blank line here.
> if (FIELD_GET(DFHv1_PARAM_HDR_NEXT_EOP, v))
> return size;
> }
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread