netdev - Re: [PATCH net-next] netdevsim: add couple of fw_update_flash

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: <20250720141502.GV2459@horms.kernel.org>
Date: Sun, 20 Jul 2025 15:15:02 +0100
From: Simon Horman <horms@...nel.org>
To: Jiri Pirko <jiri@...nulli.us>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, andrew+netdev@...n.ch
Subject: Re: [PATCH net-next] netdevsim: add couple of fw_update_flash_*
 debugfs knobs

On Sat, Jul 19, 2025 at 03:13:15PM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@...dia.com>
> 
> Netdevsim emulates firmware update and it takes 5 seconds to complete.
> For some usecases, this is too long and unnecessary. Allow user to
> configure the time by exposing debugfs knobs to set flash size, chunk
> size and chunk time.
> 
> Signed-off-by: Jiri Pirko <jiri@...dia.com>

...

> @@ -1035,20 +1040,20 @@ static int nsim_dev_flash_update(struct devlink *devlink,
>  						   params->component, 0, 0);
>  	}
>  
> -	for (i = 0; i < NSIM_DEV_FLASH_SIZE / NSIM_DEV_FLASH_CHUNK_SIZE; i++) {
> +	for (i = 0; i < flash_size / flash_chunk_size; i++) {
>  		if (nsim_dev->fw_update_status)
>  			devlink_flash_update_status_notify(devlink, "Flashing",
>  							   params->component,
> -							   i * NSIM_DEV_FLASH_CHUNK_SIZE,
> -							   NSIM_DEV_FLASH_SIZE);
> -		msleep(NSIM_DEV_FLASH_CHUNK_TIME_MS);
> +							   i * flash_chunk_size,
> +							   flash_size);
> +		msleep(flash_chunk_time_ms);
>  	}

Hi Jiri,

This loop seems to assume that flash_size is an integer number multiple
of flash_chunk_size. But with this change that may not be the case,
leading to less than flash_size bytes being written.

Perhaps the code should to guard against that, or handle it somehow.

>  
>  	if (nsim_dev->fw_update_status) {
>  		devlink_flash_update_status_notify(devlink, "Flashing",
>  						   params->component,
> -						   NSIM_DEV_FLASH_SIZE,
> -						   NSIM_DEV_FLASH_SIZE);
> +						   flash_size,
> +						   flash_size);
>  		devlink_flash_update_timeout_notify(devlink, "Flash select",
>  						    params->component, 81);
>  		devlink_flash_update_status_notify(devlink, "Flashing done",

...