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

Message-ID: <38a0d34d.3d9.1947d5437b6.Coremail.wh1sper@zju.edu.cn>
Date: Sun, 19 Jan 2025 14:50:58 +0800 (GMT+08:00)
From: 张浩然 <wh1sper@....edu.cn>
To: "Mike Christie" <michael.christie@...cle.com>
Cc: mst@...hat.com, jasowang@...hat.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()

On 2025-01-18 00:50:04, Mike Christie wrote:
> Yeah, I'm not sure if being able to call vhost_scsi_set_endpoint multiple
> times and pick up new tpgs is actually a feature or not. There's so many
> bugs and it also doesn't support tpg removal.

It seems vhost_scsi_clear_endpoint() is attempting to handle this, but it actually undepends all TPGs, ignoring the target, and also introduces the dangling pointer when `match == 0`.


> > [PATCH] vhost/scsi: Fix dangling pointer in vhost_scsi_set_endpoint()
> > 
> > Since commit 4f7f46d32c98 ("tcm_vhost: Use vq->private_data to indicate
> > if the endpoint is setup"), a dangling pointer issue has been introduced
> > in vhost_scsi_set_endpoint() when the host fails to reconfigure the
> > vhost-scsi endpoint. Specifically, this causes a UAF fault in
> > vhost_scsi_get_req() when the guest attempts to send an SCSI request.
> > 
> I saw that while reviewing the code. Here is my patch. I just added a new
> goto, because we don't need to do the undepend since we never did any
> depend calls.

Yes, there's no need to call undepend_item - just free vs_tpg. My patch was incorrect, thanks for bringing that to my attention.