Re: [PATCH v19 net-next 1/2] octeontx2: use atomic bitops for PF/VF and rep flags

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net/octeontx2: Data race losing atomic flags in otx2_sync_flags_from_rep()
--

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

octeontx2: use atomic bitops for PF/VF and rep flags

This commit switches PF/VF and representor flags from non-atomic u64 masks
to unsigned long bitmaps. It also introduces otx2_sync_flags_from_rep to
sync representor-owned flags onto the PF mailbox context locklessly.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
[ ... ]
> @@ -593,6 +607,32 @@ struct otx2_nic {
[ ... ]
> +static inline void otx2_sync_flags_from_rep(struct otx2_nic *dst,
> +					    unsigned long *src_flags)
> +{
> +	unsigned long src = READ_ONCE(*src_flags);
> +	unsigned long new_flags;
> +
> +	new_flags = (READ_ONCE(dst->flags) & ~OTX2_REP_SYNC_FLAGS_MASK) |
> +		    (src & OTX2_REP_SYNC_FLAGS_MASK);
> +	WRITE_ONCE(dst->flags, new_flags);
> +}

[Severity: High]
Does this non-atomic read-modify-write risk overwriting and losing
concurrent atomic bit operations?

If a concurrent thread sets a flag asynchronously using atomic bitops,
such as OTX2_FLAG_PORT_UP in otx2_mbox_up_handler_rep_event_up_notify():

drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:otx2_mbox_up_handler_rep_event_up_notify() {
    ...
    if (info->evt_data.port_state) {
        otx2_set_flag(pf, OTX2_FLAG_PORT_UP);
        netif_carrier_on(netdev);
    ...
}

or OTX2_FLAG_INTF_DOWN in rvu_rep_destroy():

drivers/net/ethernet/marvell/octeontx2/nic/rep.c:rvu_rep_destroy() {
    ...
    rvu_eswitch_config(priv, false);
    otx2_set_flag(priv, OTX2_FLAG_INTF_DOWN);
    ...
}

it appears the atomic change will be permanently lost if it occurs between
the READ_ONCE() and WRITE_ONCE() in otx2_sync_flags_from_rep(). 

Can losing state transitions like OTX2_FLAG_INTF_DOWN allow async routines
like otx2_napi_handler() to continue executing on teardown queues that
have already been freed, leading to a use-after-free?

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