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

Jakub Kicinski <[email protected]>
Newsgroups org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <[email protected]>
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 <[email protected]>

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;
> +}
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.