mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] scsi: fix scsi_debug module-exit panic and pseudo target leak
       [not found] <8e4ba1820a7c56867bf7fff07bd52ff3f79573a1.camel@redhat.com>
@ 2026-11-03 19:32 ` Laurence Oberman
  2026-11-03 19:32   ` [PATCH 1/2] scsi: core: Fix target reap_ref leak in scsi_get_pseudo_sdev() Laurence Oberman
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Laurence Oberman @ 2026-11-03 19:32 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James E . J . Bottomley, Hannes Reinecke, Bart Van Assche,
	Wenchao Hao, Wenchao Hao, Catalin Marinas, Ming Lei, Will Deacon,
	cki-project, jmoyer, bgoncalv, mpatalan, ftjahjad, fsuba,
	linux-arm-kernel, linux-kernel

CKI reported a NULL pointer dereference in debugfs_remove() from
sdebug_tartget_cleanup_async() while blktests throtl/001 unloaded
scsi_debug on 7.4.0-rc1 (arm64). The bug is not arm64 specific. It
reproduces on x86_64 with KASAN as a slab-use-after-free of the
per-target debugfs dentry.

While fixing it, instrumenting target alloc/destroy in scsi_debug showed
a second, independent problem in the SCSI core: the pseudo SCSI device's
target is never destroyed, because scsi_get_pseudo_sdev() leaks a
reap_ref. With add_host=4 num_tgts=16, ->target_alloc() was called 68
times and ->target_destroy() only 64; the missing four were the
pseudo-device targets at id max_id. Any LLD that sets nr_reserved_cmds
leaks its pseudo target.

Patch 1 fixes the core leak. Patch 2 makes scsi_debug_exit() wait for
the deferred per-target debugfs cleanup before removing the debugfs
root and unloading. Patch 1 was found while testing patch 2, and
patch 2 was tested on top of it.

Testing (x86_64 VM, KASAN, 7.4.0-rc1): loop
"modprobe scsi_debug add_host=4 num_tgts=16; rmmod scsi_debug", with
an msleep(200) temporarily added to sdebug_tartget_cleanup_async() to
force the race window.
  - unpatched: KASAN slab-use-after-free / oops on the first rmmod
  - patch 1: 68 target_alloc / 68 target_destroy, pseudo targets
    destroyed during scsi_remove_host()
  - patches 1+2: 50 iterations clean with the msleep

I had assistatnce from LLM for both of these and ran a local sashiko
which was clean other than a pre-existing issue

Laurence Oberman (2):
  scsi: core: Fix target reap_ref leak in scsi_get_pseudo_sdev()
  scsi: scsi_debug: Wait for async target cleanup on module exit

 drivers/scsi/scsi_debug.c |  2 ++
 drivers/scsi/scsi_scan.c  | 15 ++++++++-------
 2 files changed, 10 insertions(+), 7 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] scsi: core: Fix target reap_ref leak in scsi_get_pseudo_sdev()
  2026-11-03 19:32 ` [PATCH 0/2] scsi: fix scsi_debug module-exit panic and pseudo target leak Laurence Oberman
@ 2026-11-03 19:32   ` Laurence Oberman
  2026-11-03 19:32   ` [PATCH] scsi: scsi_debug: Wait for async target cleanup on module exit Laurence Oberman
  2026-11-03 20:52   ` [PATCH 2/2] " Laurence Oberman
  2 siblings, 0 replies; 5+ messages in thread
From: Laurence Oberman @ 2026-11-03 19:32 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James E . J . Bottomley, Hannes Reinecke, Bart Van Assche,
	Wenchao Hao, Wenchao Hao, Catalin Marinas, Ming Lei, Will Deacon,
	cki-project, jmoyer, bgoncalv, mpatalan, ftjahjad, fsuba,
	linux-arm-kernel, linux-kernel

scsi_alloc_target() returns a target holding the initial reap_ref from
kref_init(). The scan paths drop that reference with scsi_target_reap()
once they are done with the target; the device allocated under it holds
its own reap_ref, taken in scsi_sysfs_device_initialize() and dropped
by __scsi_remove_device().

scsi_get_pseudo_sdev() only drops the initial reference on the failure
path. On success the pseudo target is left with an extra reap_ref, so
when scsi_forget_host() removes the pseudo device the count only drops
to one and the target is never destroyed: ->target_destroy() is not
called, and the scsi_target, the LLD's per-target data and the
reference the target holds on the host's gendev are leaked.

