Re: [PATCH v22 net-next 06/12] net/nebula-matrix: add intr resource implementation

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
--- Patch [6]: [PATCH v22 net-next 06/12] net/nebula-matrix: add intr resource implementation ---
commit 32d01aebd4d29040305943012638ac9e59c45fb8
Author: illusion wang <illusion.wang@xxxxxxxxxxxxxxxxx>

net/nebula-matrix: add intr resource implementation
    
This commit adds the nbl_interrupt module to manage driver global MSI-X
vector indices. It extends hw_ops with callbacks to program vendor-specific
internal MSI-X mapping registers, configure MSI-X metadata, and bind
interrupts to PF mailbox channels.

> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c
> new file mode 100644
> index 0000000000000..cb6874d13a88f
> --- /dev/null
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c

[ ... ]

> @@ -0,0 +1,335 @@

[ ... ]

> +release_net_bits:
> +	while (i--) {
> +		intr_index = tmp_interrupts[i];
> +		if (intr_index >= NBL_NET_INTR_BASE)
> +			clear_bit(intr_index - NBL_NET_INTR_BASE,
> +				  intr_mgt->interrupt_net_bitmap);
> +		else
> +			clear_bit(intr_index,
> +				  intr_mgt->interrupt_others_bitmap);
> +	}
> +	i = num_net_msix;
> +release_all_bits:
> +	while (i--) {

[Severity: High]
Does this fall-through inadvertently corrupt the global interrupt bitmaps?

If resource allocation fails in the first loop for net interrupts, the code
jumps to release_net_bits, loops i down to 0, sets i = num_net_msix, and then
falls through into release_all_bits.

Because tmp_interrupts was allocated with devm_kcalloc, any uninitialized
entries are 0. When the second loop evaluates these uninitialized zero
values, 0 < NBL_NET_INTR_BASE, meaning the code will clear bit 0 in
interrupt_others_bitmap and also doubly clear the already-released net bits.

Should there be a jump to free_prealloc right before release_all_bits to
avoid evaluating the array twice?

> +		intr_index = tmp_interrupts[i];
> +		if (intr_index >= NBL_NET_INTR_BASE)
> +			clear_bit(intr_index - NBL_NET_INTR_BASE,
> +				  intr_mgt->interrupt_net_bitmap);
> +		else
> +			clear_bit(intr_index,
> +				  intr_mgt->interrupt_others_bitmap);
> +	}
> +	goto free_prealloc;
> +}