netdev - Re: [PATCH net-next RFC] net: dsa: mv88e6xxx: Correct check for empty list

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6fff92e4-d4e4-4a41-ae5c-5bfb7e72c217@lunn.ch>
Date: Fri, 19 Apr 2024 17:44:03 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Simon Horman <horms@...nel.org>
Cc: Florian Fainelli <f.fainelli@...il.com>,
	Vladimir Oltean <olteanv@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Dan Carpenter <dan.carpenter@...aro.org>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next RFC] net: dsa: mv88e6xxx: Correct check for
 empty list

On Fri, Apr 19, 2024 at 01:17:48PM +0100, Simon Horman wrote:
> Since commit a3c53be55c95 ("net: dsa: mv88e6xxx: Support multiple MDIO
> busses") mv88e6xxx_default_mdio_bus() has checked that the
> return value of list_first_entry() is non-NULL. This appears to be
> intended to guard against the list chip->mdios being empty.
> However, it is not the correct check as the implementation of
> list_first_entry is not designed to return NULL for empty lists.
> 
> Instead check directly if the list is empty.
> 
> Flagged by Smatch
> 
> Signed-off-by: Simon Horman <horms@...nel.org>

Hi Simon

This looks good to me. I would not consider it a fix. As you say, it
has been like this a long time and never bothered anybody, which is
one of the stable rules. It might be possible to have an empty list,
if there are no nodes in DT. But that is something which a novice
would do when writing the DT, and so probably would of reported it.

However, list_first_entry() does document:

* Note, that list is expected to be not empty.

So testing it first is wise.

Reviewed-by: Andrew Lunn <andrew@...n.ch>

Thanks
	Andrew