netdev - Re: [PATCH ethtool 2/2] netlink: settings: Drop port filter for MDI-X

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211027195923.3vhfr4diwuelw3gg@lion.mk-sys.cz>
Date:   Wed, 27 Oct 2021 21:59:23 +0200
From:   Michal Kubecek <mkubecek@...e.cz>
To:     bage@...utronix.de
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH ethtool 2/2] netlink: settings: Drop port filter for MDI-X

On Wed, Oct 27, 2021 at 08:11:40PM +0200, bage@...utronix.de wrote:
> From: Bastian Germann <bage@...utronix.de>
> 
> The port == PORT_TP condition on printing linkinfo's MDI-X field prevents
> ethtool from printing that info even if it is present and valid, e.g. with
> the port being MII and still having that info.
> 
> Signed-off-by: Bastian Germann <bage@...utronix.de>
> ---
>  netlink/settings.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/netlink/settings.c b/netlink/settings.c
> index c4f5d61..4da251b 100644
> --- a/netlink/settings.c
> +++ b/netlink/settings.c
> @@ -560,8 +560,7 @@ int linkinfo_reply_cb(const struct nlmsghdr *nlhdr, void *data)
>  		print_enum(names_transceiver, ARRAY_SIZE(names_transceiver),
>  			   val, "Transceiver");
>  	}
> -	if (tb[ETHTOOL_A_LINKINFO_TP_MDIX] && tb[ETHTOOL_A_LINKINFO_TP_MDIX_CTRL] &&
> -	    port == PORT_TP) {
> +	if (tb[ETHTOOL_A_LINKINFO_TP_MDIX] && tb[ETHTOOL_A_LINKINFO_TP_MDIX_CTRL]) {
>  		uint8_t mdix = mnl_attr_get_u8(tb[ETHTOOL_A_LINKINFO_TP_MDIX]);
>  		uint8_t mdix_ctrl =
>  			mnl_attr_get_u8(tb[ETHTOOL_A_LINKINFO_TP_MDIX_CTRL]);

It's a bit more complicated, I'm afraid. With current kernel code, both
ETHTOOL_A_LINKINFO_TP_MDIX and ETHTOOL_A_LINKINFO_TP_MDIX_CTRL will be
always present in kernel reply so that we would always show something.
Also, the same condition is also used in ioctl code path and we should
try to keep the output consistent between ioctl and netlink.

How about replacing "port == PORT_TP" with

  (port == TP || mdix != ETH_TP_MDI_INVALID || mdix_ctrl != ETH_TP_MDI_INVALID)

in both code path and probably moving the check into dump_mdix()?

We could (and perhaps should) also modify kernel code to omit
ETHTOOL_A_LINKINFO_TP_MDIX or ETHTOOL_A_LINKINFO_TP_MDIX_CTRL if the
value is ETH_TP_MDI_INVALID but ethtool would still have to check for
both options (absence and ETH_TP_MDI_INVALID value) to preserve
compatibility with older kernels.

Michal

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)