With scsi_debug (nr_reserved_cmds = 1) this is visible as one target
per host, at id max_id, for which ->target_alloc() is called but
->target_destroy() never is.

Drop the initial reap_ref unconditionally, as the scan paths do. The
pseudo target is then destroyed when the pseudo device is removed.

Fixes: d630fbf6fc8c ("scsi: core: Support allocating a pseudo SCSI device")
Signed-off-by: Laurence Oberman <loberman@redhat.com>
---
 drivers/scsi/scsi_scan.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 8aae12fd38f3..9ed52724f97c 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -2154,14 +2154,15 @@ struct scsi_device *scsi_get_pseudo_sdev(struct Scsi_Host *shost)
 		goto out;
 
 	sdev = scsi_alloc_sdev(starget, U64_MAX, NULL);
-	if (!sdev) {
-		scsi_target_reap(starget);
-		goto put_target;
-	}
-
-	sdev->borken = 0;
+	if (sdev)
+		sdev->borken = 0;
 
-put_target:
+	/*
+	 * Drop the reap_ref taken by scsi_alloc_target(). If the pseudo device
+	 * was allocated it holds its own reap_ref, which __scsi_remove_device()
+	 * drops, so the target is destroyed when the pseudo device is removed.
+	 */
+	scsi_target_reap(starget);
 	/* See also the get_device(dev) call in scsi_alloc_target(). */
 	put_device(&starget->dev);
 
-- 
2.56.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] scsi: scsi_debug: Wait for async target cleanup on module exit
  2026-11-03 19:32 ` [PATCH 0/2] scsi: fix scsi_debug module-exit panic and pseudo target leak Laurence Oberman
  2026-11-03 19:32   ` [PATCH 1/2] scsi: core: Fix target reap_ref leak in scsi_get_pseudo_sdev() Laurence Oberman
@ 2026-11-03 19:32   ` Laurence Oberman
  2026-11-03 20:53     ` Laurence Oberman
  2026-11-03 20:52   ` [PATCH 2/2] " Laurence Oberman
  2 siblings, 1 reply; 5+ messages in thread
From: Laurence Oberman @ 2026-11-03 19:32 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James E . J . Bottomley, Hannes Reinecke, Bart Van Assche,
	Wenchao Hao, Wenchao Hao, Catalin Marinas, Ming Lei, Will Deacon,
	cki-project, jmoyer, bgoncalv, mpatalan, ftjahjad, fsuba,
	linux-arm-kernel, linux-kernel

sdebug_target_destroy() is called with the host lock held, so it defers
removal of the per-target debugfs directory to
sdebug_tartget_cleanup_async() on sdebug_async_domain.
scsi_debug_exit() removes the hosts and then removes
sdebug_debugfs_root without waiting for that work. The recursive
removal of the root frees the per-target dentries, and a cleanup that
runs afterwards calls debugfs_remove() on a stale dentry:

  BUG: KASAN: slab-use-after-free in _raw_spin_lock+0x2d/0x40
  Workqueue: async async_run_entry_fn
  Call Trace:
   kasan_report+0x92/0xc0
   _raw_spin_lock+0x2d/0x40
   lockref_get+0x9/0x40
   __simple_recursive_removal+0x35/0x570
   debugfs_remove+0x40/0x60
   sdebug_tartget_cleanup_async+0x43/0x60 [scsi_debug]
   async_run_entry_fn+0x96/0x520
   process_one_work+0x925/0x1910
   worker_thread+0x567/0xcc0
   kthread+0x355/0x450
  Allocated by task 150:
   __d_alloc+0x34/0x9e0
   d_alloc+0x44/0x1d0
   ...
   debugfs_create_dir+0x18/0x3d0
   sdebug_target_alloc
   scsi_alloc_target+0x7c7/0xa70

followed by a NULL inode dereference in down_write() from
__simple_recursive_removal(). CKI hit this running blktests throtl/001
on arm64. The cleanup can also run after the module text has been
freed.

Commit b402a0dce64a ("scsi: scsi_debug: Fix create target debugfs
failure") drains sdebug_async_domain before creating a target. Do the
same in scsi_debug_exit() just before removing the debugfs root, after
the driver has been unregistered so that no host can still be bound and
queue further cleanup work.

Fixes: f084fe52c640 ("scsi: scsi_debug: Add debugfs interface to fail target reset")
Reported-by: Catalin Marinas <catalin.marinas@arm.com>
Closes: https://lore.kernel.org/r/auja2VnJTp-ijxqu@arm.com
Reported-by: CKI Project <cki-project@redhat.com>
Closes: https://datawarehouse.cki-project.org/kcidb/checkouts/redhat:3017229113
Signed-off-by: Laurence Oberman <loberman@redhat.com>
---
 drivers/scsi/scsi_debug.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index a922c8318a2f..deaed8b3e6c4 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -9053,6 +9053,8 @@ static void __exit scsi_debug_exit(void)
 
 	sdebug_erase_all_stores(false);
 	xa_destroy(per_store_ap);
+	/* Wait for deferred per-target debugfs cleanup before removing the root */
+	async_synchronize_full_domain(&sdebug_async_domain);
 	debugfs_remove(sdebug_debugfs_root);
 }
 
-- 
2.56.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] scsi: scsi_debug: Wait for async target cleanup on module exit
  2026-11-03 19:32 ` [PATCH 0/2] scsi: fix scsi_debug module-exit panic and pseudo target leak Laurence Oberman
  2026-11-03 19:32   ` [PATCH 1/2] scsi: core: Fix target reap_ref leak in scsi_get_pseudo_sdev() Laurence Oberman
  2026-11-03 19:32   ` [PATCH] scsi: scsi_debug: Wait for async target cleanup on module exit Laurence Oberman
