Skip to contentSkip to conversation

zfcp bugfix for 7.2

PATCH Applied Nihar Panda 3 participants 5 messages Jul 20 – Jul 29

Subsystems (3)
SCSI s390 s390 zfcp driver
lore
Share & tools
lore

Nihar Panda

author maintainer
#0 lore raw

cover zfcp bugfix for 7.2

Hi Martin and James,
we have a zfcp bugfix that address memory leak in zfcp. It would be great if this could be included in v7.2 release.

Benjamin Block (1):
fix memory leak during adapter release by destroying gid_pn_req

 drivers/s390/scsi/zfcp_aux.c | 1 +
 1 file changed, 1 insertion(+)
Signature · 1 line
2.53.0
author maintainer
#1 lore raw

PATCH 1/1 fix memory leak during adapter release by destroying gid_pn_req

From: Benjamin Block <[email protected]>

When releasing an adapter we don't free the mempool gid_pn_req that is allocated during the enqueue. This leaks memory:

unreferenced object 0xd8d29297de700 (size 256):

    comm "(udev-worker)", pid 2105, jiffies 4294945794
    hex dump (first 32 bytes):
      00 00 00 00 de ad 4e ad ff ff ff ff 00 00 00 00  ......N.........
      ff ff ff ff ff ff ff ff 00 0d c4 5f 67 9d 99 e0  ..........._g...
    backtrace (crc 4a5b5da2):
      [<000dc45f64da418c>] kmemleak_alloc+0x6c/0xa0
      [<000dc45f62b430aa>] __kmalloc_cache_node_noprof+0x36a/0x4d0
      [<000dc45f629a535a>] mempool_create_node_noprof+0xaa/0x150
      [<000dc45ee2c065e6>] zfcp_allocate_low_mem_buffers+0x96/0x370 [zfcp]
      [<000dc45ee2c070f8>] zfcp_adapter_enqueue+0x598/0xd40 [zfcp]
      [<000dc45ee2c08eb0>] zfcp_ccw_set_online+0x160/0x210 [zfcp]
      [<000dc45f643d4762>] ccw_device_set_online+0x232/0xd80
      [<000dc45f643d53d4>] online_store_recog_and_online+0x124/0x390
      [<000dc45f643d8238>] online_store+0x298/0x5b0
      [<000dc45f62eb0a04>] kernfs_fop_write_iter+0x2c4/0x480
      [<000dc45f62c81150>] new_sync_write+0x370/0x4b0
      [<000dc45f62c87abe>] vfs_write+0x43e/0x5b0
      [<000dc45f62c87ff4>] ksys_write+0x114/0x1f0
      [<000dc45f621c4a16>] do_syscall+0x2f6/0x430
      [<000dc45f64d9d5d8>] __do_syscall+0xc8/0x1c0
      [<000dc45f64dc2224>] system_call+0x74/0xa0

Fix this by destroying the mempool during the adapter's release.

Fixes 799b76d09aee ("[SCSI] zfcp: Decouple gid_pn requests from erp")
Signed-off-by Benjamin Block <[email protected]>
Tested-by M Nikhil <[email protected]>
Acked-by M Nikhil <[email protected]>
Reviewed-by Chinmaya Kajagar <[email protected]>
Reviewed-by Nihar Panda <[email protected]>
---
 drivers/s390/scsi/zfcp_aux.c | 1 +
 1 file changed, 1 insertion(+)
drivers/s390/scsi/zfcp_aux.c +1 -0
index 8ff7db7921b5..fea573e00f9d 100644
@@ -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);
 	mempool_destroy(adapter->pool.scsi_req);
 	mempool_destroy(adapter->pool.scsi_abort);
 	mempool_destroy(adapter->pool.qtcb_pool);
Signature · 1 line
2.53.0
AI review
#2 lore raw

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

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

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 <[email protected]>

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.

drivers/s390/scsi/zfcp_aux.c static int zfcp_allocate_low_mem_buffers(struct zfcp_adapter *adapter) · line 256

Quoted 9 lines
drivers/s390/scsi/zfcp_aux.c +1 -0
index 8ff7db7921b5a..fea573e00f9df 100644
@@ -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?

Quoted 2 lines
drivers/s390/scsi/zfcp_aux.c +0 -0
 	mempool_destroy(adapter->pool.scsi_req);
 	mempool_destroy(adapter->pool.scsi_abort);
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
author maintainer
#3 lore raw

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

Hi Martin and James,
The bug reported by the sashiko bot is a pre-existing issue and was not introduced by this fix.

Please allow this fix to go upstream. We will create an EWM ticket to address the bot's finding in a separate patch.

Quoted 9 lines · On 20/07/26 1:10 pm, [email protected] wrote:

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 <[email protected]>

fix memory leak during adapter release by destroying gid_pn_req

Applied
#4 lore raw

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

In reply to Nihar Panda
Quoted 26 lines · Nihar Panda

When releasing an adapter we don't free the mempool gid_pn_req that is allocated during the enqueue. This leaks memory:

unreferenced object 0xd8d29297de700 (size 256):

    comm "(udev-worker)", pid 2105, jiffies 4294945794
    hex dump (first 32 bytes):
      00 00 00 00 de ad 4e ad ff ff ff ff 00 00 00 00  ......N.........
      ff ff ff ff ff ff ff ff 00 0d c4 5f 67 9d 99 e0  ..........._g...
    backtrace (crc 4a5b5da2):
      [<000dc45f64da418c>] kmemleak_alloc+0x6c/0xa0
      [<000dc45f62b430aa>] __kmalloc_cache_node_noprof+0x36a/0x4d0
      [<000dc45f629a535a>] mempool_create_node_noprof+0xaa/0x150
      [<000dc45ee2c065e6>] zfcp_allocate_low_mem_buffers+0x96/0x370 [zfcp]
      [<000dc45ee2c070f8>] zfcp_adapter_enqueue+0x598/0xd40 [zfcp]
      [<000dc45ee2c08eb0>] zfcp_ccw_set_online+0x160/0x210 [zfcp]
      [<000dc45f643d4762>] ccw_device_set_online+0x232/0xd80
      [<000dc45f643d53d4>] online_store_recog_and_online+0x124/0x390
      [<000dc45f643d8238>] online_store+0x298/0x5b0
      [<000dc45f62eb0a04>] kernfs_fop_write_iter+0x2c4/0x480
      [<000dc45f62c81150>] new_sync_write+0x370/0x4b0
      [<000dc45f62c87abe>] vfs_write+0x43e/0x5b0
      [<000dc45f62c87ff4>] ksys_write+0x114/0x1f0
      [<000dc45f621c4a16>] do_syscall+0x2f6/0x430
      [<000dc45f64d9d5d8>] __do_syscall+0xc8/0x1c0
      [<000dc45f64dc2224>] system_call+0x74/0xa0
Quoted 1 line

[...]

Applied to 7.2/scsi-fixes, thanks!

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

      https://git.kernel.org/mkp/scsi/c/b601fa590e66
Signature · 1 line
Martin K. Petersen