When ACE images require dynamic address allocation, the code accesses
frags[frag_index - 1] without bounds checking. This could lead to:

- Array underflow if frag_index is 0
- Use of uninitialized fragment data for address calculations
- Silent failures in address allocation

Add proper validation before accessing the previous fragment and
provide clear error messages when validation fails.

Signed-off-by: WangYuli <wangyuli@uniontech.com>
---
 drivers/misc/mei/vsc-fw-loader.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/misc/mei/vsc-fw-loader.c b/drivers/misc/mei/vsc-fw-loader.c
index 43abefa806e1..e2d318ecb76a 100644
--- a/drivers/misc/mei/vsc-fw-loader.c
+++ b/drivers/misc/mei/vsc-fw-loader.c
@@ -516,7 +516,19 @@ static int vsc_identify_ace_image(struct vsc_fw_loader *fw_loader)
 		frag->type = ace_image_map[i].image_type;
 
 		if (!frag->location) {
+			if (frag_index == 0) {
+				dev_err(fw_loader->dev,
+					"Cannot auto-allocate address for first fragment\n");
+				ret = -EINVAL;
+				goto err_release_image;
+			}
 			last_frag = &fw_loader->frags[frag_index - 1];
+			if (!last_frag->location || !last_frag->size) {
+				dev_err(fw_loader->dev,
+					"Previous fragment not properly initialized for auto-allocation\n");
+				ret = -EINVAL;
+				goto err_release_image;
+			}
 			frag->location =
 				ALIGN(last_frag->location + last_frag->size, SZ_4K);
 		}
-- 
2.50.1

On Fri, Aug 01, 2025 at 05:06:00PM +0800, WangYuli wrote:
> When ACE images require dynamic address allocation, the code accesses
> frags[frag_index - 1] without bounds checking. This could lead to:
> 
> - Array underflow if frag_index is 0

How can that happen?  Does this value come from the kernel or from the
hardware?

> - Use of uninitialized fragment data for address calculations
> - Silent failures in address allocation
> 
> Add proper validation before accessing the previous fragment and
> provide clear error messages when validation fails.
> 
> Signed-off-by: WangYuli <wangyuli@uniontech.com>

What commit id does this fix?

How was this tested?

thanks,

greg k-h

On Fri, Aug 01, 2025 at 05:06:00PM +0800, WangYuli wrote:
> When ACE images require dynamic address allocation, the code accesses
> frags[frag_index - 1] without bounds checking. This could lead to:
> 
> - Array underflow if frag_index is 0

How can that happen?  It's coming directly from a static array in the
code itself that it declared right above these lines?

> - Use of uninitialized fragment data for address calculations

Where will that come from?

> - Silent failures in address allocation

Where?

> Add proper validation before accessing the previous fragment and
> provide clear error messages when validation fails.

But how can any of this really happen?  If it does, it's a bug in the
code that people added.  So why is any of this needed to the code today?

How did you hit any of the above, and how was this patch tested?

thanks,

greg k-h