[PATCH 0/2] Remove the /proc/scsi/${proc_name} directory earlier

[PATCH 0/2] Remove the /proc/scsi/${proc_name} directory earlier

[Bart Van Assche]

Hi Martin,

This patch series fixes a race condition in the SCSI core. Please consider
this patch series for the next merge window.

Thanks,

Bart.

Bart Van Assche (2):
  scsi: core: Fix a source code comment
  scsi: core: Remove the /proc/scsi/${proc_name} directory earlier

 drivers/scsi/hosts.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

[PATCH 1/2] scsi: core: Fix a source code comment

[Bart Van Assche]

Fix a reference in a source code comment to the scsi_remove_host()
function.

Fixes: b49493f99690 ("Fix a memory leak in scsi_host_dev_release()")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/hosts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 12346e2297fd..b28375f9e019 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -356,7 +356,7 @@ static void scsi_host_dev_release(struct device *dev)
 		/*
 		 * Free the shost_dev device name here if scsi_host_alloc()
 		 * and scsi_host_put() have been called but neither
-		 * scsi_host_add() nor scsi_host_remove() has been called.
+		 * scsi_host_add() nor scsi_remove_host() has been called.
 		 * This avoids that the memory allocated for the shost_dev
 		 * name is leaked.
 		 */

[PATCH 2/2] scsi: core: Remove the /proc/scsi/${proc_name} directory earlier

[Bart Van Assche]

Remove the /proc/scsi/${proc_name} directory earlier to fix a race
condition between unloading and reloading kernel modules. This patch
fixes a bug introduced in 2009 by patch "[SCSI] fix /proc memory leak
in the SCSI core".

This patch fixes the following kernel warning:

proc_dir_entry 'scsi/scsi_debug' already registered
WARNING: CPU: 19 PID: 27986 at fs/proc/generic.c:376 proc_register+0x27d/0x2e0
Call Trace:
 proc_mkdir+0xb5/0xe0
 scsi_proc_hostdir_add+0xb5/0x170
 scsi_host_alloc+0x683/0x6c0
 sdebug_driver_probe+0x6b/0x2d0 [scsi_debug]
 really_probe+0x159/0x540
 __driver_probe_device+0xdc/0x230
 driver_probe_device+0x4f/0x120
 __device_attach_driver+0xef/0x180
 bus_for_each_drv+0xe5/0x130
 __device_attach+0x127/0x290
 device_initial_probe+0x17/0x20
 bus_probe_device+0x110/0x130
 device_add+0x673/0xc80
 device_register+0x1e/0x30
 sdebug_add_host_helper+0x1a7/0x3b0 [scsi_debug]
 scsi_debug_init+0x64f/0x1000 [scsi_debug]
 do_one_initcall+0xd7/0x470
 do_init_module+0xe7/0x330
 load_module+0x122a/0x12c0
 __do_sys_finit_module+0x124/0x1a0
 __x64_sys_finit_module+0x46/0x50
 do_syscall_64+0x38/0x80
 entry_SYSCALL_64_after_hwframe+0x46/0xb0

Reported-by: Yi Zhang <yi.zhang@redhat.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Yi Zhang <yi.zhang@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/hosts.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index b28375f9e019..f7f62e56afca 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -181,6 +181,7 @@ void scsi_remove_host(struct Scsi_Host *shost)
 	scsi_forget_host(shost);
 	mutex_unlock(&shost->scan_mutex);
 	scsi_proc_host_rm(shost);
+	scsi_proc_hostdir_rm(shost->hostt);
 
 	/*
 	 * New SCSI devices cannot be attached anymore because of the SCSI host
@@ -340,6 +341,7 @@ static void scsi_host_dev_release(struct device *dev)
 	struct Scsi_Host *shost = dev_to_shost(dev);
 	struct device *parent = dev->parent;
 
+	/* In case scsi_remove_host() has not been called. */
 	scsi_proc_hostdir_rm(shost->hostt);
 
 	/* Wait for functions invoked through call_rcu(&scmd->rcu, ...) */

Re: [PATCH 0/2] Remove the /proc/scsi/${proc_name} directory earlier

[Martin K. Petersen]

Bart,

> This patch series fixes a race condition in the SCSI core. Please
> consider this patch series for the next merge window.

Applied to 6.3/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/2] Remove the /proc/scsi/${proc_name} directory earlier

[John Garry]

On 10/02/2023 20:51, Bart Van Assche wrote:
> Hi Martin,
> 
> This patch series fixes a race condition in the SCSI core. Please consider
> this patch series for the next merge window.
> 
> Thanks,
> 
> Bart.
> 
> Bart Van Assche (2):
>    scsi: core: Fix a source code comment
>    scsi: core: Remove the /proc/scsi/${proc_name} directory earlier
> 
>   drivers/scsi/hosts.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 

