linux-kernel - Re: Re: [PATCH] vhost/scsi: Fix improper cleanup in vhost_scsi_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <3e1e312e.21abc.194546e119c.Coremail.wh1sper@zju.edu.cn>
Date: Sat, 11 Jan 2025 16:14:47 +0800 (GMT+08:00)
From: 张浩然 <wh1sper@....edu.cn>
To: "Kuan-Wei Chiu" <visitorckw@...il.com>
Cc: mst@...hat.com, jasowang@...hat.com, michael.christie@...cle.com,
	pbonzini@...hat.com, stefanha@...hat.com, eperezma@...hat.com,
	virtualization@...ts.linux.dev, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: Re: [PATCH] vhost/scsi: Fix improper cleanup in
 vhost_scsi_set_endpoint()

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@....edu.cn>
> 
> 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