netdev - Re: [PATCH net] net: dpaa2-switch: disable the control interface on error path

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: <20210819152947.cnx3vyueud6rfupn@skbuf>
Date:   Thu, 19 Aug 2021 15:29:49 +0000
From:   Ioana Ciornei <ioana.ciornei@....com>
To:     Vladimir Oltean <vladimir.oltean@....com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net] net: dpaa2-switch: disable the control interface on
 error path

On Thu, Aug 19, 2021 at 05:17:55PM +0300, Vladimir Oltean wrote:
> Currently dpaa2_switch_takedown has a funny name and does not do the
> opposite of dpaa2_switch_init, which makes probing fail when we need to
> handle an -EPROBE_DEFER.
> 
> A sketch of what dpaa2_switch_init does:
> 
> 	dpsw_open
> 
> 	dpaa2_switch_detect_features
> 
> 	dpsw_reset
> 
> 	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> 		dpsw_if_disable
> 
> 		dpsw_if_set_stp
> 
> 		dpsw_vlan_remove_if_untagged
> 
> 		dpsw_if_set_tci
> 
> 		dpsw_vlan_remove_if
> 	}
> 
> 	dpsw_vlan_remove
> 
> 	alloc_ordered_workqueue
> 
> 	dpsw_fdb_remove
> 
> 	dpaa2_switch_ctrl_if_setup
> 
> When dpaa2_switch_takedown is called from the error path of
> dpaa2_switch_probe(), the control interface, enabled by
> dpaa2_switch_ctrl_if_setup from dpaa2_switch_init, remains enabled,
> because dpaa2_switch_takedown does not call
> dpaa2_switch_ctrl_if_teardown.
> 
> Since dpaa2_switch_probe might fail due to EPROBE_DEFER of a PHY, this
> means that a second probe of the driver will happen with the control
> interface directly enabled.
> 
> This will trigger a second error:
> 
> [   93.273528] fsl_dpaa2_switch dpsw.0: dpsw_ctrl_if_set_pools() failed
> [   93.281966] fsl_dpaa2_switch dpsw.0: fsl_mc_driver_probe failed: -13
> [   93.288323] fsl_dpaa2_switch: probe of dpsw.0 failed with error -13
> 
> Which if we investigate the /dev/dpaa2_mc_console log, we find out is
> caused by:
> 
> [E, ctrl_if_set_pools:2211, DPMNG]  ctrl_if must be disabled
> 
> So make dpaa2_switch_takedown do the opposite of dpaa2_switch_init (in
> reasonable limits, no reason to change STP state, re-add VLANs etc), and
> rename it to something more conventional, like dpaa2_switch_teardown.
> 
> Fixes: 613c0a5810b7 ("staging: dpaa2-switch: enable the control interface")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>

Reviewed-by: Ioana Ciornei <ioana.ciornei@....com>

Thanks!