Hi Bart,

I notice this following issue with v6.3-rc1 for scsi_debug when I turn 
on CONFIG_DEBUG_TEST_DRIVER_REMOVE and set DEF_NUM_HOST as 2 in that driver:

[    1.330849] ------------[ cut here ]------------
[    1.333027] remove_proc_entry: removing non-empty directory 
'scsi/scsi_debug', leaking at least '0'
[    1.335280] sd 0:0:0:0: [sda] Attached SCSI disk
[    1.337231] WARNING: CPU: 3 PID: 1 at fs/proc/generic.c:718 
remove_proc_entry+0x180/0x190
[    1.342979] Modules linked in:
[    1.344389] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 6.3.0-rc1-dirty 
#414
[    1.346760] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
1.15.0-1 04/01/2014
[    1.349493] RIP: 0010:remove_proc_entry+0x180/0x190
[    1.351162] Code: c7 80 e8 6e aa 48 85 c0 48 8d 90 78 ff ff ff 48 0f 
45 c2 48 8b 55 78 4c 8b 80 a0 00 00 00 48 8b 92 a0 00 00 00 e8 10 0a d8 
ff <0f> 0b0
[    1.356712] RSP: 0000:ff7ad00780017c00 EFLAGS: 00010286
[    1.357991] RAX: 0000000000000000 RBX: ff4af384c146f440 RCX: 
0000000000000000
[    1.359716] RDX: 0000000000000001 RSI: 0000000000000001 RDI: 
0000000000000001
[    1.361439] RBP: ff4af384c1bc0300 R08: 6f6d6572203a7972 R09: 
746e655f636f7270
[    1.363145] R10: 72746e655f636f72 R11: 705f65766f6d6572 R12: 
ff4af384c1bc0380
[    1.364881] R13: ffffffffaa2ccd00 R14: ff4af384c1b47128 R15: 
ff4af384c1b4c420
[    1.366152] FS:  0000000000000000(0000) GS:ff4af3853aec0000(0000) 
knlGS:0000000000000000
[    1.367500] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    1.368463] CR2: 0000000000000000 CR3: 0000000030c2e001 CR4: 
0000000000771ee0
[    1.369639] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 
0000000000000000
[    1.370815] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 
0000000000000400
[    1.372040] PKRU: 55555554
[    1.372491] Call Trace:
[    1.372915]  <TASK>
[    1.373280]  scsi_proc_hostdir_rm+0x75/0xb0
[    1.373998]  scsi_host_dev_release+0x36/0xe0
[    1.374723]  device_release+0x30/0x90
[    1.375345]  kobject_put+0x68/0xd0
[    1.375923]  really_probe+0x24b/0x330
[    1.376451]  ? __pfx___device_attach_driver+0x10/0x10
[    1.377177]  __driver_probe_device+0x6d/0xd0
[    1.377794]  driver_probe_device+0x19/0xe0
[    1.378382]  __device_attach_driver+0x7e/0x110
[    1.379026]  bus_for_each_drv+0x7f/0xe0
[    1.379571]  __device_attach+0xb7/0x1d0
[    1.380126]  bus_probe_device+0x88/0xa0
[    1.380681]  device_add+0x612/0x800
[    1.381190]  sdebug_add_host_helper+0x1ce/0x260
[    1.381839]  scsi_debug_init+0x391/0x870
[    1.382431]  ? __pfx_scsi_debug_init+0x10/0x10
[    1.383081]  do_one_initcall+0x40/0x220
[    1.383637]  kernel_init_freeable+0x19a/0x2d0
[    1.384270]  ? __pfx_kernel_init+0x10/0x10
[    1.384862]  kernel_init+0x15/0x1b0
[    1.385368]  ret_from_fork+0x29/0x50
[    1.385891]  </TASK>
[    1.386203] ---[ end trace 0000000000000000 ]---
[    1.389637] scsi_debug:sdebug_driver_probe: scsi_debug: trim 
poll_queues to 0. poll_q/nr_hw = (0/1)
[    1.390954] scsi host1: scsi_debug: version 0191 [20210520]
                  dev_size_mb=8, opts=0x0, submit_queues=1, statistics=0

Is it related to this series, do you think?

Thanks,
John

Re: [PATCH 0/2] Remove the /proc/scsi/${proc_name} directory earlier

[Bart Van Assche]

On 3/7/23 07:08, John Garry wrote:
> Is it related to this series, do you think?

Probably. I'm not sure there is an alternative to reverting patch 2/2 
from this series and adding a wait loop inside
scsi_proc_hostdir_add(). Adding a wait loop inside 
scsi_proc_hostdir_add() might be controversial. Anyway, I will look into 
this.

Bart.