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
| ||
|
Message-ID: <20231115181858.GY74656@kernel.org> Date: Wed, 15 Nov 2023 18:18:58 +0000 From: Simon Horman <horms@...nel.org> To: Romain Gantois <romain.gantois@...tlin.com> Cc: davem@...emloft.net, Rob Herring <robh+dt@...nel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Jakub Kicinski <kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, thomas.petazzoni@...tlin.com, Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, Heiner Kallweit <hkallweit1@...il.com>, Russell King <linux@...linux.org.uk>, linux-arm-kernel@...ts.infradead.org, Vladimir Oltean <vladimir.oltean@....com>, Luka Perkov <luka.perkov@...tura.hr>, Robert Marko <robert.marko@...tura.hr>, Andy Gross <agross@...nel.org>, Bjorn Andersson <andersson@...nel.org>, Konrad Dybcio <konrad.dybcio@...ainline.org> Subject: Re: [PATCH net-next v3 4/8] net: qualcomm: ipqess: Add Ethtool ops to IPQESS port netdevices On Tue, Nov 14, 2023 at 11:55:54AM +0100, Romain Gantois wrote: > The IPQESS driver registers one netdevice for each front-facing switch > port. Add support for several ethtool operations to these netdevices. > > Signed-off-by: Romain Gantois <romain.gantois@...tlin.com> Hi Romain, some more minor feedback from my side. > --- > drivers/net/ethernet/qualcomm/ipqess/Makefile | 2 +- > .../ethernet/qualcomm/ipqess/ipqess_ethtool.c | 245 ++++++++++++++++++ > .../ethernet/qualcomm/ipqess/ipqess_port.c | 1 + > .../ethernet/qualcomm/ipqess/ipqess_port.h | 3 + > 4 files changed, 250 insertions(+), 1 deletion(-) > create mode 100644 drivers/net/ethernet/qualcomm/ipqess/ipqess_ethtool.c ... > diff --git a/drivers/net/ethernet/qualcomm/ipqess/ipqess_ethtool.c b/drivers/net/ethernet/qualcomm/ipqess/ipqess_ethtool.c ... > +static int ipqess_port_set_eee(struct net_device *dev, struct ethtool_eee *eee) > +{ > + struct ipqess_port *port = netdev_priv(dev); > + int ret; > + u32 lpi_en = QCA8K_REG_EEE_CTRL_LPI_EN(port->index); > + struct qca8k_priv *priv = port->sw->priv; > + u32 lpi_ctl1; nit: Please consider using reverse xmas tree - longest line to shortest - for local variable declarations in networking code. > + > + /* Port's PHY and MAC both need to be EEE capable */ > + if (!dev->phydev || !port->pl) > + return -ENODEV; > + > + mutex_lock(&priv->reg_mutex); > + lpi_ctl1 = qca8k_read(priv, QCA8K_REG_EEE_CTRL, &lpi_ctl1); > + if (lpi_ctl1 < 0) { lpi_ctl1 is unsigned, it can never be less than zero. As flagged by Smatch and Coccinelle. I think this should probably be (completely untested!): ret = qca8k_read(priv, QCA8K_REG_EEE_CTRL, &lpi_ctl1); if (ret < 0) { Which would also resolve the issue immediately below too. > + mutex_unlock(&priv->reg_mutex); > + return ret; It seems that ret is used uninitialised here. Flagged by clang-16 W=1 builds. > + } ...
Powered by blists - more mailing lists