Re: [PATCH v17 net-next 2/2] octeontx2: add mqprio bandwidth offload for NIX TX schedulers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] core: race condition and UAF in teardown due to inverted netdev unregister ordering
--

commit ccdaeef10f51af218ecef73bdbd0e10716a461fd
Author: Ratheesh Kannoth <rkannoth@marvell.com>

octeontx2: add mqprio bandwidth offload for NIX TX schedulers

This commit adds TC_SETUP_QDISC_MQPRIO offload with
TC_MQPRIO_SHAPER_BW_RATE on PF and VF netdevs. It programs per-queue
MDQ CIR/PIR via the NIX TX scheduler mailbox and caches rates in
software.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
[ ... ]
> @@ -3420,11 +3439,13 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	otx2_sriov_vfcfg_cleanup(pf);
>  err_pf_sriov_init:
>  	otx2_unregister_dl(pf);
> -	otx2_shutdown_tc(pf);
>  err_mcam_flow_del:
>  	otx2_mcam_flow_del(pf);
>  err_unreg_netdev:
> +	otx2_shutdown_tc(pf);
>  	unregister_netdev(netdev);

[Severity: High]
Does this sequence introduce a use-after-free race during teardown?

By moving otx2_shutdown_tc() before unregister_netdev(), the device is
still visible to the networking stack and can receive RTNL-protected
configuration commands like ndo_setup_tc.

If a concurrent tc qdisc command executes ndo_setup_tc() right after
otx2_shutdown_tc() has run, it could access freed structures like
cur_mq_snap or re-arm mqprio.netdev_tc_work, which would then execute
after the device is completely destroyed. Should unregister_netdev()
be called first to provide the RTNL barrier before destroying TC
resources?

[ ... ]
> @@ -3622,6 +3643,7 @@ static void otx2_remove(struct pci_dev *pdev)
>  	otx2_cgx_config_linkevents(pf, false);
>  
>  	otx2_unregister_dl(pf);
> +	otx2_shutdown_tc(pf);
>  	unregister_netdev(netdev);

[Severity: High]
Is the same race condition present here in the PF removal path?

It appears otx2_shutdown_tc() frees TC structures and cancels workqueues
without holding RTNL, while unregister_netdev() is what prevents further
ndo_ops from executing. Moving otx2_shutdown_tc() above
unregister_netdev() seems to break the LIFO teardown order.

[ ... ]
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
[ ... ]
> @@ -840,6 +842,7 @@ static void otx2vf_remove(struct pci_dev *pdev)
>  #endif
>  
>  	otx2_unregister_dl(vf);
> +	otx2_shutdown_tc(vf);
>  	unregister_netdev(netdev);

[Severity: High]
Could this VF teardown sequence trigger the same use-after-free if
a user space process configures mqprio offload concurrently with driver
removal?

Like the PF driver, otx2_shutdown_tc() is called before the netdev
unregistration barrier.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923032217.1732753-1-rkannoth@marvell.com?part=2