linux-kernel - Re: [PATCH 1/3] leds: trigger: netdev: add additional specific link speed mode

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: <b4d25621-f89a-4b48-9a00-33274f130e8c@lunn.ch>
Date:   Fri, 9 Jun 2023 16:25:14 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Christian Marangi <ansuelsmth@...il.com>
Cc:     Pavel Machek <pavel@....cz>, Lee Jones <lee@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Yang Li <yang.lee@...ux.alibaba.com>,
        linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] leds: trigger: netdev: add additional specific link
 speed mode

> +	if (trigger_data->net_dev != NULL) {
> +		struct ethtool_link_ksettings cmd;
> +

cmd is a stack variable, so contains random junk:

>  		trigger_data->carrier_link_up = netif_carrier_ok(trigger_data->net_dev);
>  
> +		if (trigger_data->carrier_link_up) {
> +			rtnl_lock();
> +			__ethtool_get_link_ksettings(trigger_data->net_dev, &cmd);

/* Internal kernel helper to query a device ethtool_link_settings. */
int __ethtool_get_link_ksettings(struct net_device *dev,
				 struct ethtool_link_ksettings *link_ksettings)
{
	ASSERT_RTNL();

	if (!dev->ethtool_ops->get_link_ksettings)
		return -EOPNOTSUPP;

If the op is not implemented, it just returns.


> +			rtnl_unlock();
> +
> +			trigger_data->link_speed = cmd.base.speed;

and now you are accessing the random junk.

> +			__ethtool_get_link_ksettings(trigger_data->net_dev, &cmd);
> +
> +			trigger_data->link_speed = cmd.base.speed;

You have this code three times. I suggest you pull it out into a
little helper, and within the helper, deal with the return code, and
set speed to 0 if it is unknown.

    Andrew