linux-kernel - Re: [PATCH net] net: pse-pd: pd692x0: Fix power limit retrieval

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: <20250217171500.0fd4a519@kmaincent-XPS-13-7390>
Date: Mon, 17 Feb 2025 17:15:00 +0100
From: Kory Maincent <kory.maincent@...tlin.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Jakub Kicinski <kuba@...nel.org>, Oleksij Rempel
 <o.rempel@...gutronix.de>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, thomas.petazzoni@...tlin.com, Andrew Lunn
 <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net] net: pse-pd: pd692x0: Fix power limit retrieval

On Mon, 17 Feb 2025 16:24:44 +0100
Andrew Lunn <andrew@...n.ch> wrote:

> On Mon, Feb 17, 2025 at 02:48:11PM +0100, Kory Maincent wrote:
> > Fix incorrect data offset read in the pd692x0_pi_get_pw_limit callback.
> > The issue was previously unnoticed as it was only used by the regulator
> > API and not thoroughly tested, since the PSE is mainly controlled via
> > ethtool.
> > 
> > The function became actively used by ethtool after commit 3e9dbfec4998
> > ("net: pse-pd: Split ethtool_get_status into multiple callbacks"),
> > which led to the discovery of this issue.
> > 
> > Fix it by using the correct data offset.
> > 
> > Fixes: a87e699c9d33 ("net: pse-pd: pd692x0: Enhance with new current limit
> > and voltage read callbacks") Signed-off-by: Kory Maincent
> > <kory.maincent@...tlin.com> ---
> >  drivers/net/pse-pd/pd692x0.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c
> > index fc9e23927b3b..7d60a714ca53 100644
> > --- a/drivers/net/pse-pd/pd692x0.c
> > +++ b/drivers/net/pse-pd/pd692x0.c
> > @@ -1047,7 +1047,7 @@ static int pd692x0_pi_get_pw_limit(struct
> > pse_controller_dev *pcdev, if (ret < 0)
> >  		return ret;
> >  
> > -	return pd692x0_pi_get_pw_from_table(buf.data[2], buf.data[3]);
> > +	return pd692x0_pi_get_pw_from_table(buf.data[0], buf.data[1]);  
> 
> Would the issue of been more obvious if some #defines were used,
> rather than magic numbers?

We would need lots of defines as the offset of the useful data can
change between each command. Don't know if it would have been better. 

On my current patch priority series.
git grep "buf\." drivers/net/pse-pd/pd692x0.c | wc -l
29

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com