linux-kernel - Re: [PATCH v9 9/9] drm/xe/xe_late_bind

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d695d564-6c4d-431f-9b21-5ebf574e0c2d@intel.com>
Date: Thu, 25 Sep 2025 17:39:27 +0530
From: "Nilawar, Badal" <badal.nilawar@...el.com>
To: Kees Bakker <kees@...erbout.nl>, <intel-xe@...ts.freedesktop.org>,
	<dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
CC: <anshuman.gupta@...el.com>, <rodrigo.vivi@...el.com>,
	<alexander.usyskin@...el.com>, <gregkh@...uxfoundation.org>,
	<daniele.ceraolospurio@...el.com>, <mika.westerberg@...ux.intel.com>,
	<lucas.demarchi@...el.com>, <karthik.poosa@...el.com>
Subject: Re: [PATCH v9 9/9] drm/xe/xe_late_bind_fw: Extract and print version
 info


On 25-09-2025 02:14, Kees Bakker wrote:
> Op 05-09-2025 om 17:49 schreef Badal Nilawar:
>> Extract and print version info of the late binding binary.
>>
>> v2: Some refinements (Daniele)
>>
>> Signed-off-by: Badal Nilawar <badal.nilawar@...el.com>
>> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@...el.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@...el.com>
>> ---
>>   drivers/gpu/drm/xe/xe_late_bind_fw.c       | 124 +++++++++++++++++++++
>>   drivers/gpu/drm/xe/xe_late_bind_fw_types.h |   3 +
>>   drivers/gpu/drm/xe/xe_uc_fw_abi.h          |  66 +++++++++++
>>   3 files changed, 193 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c 
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> index 0f062008ca83..38f3feb2aecd 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> @@ -45,6 +45,121 @@ late_bind_to_xe(struct xe_late_bind *late_bind)
>>       return container_of(late_bind, struct xe_device, late_bind);
>>   }
>>   +static struct xe_device *
>> +late_bind_fw_to_xe(struct xe_late_bind_fw *lb_fw)
>> +{
>> +    return container_of(lb_fw, struct xe_device, 
>> late_bind.late_bind_fw[lb_fw->id]);
>> +}
>> +
>> +/* Refer to the "Late Bind based Firmware Layout" documentation 
>> entry for details */
>> +static int parse_cpd_header(struct xe_late_bind_fw *lb_fw,
>> +                const void *data, size_t size, const char 
>> *manifest_entry)
>> +{
>> +    struct xe_device *xe = late_bind_fw_to_xe(lb_fw);
>> +    const struct gsc_cpd_header_v2 *header = data;
>> +    const struct gsc_manifest_header *manifest;
>> +    const struct gsc_cpd_entry *entry;
>> +    size_t min_size = sizeof(*header);
>> +    u32 offset;
>> +    int i;
>> +
>> +    /* manifest_entry is mandatory */
>> +    xe_assert(xe, manifest_entry);
>> +
>> +    if (size < min_size || header->header_marker != 
>> GSC_CPD_HEADER_MARKER)
>> +        return -ENOENT;
>> +
>> +    if (header->header_length < sizeof(struct gsc_cpd_header_v2)) {
>> +        drm_err(&xe->drm, "%s late binding fw: Invalid CPD header 
>> length %u!\n",
>> +            fw_id_to_name[lb_fw->id], header->header_length);
>> +        return -EINVAL;
>> +    }
>> +
>> +    min_size = header->header_length + sizeof(struct gsc_cpd_entry) 
>> * header->num_of_entries;
>> +    if (size < min_size) {
>> +        drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
>> +            fw_id_to_name[lb_fw->id], size, min_size);
>> +        return -ENODATA;
>> +    }
>> +
>> +    /* Look for the manifest first */
>> +    entry = (void *)header + header->header_length;
>> +    for (i = 0; i < header->num_of_entries; i++, entry++)
>> +        if (strcmp(entry->name, manifest_entry) == 0)
>> +            offset = entry->offset & GSC_CPD_ENTRY_OFFSET_MASK;
>> +
>> +    if (!offset) {
> This for loop looks suspicious. Do you continue the loop on purpose
> after finding the first match? Or should there be a break?
> Also, if there is no match then offset is uninitialized. Isn't it better
> to initialize offset at the start?

Thanks for highlighting. This has been addressed in this patch 
https://lore.kernel.org/intel-xe/20250924102208.9216-1-colin.i.king@gmail.com/.

Regards,
Badal