Re: [PATCH 1/1] fix memory leak during adapter release by destroying gid_pn_req

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The error path in `zfcp_adapter_enqueue()` calls `zfcp_erp_thread_kill()` which unconditionally passes `adapter->erp_thread` to `kthread_stop()`. If adapter initialization fails before the thread is created (e.g., during `zfcp_allocate_low_mem_buffers()`), this results in a kernel panic because `kthread_stop()` does not handle NULL pointers.
--

--- Patch [1]: [PATCH 1/1] fix memory leak during adapter release by destroying gid_pn_req ---
commit 5b4668bd066d14a43f77f6f0301f8c98cbb5069c
Author: Benjamin Block <bblock@linux.ibm.com>

fix memory leak during adapter release by destroying gid_pn_req

When releasing an adapter we don't free the mempool gid_pn_req that is
allocated during the enqueue. This fixes the memory leak by destroying the
mempool during the adapter's release.

> diff --git a/drivers/s390/scsi/zfcp_aux.c b/drivers/s390/scsi/zfcp_aux.c
> index 8ff7db7921b5a..fea573e00f9df 100644
> --- a/drivers/s390/scsi/zfcp_aux.c
> +++ b/drivers/s390/scsi/zfcp_aux.c
> @@ -253,6 +253,7 @@ static int zfcp_allocate_low_mem_buffers(struct zfcp_adapter *adapter)
>  static void zfcp_free_low_mem_buffers(struct zfcp_adapter *adapter)
>  {
>  	mempool_destroy(adapter->pool.erp_req);
> +	mempool_destroy(adapter->pool.gid_pn_req);

[Severity: High]
This is a pre-existing issue, but if memory allocation fails during adapter
setup, could this lead to a kernel panic?

In zfcp_adapter_enqueue(), if zfcp_allocate_low_mem_buffers() returns -ENOMEM,
the code jumps to the failed label and unconditionally calls
zfcp_erp_thread_kill():

drivers/s390/scsi/zfcp_aux.c:zfcp_adapter_enqueue() {
    ...
failed:
    /* TODO: make this more fine-granular */
    cancel_delayed_work_sync(&adapter->scan_work);
    cancel_work_sync(&adapter->stat_work);
    cancel_work_sync(&adapter->ns_up_work);
    cancel_work_sync(&adapter->version_change_lost_work);
    zfcp_destroy_adapter_work_queue(adapter);

    zfcp_fc_wka_ports_force_offline(adapter->gs);
    zfcp_scsi_adapter_unregister(adapter);

    zfcp_erp_thread_kill(adapter);
    ...
}

However, if the thread was never created, adapter->erp_thread is NULL.
zfcp_erp_thread_kill() unconditionally passes it to kthread_stop():

drivers/s390/scsi/zfcp_erp.c:zfcp_erp_thread_kill() {
    kthread_stop(adapter->erp_thread);
    adapter->erp_thread = NULL;
}

Since kthread_stop() does not handle NULL pointers, will this result in a
NULL pointer dereference?

>  	mempool_destroy(adapter->pool.scsi_req);
>  	mempool_destroy(adapter->pool.scsi_abort);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720072736.3381816-2-niharp@linux.ibm.com?part=1