Re: [Scst-devel] [PATCH 10/15] scsi: qla2xxx: fix TMR failure handling

[Tony B. <to...@cy...>]

On 9/12/25 10:36, Dmitry Bogdanov wrote:
> On Mon, Sep 08, 2025 at 03:02:49PM -0400, Tony Battersby wrote:
>> If handle_tmr() fails (e.g. -ENOMEM):
>> - qlt_send_busy() makes no sense because it sends a SCSI command
>>   response instead of a TMR response.
> There is not only -ENOMEM can be returned by handle_tmr.

Indeed.  I will remove mention of -ENOMEM since it isn't really relevant.

>> +               mcmd->fc_tm_rsp = FCP_TMF_REJECTED;
>>
> FCP_TMF_REJECTED means that this TMF is not supported, FCP_TMF_FAILED is
> more appretiate here.

I will make that change.

>> - Calling mempool_free() directly can lead to memory-use-after-free.
> No, it is a API contract between modules. If handle_tmr returned an error,
> then the caller of handle_tmr is responsible to make a cleanup.
> Otherwise, target module (tcm_qla2xxx) is responsible. The same rule is
> for handle_cmd.
>> +               qlt_xmit_tm_rsp(mcmd);
> qlt_xmit_tm_rsp does not free mcmd for TMF ABORT. So you introduce a memleak.

I just tested it, and there is no memleak.  qlt_build_abts_resp_iocb()
sets req->outstanding_cmds[h] to mcmd, and then
qlt_handle_abts_completion() calls ->free_mcmd after getting a response
from the ISP.

The original code had a memory-use-after-free by calling
qlt_build_abts_resp_iocb() and then mempool_free(), and
then qlt_handle_abts_completion() used the freed mcmd.  I can reword the
commit message to make this clearer.

Tony