linux-kernel - [PATCH] scsi: qla2xxx: Fix memory leaks in the error handling path of 'qla2x00_mem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 10 Nov 2021 22:11:34 +0100
From:   Christophe JAILLET <christophe.jaillet@...adoo.fr>
To:     njavali@...vell.com, GR-QLogic-Storage-Upstream@...vell.com,
        jejb@...ux.ibm.com, martin.petersen@...cle.com,
        gmalavali@...vell.com, hmadhani@...vell.com
Cc:     linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-janitors@...r.kernel.org,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>
Subject: [PATCH] scsi: qla2xxx: Fix memory leaks in the error handling path of 'qla2x00_mem_alloc()'

In case of memory allocation failure, we should release many things and
should not return directly.

The tricky part here, is that some (kzalloc + dma_pool_alloc) resources
are allocated and stored in 'unusable' and a 'good' list.
The 'good' list is then freed and only the 'unusable' list remains
allocated.
So, only this 'unusable' list is then freed in the error handling path of
the function.

So, instead of adding even more code in this already huge function, just
'continue' (as already done if dma_pool_alloc() fails) instead of
returning directly.

After the 'for' loop, we will then branch to the correct place of the
error handling path when another memory allocation will (likely) fail
afterward.

Fixes: 50b812755e97 ("scsi: qla2xxx: Fix DMA error when the DIF sg buffer crosses 4GB boundary")
Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
---
Certainly not the best solution, but look 'safe' to me.
---
 drivers/scsi/qla2xxx/qla_os.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index abcd30917263..0722dd618b99 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -4151,7 +4151,7 @@ qla2x00_mem_alloc(struct qla_hw_data *ha, uint16_t req_len, uint16_t rsp_len,
 					ql_dbg_pci(ql_dbg_init, ha->pdev,
 					    0xe0ee, "%s: failed alloc dsd\n",
 					    __func__);
-					return -ENOMEM;
+					continue;
 				}
 				ha->dif_bundle_kallocs++;

-- 
2.30.2