netdev - Re: [PATCH net-next 3/5] dpll: zl3073x: Add firmware loading functionality

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: <5f2aaa88-d3fb-46ae-b325-603fda5e8851@redhat.com>
Date: Tue, 29 Jul 2025 17:20:10 +0200
From: Ivan Vecera <ivecera@...hat.com>
To: Simon Horman <horms@...nel.org>
Cc: netdev@...r.kernel.org, Jiri Pirko <jiri@...nulli.us>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Jonathan Corbet <corbet@....net>,
 Prathosh Satish <Prathosh.Satish@...rochip.com>, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, Michal Schmidt <mschmidt@...hat.com>,
 Petr Oros <poros@...hat.com>
Subject: Re: [PATCH net-next 3/5] dpll: zl3073x: Add firmware loading
 functionality

On 26. 07. 25 10:33 odp., Simon Horman wrote:
> On Fri, Jul 25, 2025 at 05:41:34PM +0200, Ivan Vecera wrote:
>> Add functionality for loading firmware files provided by the vendor
>> to be flashed into the device's internal flash memory. The firmware
>> consists of several components, such as the firmware executable itself,
>> chip-specific customizations, and configuration files.
>>
>> The firmware file contains at least a flash utility, which is executed
>> on the device side, and one or more flashable components. Each component
>> has its own specific properties, such as the address where it should be
>> loaded during flashing, one or more destination flash pages, and
>> the flashing method that should be used.
>>
>> Signed-off-by: Ivan Vecera <ivecera@...hat.com>
> 
> Hi Ivan,
> 
> Some minor feedback from my side.
> 
> ...
> 
>> diff --git a/drivers/dpll/zl3073x/fw.c b/drivers/dpll/zl3073x/fw.c
> 
> ...

Hi Simon,

> 
>> +/* Santity check */
> 
> Sanity

Will fix in v2.

>> +static_assert(ARRAY_SIZE(component_info) == ZL_FW_NUM_COMPONENTS);
> 
> ...
> 
>> +int zl3073x_fw_flash(struct zl3073x_dev *zldev, struct zl3073x_fw *zlfw,
>> +		     struct netlink_ext_ack *extack)
>> +{
>> +	int i, rc;
>> +
>> +	for (i = 0; i < ZL_FW_NUM_COMPONENTS; i++) {
>> +		if (!zlfw->component[i])
>> +			continue; /* Component is not present */
>> +
>> +		rc = zl3073x_fw_component_flash(zldev, zlfw->component[i],
>> +						extack);
>> +		if (rc)
>> +			break;
>> +	}
> 
> Perhaps it cannot happen in practice.
> But Smatch warns that rc may be used uninitialised below.
> And that does seem theoretically possible if all
> iterations of the loop above hit the "continue" path.

Yes, it should not happen as at least one component has to be present
but for sure I will init rc in the next iteration.

Thanks for review,
Ivan

> 
>> +
>> +	return rc;
>> +}
> 
> ...
>