netdev - Re: [PATCH ethtool v2] sff-8079/8472: Fix missing sff-8472 output in netlink path

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]

Date:   Thu, 16 Jun 2022 20:04:54 +0200
From:   Ivan Vecera <ivecera@...hat.com>
To:     netdev@...r.kernel.org
Cc:     Michal Kubecek <mkubecek@...e.cz>,
        Daniel Juarez <djuarezg@...n.ch>,
        Ido Schimmel <idosch@...dia.com>
Subject: Re: [PATCH ethtool v2] sff-8079/8472: Fix missing sff-8472 output
 in netlink path

On Thu, 16 Jun 2022 19:59:47 +0200
Ivan Vecera <ivecera@...hat.com> wrote:

> Commit 5b64c66f58d ("ethtool: Add netlink handler for
> getmodule (-m)") provided a netlink variant for getmodule
> but also introduced a regression as netlink output is different
> from ioctl output that provides information from A2h page
> via sff8472_show_all().
> 
> To fix this the netlink path should check a presence of A2h page
> by value of bit 6 in byte 92 of page A0h and if it is set then
> get A2h page and call sff8472_show_all().
> 
> Fixes: 5b64c66f58d ("ethtool: Add netlink handler for getmodule (-m)")
> Tested-by: Daniel Juarez <djuarezg@...n.ch>
> Tested-by: Ido Schimmel <idosch@...dia.com>
> Reviewed-by: Ido Schimmel <idosch@...dia.com>
> Co-authored-by: Ido Schimmel <idosch@...dia.com>
> Signed-off-by: Ivan Vecera <ivecera@...hat.com>
> ---
>  sfpid.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 46 insertions(+), 8 deletions(-)
> 
> diff --git a/sfpid.c b/sfpid.c
> index 621d1e86c278..1bc45c183770 100644
> --- a/sfpid.c
> +++ b/sfpid.c
> @@ -13,8 +13,9 @@
>  #include "sff-common.h"
>  #include "netlink/extapi.h"
>  
> -#define SFF8079_PAGE_SIZE	0x80
> -#define SFF8079_I2C_ADDRESS_LOW	0x50
> +#define SFF8079_PAGE_SIZE		0x80
> +#define SFF8079_I2C_ADDRESS_LOW		0x50
> +#define SFF8079_I2C_ADDRESS_HIGH	0x51
>  
>  static void sff8079_show_identifier(const __u8 *id)
>  {
> @@ -450,18 +451,55 @@ void sff8079_show_all_ioctl(const __u8 *id)
>  	sff8079_show_all_common(id);
>  }
>  
> -int sff8079_show_all_nl(struct cmd_context *ctx)
> +static int sff8079_get_eeprom_page(struct cmd_context *ctx, u8 i2c_address,
> +				   __u8 *buf)
>  {
>  	struct ethtool_module_eeprom request = {
>  		.length = SFF8079_PAGE_SIZE,
> -		.i2c_address = SFF8079_I2C_ADDRESS_LOW,
> +		.i2c_address = i2c_address,
>  	};
>  	int ret;
>  
>  	ret = nl_get_eeprom_page(ctx, &request);
> -	if (ret < 0)
> -		return ret;
> -	sff8079_show_all_common(request.data);
> +	if (!ret)
> +		memcpy(buf, request.data, SFF8079_PAGE_SIZE);
> +
> +	return ret;
> +}
> +
> +int sff8079_show_all_nl(struct cmd_context *ctx)
> +{
> +	u8 *buf;
> +	int ret;
> +
> +	/* The SFF-8472 parser expects a single buffer that contains the
> +	 * concatenation of the first 256 bytes from addresses A0h and A2h,
> +	 * respectively.
> +	 */
> +	buf = calloc(1, ETH_MODULE_SFF_8472_LEN);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	/* Read A0h page */
> +	ret = sff8079_get_eeprom_page(ctx, SFF8079_I2C_ADDRESS_LOW, buf);
> +	if (ret)
> +		goto out;
> +
> +	sff8079_show_all_common(buf);
> +
> +	/* Finish if A2h page is not present */
> +	if (!(buf[92] & (1 << 6)))
> +		goto out;
> +
> +	/* Read A2h page */
> +	ret = sff8079_get_eeprom_page(ctx, SFF8079_I2C_ADDRESS_HIGH,
> +				      buf + ETH_MODULE_SFF_8079_LEN);
> +	if (ret)
> +		goto out;
> +
> +	sff8472_show_all(buf);
> +out:
> +	free(buf);
>  
> -	return 0;
> +	return ret;
>  }

Too fast fingers :-( missing leading '2'.

Nacked-by: Ivan Vecera <ivecera@...hat.com>