Hello Maurizio,
Thanks for looking into this. The patch looks good, but I would like to
extend it with the following allocation failure portion to completely
remove the memory leak. Please incorporate this into your next
submission.
Thanks,
Eddie
@@ -2020,7 +2026,7 @@ static int bnx2fc_allocate_hash_table(struct
bnx2fc_hba *hba)
dma_segment_array = kzalloc(dma_segment_array_size, GFP_KERNEL);
if (!dma_segment_array) {
printk(KERN_ERR PFX "hash table pointers (dma) alloc
failed\n");
- return -ENOMEM;
+ goto free_hash_tbl_seg;
}
for (i = 0; i < segment_count; ++i) {
@@ -2039,7 +2045,7 @@ static int bnx2fc_allocate_hash_table(struct
bnx2fc_hba *hba)
hba->hash_tbl_segments[i] = NULL;
}
kfree(dma_segment_array);
- return -ENOMEM;
+ goto free_hash_tbl_seg;
}
memset(hba->hash_tbl_segments[i], 0,
BNX2FC_HASH_TBL_CHUNK_SIZE);
@@ -2077,6 +2083,11 @@ static int bnx2fc_allocate_hash_table(struct
bnx2fc_hba *hba)
}
kfree(dma_segment_array);
return 0;
+
+free_hash_tbl_seg:
+ kfree(hba->hash_tbl_segments);
+ hba->hash_tbl_segments = NULL;
+ return -ENOMEM;
}
On Fri, 2014-03-07 at 15:37 +0100, Maurizio Lombardi wrote:
> If bnx2fc_allocate_hash_table() for some reasons fails, it is possible that
> the
> hash_tbl_segments or the hash_tbl_pbl pointers are NULL.
> In this case bnx2fc_free_hash_table() will panic the system.
>
> this patch also fixes a memory leak, the hash_tbl_segments pointer was never
> freed.
>
> Signed-off-by: Maurizio Lombardi <[email protected]>
> ---
> drivers/scsi/bnx2fc/bnx2fc_hwi.c | 34 ++++++++++++++++++++--------------
> 1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_hwi.c
> b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
> index 261af2a..f83bae4 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_hwi.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
> @@ -1968,21 +1968,27 @@ static void bnx2fc_free_hash_table(struct bnx2fc_hba
> *hba)
> int segment_count;
> u32 *pbl;
>
> - segment_count = hba->hash_tbl_segment_count;
> -
> - pbl = hba->hash_tbl_pbl;
> - for (i = 0; i < segment_count; ++i) {
> - dma_addr_t dma_address;
> -
> - dma_address = le32_to_cpu(*pbl);
> - ++pbl;
> - dma_address += ((u64)le32_to_cpu(*pbl)) << 32;
> - ++pbl;
> - dma_free_coherent(&hba->pcidev->dev,
> - BNX2FC_HASH_TBL_CHUNK_SIZE,
> - hba->hash_tbl_segments[i],
> - dma_address);
> + if (hba->hash_tbl_segments) {
> +
> + pbl = hba->hash_tbl_pbl;
> + if (pbl) {
> + segment_count = hba->hash_tbl_segment_count;
> + for (i = 0; i < segment_count; ++i) {
> + dma_addr_t dma_address;
> +
> + dma_address = le32_to_cpu(*pbl);
> + ++pbl;
> + dma_address += ((u64)le32_to_cpu(*pbl)) << 32;
> + ++pbl;
> + dma_free_coherent(&hba->pcidev->dev,
> + BNX2FC_HASH_TBL_CHUNK_SIZE,
> + hba->hash_tbl_segments[i],
> + dma_address);
> + }
> + }
>
> + kfree(hba->hash_tbl_segments);
> + hba->hash_tbl_segments = NULL;
> }
>
> if (hba->hash_tbl_pbl) {
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
