Hi Kuan-Wei,
On Sat, Jan 11, 2025 at 13:45:50 +0800, Kuan-Wei Chiu wrote:
> Hi Haoran,
>
> On Sat, Jan 11, 2025 at 11:34:18AM +0800, Haoran Zhang wrote:
> > Since commit 3f8ca2e115e55 ("vhost scsi: alloc cmds per vq instead of session"), a bug can be triggered when the host sends a duplicate VHOST_SCSI_SET_ENDPOINT ioctl command.
> >
> > In vhost_scsi_set_endpoint(), if the new `vhost_wwpn` matches the old tpg's tport_name but the tpg is still held by current vhost_scsi(i.e. it is busy), the active `tpg` will be unreferenced. Subsequently, if the owner releases vhost_scsi, the assertion `BUG_ON(sd->s_dependent_count < 1)` will be triggerred, terminating the target_undepend_item() procedure and leaving `configfs_dirent_lock` locked. If user enters configfs afterward, the CPU will become locked up.
> > This issue occurs because vhost_scsi_set_endpoint() allocates a new `vs_tpg` to hold the tpg array and copies all the old tpg entries into it before proceeding. When the new target is busy, the controw flow falls back to the `undepend` label, cause ing all the target `tpg` entries to be unreferenced, including the old one, which is still in use.
> >
> > The backtrace is:
> >
> > [ 60.085044] kernel BUG at fs/configfs/dir.c:1179!
> > [ 60.087729] RIP: 0010:configfs_undepend_item+0x76/0x80
> > [ 60.094735] Call Trace:
> > [ 60.094926] <TASK>
> > [ 60.098232] target_undepend_item+0x1a/0x30
> > [ 60.098745] vhost_scsi_clear_endpoint+0x363/0x3e0
> > [ 60.099342] vhost_scsi_release+0xea/0x1a0
> > [ 60.099860] ? __pfx_vhost_scsi_release+0x10/0x10
> > [ 60.100459] ? __pfx_locks_remove_file+0x10/0x10
> > [ 60.101025] ? __pfx_task_work_add+0x10/0x10
> > [ 60.101565] ? evm_file_release+0xc8/0xe0
> > [ 60.102074] ? __pfx_vhost_scsi_release+0x10/0x10
> > [ 60.102661] __fput+0x222/0x5a0
> > [ 60.102925] ____fput+0x1e/0x30
> > [ 60.103187] task_work_run+0x133/0x1c0
> > [ 60.103479] ? __pfx_task_work_run+0x10/0x10
> > [ 60.103813] ? pick_next_task_fair+0xe1/0x6f0
> > [ 60.104179] syscall_exit_to_user_mode+0x235/0x240
> > [ 60.104542] do_syscall_64+0x8a/0x170
> > [ 60.113301] </TASK>
> > [ 60.113931] ---[ end trace 0000000000000000 ]---
> > [ 60.121517] note: poc[2363] exited with preempt_count 1
> >
> > To fix this issue, the controw flow should be redirected to the `free_vs_tpg` label to ensure proper cleanup.
> >
> > Fixes: 3f8ca2e115e55 ("vhost scsi: alloc cmds per vq instead of session")
> > Signed-off-by: Haoran Zhang <wh1sper@xxxxxxxxxx>
>
> checkpatch.pl generated the following errors and warnings:
>
> WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
> #59:
> Since commit 3f8ca2e115e55 ("vhost scsi: alloc cmds per vq instead of session"), a bug can be triggered when the host sends a duplicate VHOST_SCSI_SET_ENDPOINT ioctl command.
>
> ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 3f8ca2e115e5 ("vhost/scsi: Extract common handling code from control queue handler")'
> #59:
> Since commit 3f8ca2e115e55 ("vhost scsi: alloc cmds per vq instead of session"), a bug can be triggered when the host sends a duplicate VHOST_SCSI_SET_ENDPOINT ioctl command.
>
> WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: 3f8ca2e115e5 ("vhost/scsi: Extract common handling code from control queue handler")'
> #91:
> Fixes: 3f8ca2e115e55 ("vhost scsi: alloc cmds per vq instead of session")
>
> total: 1 errors, 2 warnings, 15 lines checked
>
>
> Regards,
> Kuan-Wei
Thanks for your suggestion, I will send a corrected patch later.
Best regards,
Haoran Zhang
