linux-kernel - Re: [net PATCH v2] net: ethernet: sunplus: Switch to ndo_eth

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

Message-ID: <acab12d4-418f-4a94-aca4-705bab8e4a90@lunn.ch>
Date: Thu, 9 Jan 2025 15:03:39 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Yeking@...54.com
Cc: kuba@...nel.org, andrew+netdev@...n.ch, davem@...emloft.net,
	edumazet@...gle.com, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, pabeni@...hat.com, wellslutw@...il.com
Subject: Re: [net PATCH v2] net: ethernet: sunplus: Switch to ndo_eth_ioctl

On Thu, Jan 09, 2025 at 02:05:52AM +0000, Yeking@...54.com wrote:
> From: 谢致邦 (XIE Zhibang) <Yeking@...54.com>
> 
> ndo_do_ioctl is no longer called by the device ioctl handler,
> so use ndo_eth_ioctl instead. (found by code inspection)

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

There are a couple of things which would of been nice to add, but the
patch can be accepted anyway.

One is a bit more details in the commit message about when
ndo_do_ioctl is actually used, just to help make it clearer why the
code is wrong and the patch is correct. You could of quoted
a76053707dbf:

    Most users of ndo_do_ioctl are ethernet drivers that implement
    the MII commands SIOCGMIIPHY/SIOCGMIIREG/SIOCSMIIREG, or hardware
    timestamping with SIOCSHWTSTAMP/SIOCGHWTSTAMP.

    Separate these from the few drivers that use ndo_do_ioctl to
    implement SIOCBOND, SIOCBR and SIOCWANDEV commands.

Also, if i find a bug like this, i often wounder if there are more
instances of the same bug somewhere else. I did a quick grep and it
does seem to be the only case. If you did the same check, it would be
nice to mention this in the commit message, maybe below the --- marker
so it does not become part of the permanent history in git.

    Andrew