@ 2026-11-03 20:52   ` Laurence Oberman
  2 siblings, 0 replies; 5+ messages in thread
From: Laurence Oberman @ 2026-11-03 20:52 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James E . J . Bottomley, Hannes Reinecke, Bart Van Assche,
	Wenchao Hao, Wenchao Hao, Catalin Marinas, Will Deacon,
	cki-project, jmoyer, bgoncalv, mpatalan, ftjahjad, fsuba,
	linux-arm-kernel, linux-kernel

sdebug_target_destroy() is called with the host lock held, so it defers
removal of the per-target debugfs directory to
sdebug_tartget_cleanup_async() on sdebug_async_domain.
scsi_debug_exit() removes the hosts and then removes
sdebug_debugfs_root without waiting for that work. The recursive
removal of the root frees the per-target dentries, and a cleanup that
runs afterwards calls debugfs_remove() on a stale dentry:

  BUG: KASAN: slab-use-after-free in _raw_spin_lock+0x2d/0x40
  Workqueue: async async_run_entry_fn
  Call Trace:
   kasan_report+0x92/0xc0
   _raw_spin_lock+0x2d/0x40
   lockref_get+0x9/0x40
   __simple_recursive_removal+0x35/0x570
   debugfs_remove+0x40/0x60
   sdebug_tartget_cleanup_async+0x43/0x60 [scsi_debug]
   async_run_entry_fn+0x96/0x520
   process_one_work+0x925/0x1910
   worker_thread+0x567/0xcc0
   kthread+0x355/0x450
  Allocated by task 150:
   __d_alloc+0x34/0x9e0
   d_alloc+0x44/0x1d0
   ...
   debugfs_create_dir+0x18/0x3d0
   sdebug_target_alloc
   scsi_alloc_target+0x7c7/0xa70

followed by a NULL inode dereference in down_write() from
__simple_recursive_removal(). CKI hit this running blktests throtl/001
on arm64. The cleanup can also run after the module text has been
freed.

Commit b402a0dce64a ("scsi: scsi_debug: Fix create target debugfs
failure") drains sdebug_async_domain before creating a target. Do the
same in scsi_debug_exit() just before removing the debugfs root, after
the driver has been unregistered so that no host can still be bound and
queue further cleanup work.

Fixes: f084fe52c640 ("scsi: scsi_debug: Add debugfs interface to fail target reset")
Reported-by: Catalin Marinas <catalin.marinas@arm.com>
Closes: https://lore.kernel.org/r/auja2VnJTp-ijxqu@arm.com
Reported-by: CKI Project <cki-project@redhat.com>
Closes: https://datawarehouse.cki-project.org/kcidb/checkouts/redhat:3017229113
Signed-off-by: Laurence Oberman <loberman@redhat.com>
---
 drivers/scsi/scsi_debug.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index a922c8318a2f..deaed8b3e6c4 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -9053,6 +9053,8 @@ static void __exit scsi_debug_exit(void)
 
 	sdebug_erase_all_stores(false);
 	xa_destroy(per_store_ap);
+	/* Wait for deferred per-target debugfs cleanup before removing the root */
+	async_synchronize_full_domain(&sdebug_async_domain);
 	debugfs_remove(sdebug_debugfs_root);
 }
 
-- 
2.56.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] scsi: scsi_debug: Wait for async target cleanup on module exit
  2026-11-03 19:32   ` [PATCH] scsi: scsi_debug: Wait for async target cleanup on module exit Laurence Oberman
