| Message ID | alpine.LRH.2.21.1803081544410.23373@math.ut.ee (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Mar 08, 2018 at 08:45:25AM, Meelis Roos wrote: > When firmware init fails, qla2x00_probe_one() does double free of req and rsp > queues and possibly other structures allocated by qla2x00_mem_alloc(). > Fix it by pulling out qla2x00_mem_free() and qla2x00_free_queues() invocations > from qla2x00_free_device() and call them manually where needed, and also zero > the req and rsp pointers after freeing them once in the error handler of > qla2x00_probe_one(). > This fixes memory corruption and further crashes in unrelated code when qla2200 > init fails for some reason. > Signed-off-by: Meelis Roos <mroos@linux.ee> Hi Meelis, This issue should already be addressed by a very recent commit: 6a2cf8d3663e13e1 scsi: qla2xxx: Fix crashes in qla2x00_probe_one on probe failure Furthermore, the additions in qla2x00_remove_one of: + qla2x00_mem_free(ha); + + qla2x00_free_queues(ha); + are unnecessary. These routines are already called by qla2x00_free_device just above in qla2x00_remove_one. Regards, -bk
> Hi Meelis, > > This issue should already be addressed by a very recent commit: > > 6a2cf8d3663e13e1 scsi: qla2xxx: Fix crashes in qla2x00_probe_one on probe failure Good, will test. > Furthermore, the additions in qla2x00_remove_one of: > > + qla2x00_mem_free(ha); > + > + qla2x00_free_queues(ha); > + > > are unnecessary. These routines are already called by qla2x00_free_device just above > in qla2x00_remove_one. No, that was the point of my changes - they must not be called from qla2x00_free_device or they will be double freed in some cases.
> No, that was the point of my changes - they must not be called from > qla2x00_free_device or they will be double freed in some cases. Meelis, The patch I referred you to makes those routines idempotent, so they can be called multiple times without harm. Thanks and regards, -Bill Kuzeja
> Hi Meelis, > > This issue should already be addressed by a very recent commit: > > 6a2cf8d3663e13e1 scsi: qla2xxx: Fix crashes in qla2x00_probe_one on probe failure What tree is that commit in?
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c index 3c7bc2d..2ec21b2 100644 --- a/drivers/scsi/qla2xxx/qla_os.c +++ b/drivers/scsi/qla2xxx/qla_os.c @@ -3389,9 +3389,11 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct pci_device_id *id) probe_init_failed: qla2x00_free_req_que(ha, req); + req = NULL; ha->req_q_map[0] = NULL; clear_bit(0, ha->req_qid_map); qla2x00_free_rsp_que(ha, rsp); + rsp = NULL; ha->rsp_q_map[0] = NULL; clear_bit(0, ha->rsp_qid_map); ha->max_req_queues = ha->max_rsp_queues = 0; @@ -3689,6 +3691,10 @@ qla2x00_remove_one(struct pci_dev *pdev) qla2x00_free_device(base_vha); + qla2x00_mem_free(ha); + + qla2x00_free_queues(ha); + qla2x00_clear_drv_active(ha); scsi_host_put(base_vha->host); @@ -3752,12 +3758,7 @@ qla2x00_free_device(scsi_qla_host_t *vha) ha->wq = NULL; } - - qla2x00_mem_free(ha); - qla82xx_md_free(vha); - - qla2x00_free_queues(ha); } void qla2x00_free_fcports(struct scsi_qla_host *vha)
When firmware init fails, qla2x00_probe_one() does double free of req and rsp queues and possibly other structures allocated by qla2x00_mem_alloc(). Fix it by pulling out qla2x00_mem_free() and qla2x00_free_queues() invocations from qla2x00_free_device() and call them manually where needed, and also zero the req and rsp pointers after freeing them once in the error handler of qla2x00_probe_one(). This fixes memory corruption and further crashes in unrelated code when qla2200 init fails for some reason. Signed-off-by: Meelis Roos <mroos@linux.ee> --- drivers/scsi/qla2xxx/qla_os.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)