linux-kernel - [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-next>] [day] [month] [year] [list]

Message-ID: <20250111033454.26596-1-wh1sper@zju.edu.cn>
Date: Sat, 11 Jan 2025 11:34:18 +0800
From: Haoran Zhang <wh1sper@....edu.cn>
To: mst@...hat.com
Cc: 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,
	Haoran Zhang <wh1sper@....edu.cn>
Subject: [PATCH] vhost/scsi: Fix improper cleanup in vhost_scsi_set_endpoint()

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>
---
 drivers/vhost/scsi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 718fa4e0b31e..b994138837f2 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1726,7 +1726,7 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
 				mutex_unlock(&tpg->tv_tpg_mutex);
 				mutex_unlock(&vhost_scsi_mutex);
 				ret = -EEXIST;
-				goto undepend;
+				goto free_vs_tpg;
 			}
 			/*
 			 * In order to ensure individual vhost-scsi configfs
@@ -1802,6 +1802,7 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
 			target_undepend_item(&tpg->se_tpg.tpg_group.cg_item);
 		}
 	}
+free_vs_tpg:
 	kfree(vs_tpg);
 out:
 	mutex_unlock(&vs->dev.mutex);
-- 
2.43.0