Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Low] drm/i915: contradictory values in GSC manifest bounds check error log
--

commit 65566b07d00691d3c9ac58902a755662f6ba6177
Author: Jesus Narvaez <[email protected]>

drm/i915/gsc: Validate the CPD entry offset before manifest read

This commit adds bounds checking for the CPD manifest offset before reading
the GSC manifest to prevent out-of-bounds dereferencing. It also validates
the CPD header length and casts the boot1 partition offset and size to
prevent integer overflows.

> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c 
> b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
> index d550eb6edfb8e..ab6c19d4125f6 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c

[ ... ]

> @@ -205,7 +214,15 @@ int intel_gsc_fw_get_binary_info(struct intel_uc_fw 
> *gsc_fw, const void *data, s
>       cpd_entry = (void *)cpd_header + cpd_header->header_length;
>       for (i = 0; i < cpd_header->num_of_entries; i++, cpd_entry++) {
>               if (strcmp(cpd_entry->name, "RBEP.man") == 0) {
> -                     manifest = (void *)cpd_header + 
> cpd_entry_offset(cpd_entry);
> +                     u32 man_off = cpd_entry_offset(cpd_entry);
> +
> +                     if ((man_off + sizeof(struct 
> intel_gsc_manifest_header)) >
> +                         (layout->boot1.size - cpd_offset)) {
> +                             gt_err(gt, "GSC FW boot section too small for 
> manifest: %u < %zu\n",
> +                                    layout->boot1.size, man_off + 
> sizeof(*manifest));

[Severity: Low]
Are the variables passed to gt_err accurately mapping to the bounding
condition that just failed?

The bounding condition checks if man_off + sizeof(struct
intel_gsc_manifest_header) > layout->boot1.size - cpd_offset.
However, the error log prints layout->boot1.size and man_off +
sizeof(*manifest), omitting cpd_offset from the calculation.

Could this lead to misleading and mathematically contradictory values in the
error log?

> +                             return -ENODATA;
> +                     }
> +                     manifest = (void *)cpd_header + man_off;
>                       intel_uc_fw_version_from_gsc_manifest(&gsc->release,
>                                                             manifest);
>                       gsc->security_version = manifest->security_version;

-- 
Sashiko AI review · 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to