[PATCH next] ice, irdma: fix an off by one in error handling code

Dan Carpenter posted 1 patch 12 months ago
drivers/infiniband/hw/irdma/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH next] ice, irdma: fix an off by one in error handling code
Posted by Dan Carpenter 12 months ago
If we don't allocate the MIN number of IRQs then we need to free what
we have and return -ENOMEM.  The problem is this loop is off by one
so it frees an entry that wasn't allocated and it doesn't free the
first entry where i == 0.

Fixes: 3e0d3cb3fbe0 ("ice, irdma: move interrupts code to irdma")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/infiniband/hw/irdma/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/irdma/main.c b/drivers/infiniband/hw/irdma/main.c
index 1ee8969595d3..5fc081ca8905 100644
--- a/drivers/infiniband/hw/irdma/main.c
+++ b/drivers/infiniband/hw/irdma/main.c
@@ -221,7 +221,7 @@ static int irdma_init_interrupts(struct irdma_pci_f *rf, struct ice_pf *pf)
 			break;
 
 	if (i < IRDMA_MIN_MSIX) {
-		for (; i > 0; i--)
+		while (--i >= 0)
 			ice_free_rdma_qvector(pf, &rf->msix_entries[i]);
 
 		kfree(rf->msix_entries);
-- 
2.47.2
Re: [PATCH next] ice, irdma: fix an off by one in error handling code
Posted by Tony Nguyen 12 months ago

On 2/12/2025 7:25 AM, Dan Carpenter wrote:
> If we don't allocate the MIN number of IRQs then we need to free what
> we have and return -ENOMEM.  The problem is this loop is off by one
> so it frees an entry that wasn't allocated and it doesn't free the
> first entry where i == 0.
> 
> Fixes: 3e0d3cb3fbe0 ("ice, irdma: move interrupts code to irdma")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>

Thanks for the patch Dan. I've applied this to iwl-next since it 
originated from there and I'd like to bundle this with the other fixes 
related to the series.

Thanks,
Tony

> ---
>   drivers/infiniband/hw/irdma/main.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/irdma/main.c b/drivers/infiniband/hw/irdma/main.c
> index 1ee8969595d3..5fc081ca8905 100644
> --- a/drivers/infiniband/hw/irdma/main.c
> +++ b/drivers/infiniband/hw/irdma/main.c
> @@ -221,7 +221,7 @@ static int irdma_init_interrupts(struct irdma_pci_f *rf, struct ice_pf *pf)
>   			break;
>   
>   	if (i < IRDMA_MIN_MSIX) {
> -		for (; i > 0; i--)
> +		while (--i >= 0)
>   			ice_free_rdma_qvector(pf, &rf->msix_entries[i]);
>   
>   		kfree(rf->msix_entries);
Re: [PATCH next] ice, irdma: fix an off by one in error handling code
Posted by Michal Swiatkowski 12 months ago
On Wed, Feb 12, 2025 at 06:25:44PM +0300, Dan Carpenter wrote:
> If we don't allocate the MIN number of IRQs then we need to free what
> we have and return -ENOMEM.  The problem is this loop is off by one
> so it frees an entry that wasn't allocated and it doesn't free the
> first entry where i == 0.
> 
> Fixes: 3e0d3cb3fbe0 ("ice, irdma: move interrupts code to irdma")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  drivers/infiniband/hw/irdma/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/irdma/main.c b/drivers/infiniband/hw/irdma/main.c
> index 1ee8969595d3..5fc081ca8905 100644
> --- a/drivers/infiniband/hw/irdma/main.c
> +++ b/drivers/infiniband/hw/irdma/main.c
> @@ -221,7 +221,7 @@ static int irdma_init_interrupts(struct irdma_pci_f *rf, struct ice_pf *pf)
>  			break;
>  
>  	if (i < IRDMA_MIN_MSIX) {
> -		for (; i > 0; i--)
> +		while (--i >= 0)
>  			ice_free_rdma_qvector(pf, &rf->msix_entries[i]);
>  
>  		kfree(rf->msix_entries);

Przemek pointed that in the review, but somehow I missed that :( .
Thanks for fixing:
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

> -- 
> 2.47.2
>