diff mbox series

[net] net: lan966x: fix kernel oops on ioctl when I/F is down

Message ID 20220326000251.2687897-1-michael@walle.cc (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net] net: lan966x: fix kernel oops on ioctl when I/F is down | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Michael Walle March 26, 2022, 12:02 a.m. UTC
A SIOCGMIIPHY ioctl will cause a kernel oops when the interface is down.
Fix it by checking the state and if it's no running, return an error.

Fixes: 735fec995b21 ("net: lan966x: Implement SIOCSHWTSTAMP and SIOCGHWTSTAMP")
Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/net/ethernet/microchip/lan966x/lan966x_main.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Andrew Lunn March 26, 2022, 2:17 a.m. UTC | #1
On Sat, Mar 26, 2022 at 01:02:51AM +0100, Michael Walle wrote:
> A SIOCGMIIPHY ioctl will cause a kernel oops when the interface is down.
> Fix it by checking the state and if it's no running, return an error.

s/no/not/

I don't think it is just SIOCGMIIPHY. phy_has_hwtstamp(dev->phydev) is
probably also an issue. The phy is connected in open, and disconnected
in stop. So dev->phydev is not valid outside of that time.

But i'm also not sure it is guaranteed to be valid while the interface
is up. The driver uses phylink, so there could be an SFP attached to a
port, in which case, dev->phydev will not be set.

So rather than testing of running, it would be better to test if the
phydev is NULL or not.

       Andrew

> 
> Fixes: 735fec995b21 ("net: lan966x: Implement SIOCSHWTSTAMP and SIOCGHWTSTAMP")
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  drivers/net/ethernet/microchip/lan966x/lan966x_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> index ec42e526f6fb..0adf49d19142 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> @@ -399,6 +399,9 @@ static int lan966x_port_ioctl(struct net_device *dev, struct ifreq *ifr,
>  {
>  	struct lan966x_port *port = netdev_priv(dev);
>  
> +	if (!netif_running(dev))
> +		return -EINVAL;
> +
>  	if (!phy_has_hwtstamp(dev->phydev) && port->lan966x->ptp) {
>  		switch (cmd) {
>  		case SIOCSHWTSTAMP:
> -- 
> 2.30.2
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
index ec42e526f6fb..0adf49d19142 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
@@ -399,6 +399,9 @@  static int lan966x_port_ioctl(struct net_device *dev, struct ifreq *ifr,
 {
 	struct lan966x_port *port = netdev_priv(dev);
 
+	if (!netif_running(dev))
+		return -EINVAL;
+
 	if (!phy_has_hwtstamp(dev->phydev) && port->lan966x->ptp) {
 		switch (cmd) {
 		case SIOCSHWTSTAMP: