RE: [RFC PATCH net-next 9/9] ethtool: Add ability to flash transceiver modules' firmware

[Danielle Ratson]

> -----Original Message-----
> From: Simon Horman <horms@xxxxxxxxxx>
> Sent: Monday, 22 January 2024 12:38
> To: Danielle Ratson <danieller@xxxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx;
> kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; corbet@xxxxxxx;
> linux@xxxxxxxxxxxxxxx; sdf@xxxxxxxxxx; kory.maincent@xxxxxxxxxxx;
> maxime.chevallier@xxxxxxxxxxx; vladimir.oltean@xxxxxxx;
> przemyslaw.kitszel@xxxxxxxxx; ahmed.zaki@xxxxxxxxx;
> richardcochran@xxxxxxxxx; shayagr@xxxxxxxxxx;
> paul.greenwalt@xxxxxxxxx; jiri@xxxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; mlxsw <mlxsw@xxxxxxxxxx>; Petr Machata
> <petrm@xxxxxxxxxx>; Ido Schimmel <idosch@xxxxxxxxxx>
> Subject: Re: [RFC PATCH net-next 9/9] ethtool: Add ability to flash transceiver
> modules' firmware
> 
> On Mon, Jan 22, 2024 at 10:45:30AM +0200, Danielle Ratson wrote:
> 
> ...
> 
> > +static int module_flash_fw(struct net_device *dev, struct nlattr **tb,
> > +			   struct netlink_ext_ack *extack) {
> > +	struct ethtool_module_fw_flash_params params = {};
> > +	struct nlattr *attr;
> > +
> > +	if (!tb[ETHTOOL_A_MODULE_FW_FLASH_FILE_NAME]) {
> > +		NL_SET_ERR_MSG_ATTR(extack,
> > +
> tb[ETHTOOL_A_MODULE_FW_FLASH_FILE_NAME],
> > +				    "File name attribute is missing");
> > +		return -EINVAL;
> > +	}
> > +
> > +	params.file_name =
> > +		nla_data(tb[ETHTOOL_A_MODULE_FW_FLASH_FILE_NAME]);
> > +
> > +	attr = tb[ETHTOOL_A_MODULE_FW_FLASH_PASSWORD];
> > +	if (attr) {
> > +		params.password = cpu_to_be32(nla_get_u32(attr));
> 
> Hi Danielle,
> 
> The type of password is u32, so perhaps cpu_to_be32() isn't needed here?
> 
> Flagged by Sparse.

Hi Simon,

The cpu_to_be32() is actually needed here, without it the password parameter from user space is passed with wrong endianness.

Thanks,
Danielle

> 
> > +		params.password_valid = true;
> > +	}
> > +
> > +	return module_flash_fw_schedule(dev, &params, extack); }
> 
> ...