@ 2026-11-03 20:53     ` Laurence Oberman
  0 siblings, 0 replies; 5+ messages in thread
From: Laurence Oberman @ 2026-11-03 20:53 UTC (permalink / raw)
  To: linux-scsi, Martin K . Petersen
  Cc: James E . J . Bottomley, Hannes Reinecke, Bart Van Assche,
	Wenchao Hao, Wenchao Hao, Catalin Marinas, Ming Lei, Will Deacon,
	cki-project, jmoyer, bgoncalv, mpatalan, ftjahjad, fsuba,
	linux-arm-kernel, linux-kernel

On Tue, 2026-11-03 at 14:32 -0500, Laurence Oberman wrote:
> sdebug_target_destroy() is called with the host lock held, so it
> defers
> removal of the per-target debugfs directory to
> sdebug_tartget_cleanup_async() on sdebug_async_domain.
> scsi_debug_exit() removes the hosts and then removes
> sdebug_debugfs_root without waiting for that work. The recursive
> removal of the root frees the per-target dentries, and a cleanup that
> runs afterwards calls debugfs_remove() on a stale dentry:
> 
>   BUG: KASAN: slab-use-after-free in _raw_spin_lock+0x2d/0x40
>   Workqueue: async async_run_entry_fn
>   Call Trace:
>    kasan_report+0x92/0xc0
>    _raw_spin_lock+0x2d/0x40
>    lockref_get+0x9/0x40
>    __simple_recursive_removal+0x35/0x570
>    debugfs_remove+0x40/0x60
>    sdebug_tartget_cleanup_async+0x43/0x60 [scsi_debug]
>    async_run_entry_fn+0x96/0x520
>    process_one_work+0x925/0x1910
>    worker_thread+0x567/0xcc0
>    kthread+0x355/0x450
>   Allocated by task 150:
>    __d_alloc+0x34/0x9e0
>    d_alloc+0x44/0x1d0
>    ...
>    debugfs_create_dir+0x18/0x3d0
>    sdebug_target_alloc
>    scsi_alloc_target+0x7c7/0xa70
> 
> followed by a NULL inode dereference in down_write() from
> __simple_recursive_removal(). CKI hit this running blktests
> throtl/001
> on arm64. The cleanup can also run after the module text has been
> freed.
> 
> Commit b402a0dce64a ("scsi: scsi_debug: Fix create target debugfs
> failure") drains sdebug_async_domain before creating a target. Do the
> same in scsi_debug_exit() just before removing the debugfs root,
> after
> the driver has been unregistered so that no host can still be bound
> and
> queue further cleanup work.
> 
> Fixes: f084fe52c640 ("scsi: scsi_debug: Add debugfs interface to fail
> target reset")
> Reported-by: Catalin Marinas <catalin.marinas@arm.com>
> Closes: https://lore.kernel.org/r/auja2VnJTp-ijxqu@arm.com
> Reported-by: CKI Project <cki-project@redhat.com>
> Closes:
> https://datawarehouse.cki-project.org/kcidb/checkouts/redhat:3017229113
> Signed-off-by: Laurence Oberman <loberman@redhat.com>
> ---
>  drivers/scsi/scsi_debug.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index a922c8318a2f..deaed8b3e6c4 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -9053,6 +9053,8 @@ static void __exit scsi_debug_exit(void)
>  
>  	sdebug_erase_all_stores(false);
>  	xa_destroy(per_store_ap);
> +	/* Wait for deferred per-target debugfs cleanup before
> removing the root */
> +	async_synchronize_full_domain(&sdebug_async_domain);
>  	debugfs_remove(sdebug_debugfs_root);
>  }
>  
Please ignore, this went out without series numbering. Resent as [PATCH
2/2] in reply to the cover letter.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-03 20:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <8e4ba1820a7c56867bf7fff07bd52ff3f79573a1.camel@redhat.com>
2026-11-03 19:32 ` [PATCH 0/2] scsi: fix scsi_debug module-exit panic and pseudo target leak Laurence Oberman
2026-11-03 19:32   ` [PATCH 1/2] scsi: core: Fix target reap_ref leak in scsi_get_pseudo_sdev() Laurence Oberman
2026-11-03 19:32   ` [PATCH] scsi: scsi_debug: Wait for async target cleanup on module exit Laurence Oberman
2026-11-03 20:53     ` Laurence Oberman
2026-11-03 20:52   ` [PATCH 2/2] " Laurence Oberman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®