linux-kernel - 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: <481cd60a-d633-4251-bb53-d3026e005930@oracle.com>
Date: Sun, 12 Jan 2025 15:19:44 -0600
From: Mike Christie <michael.christie@...cle.com>
To: Haoran Zhang <wh1sper@....edu.cn>, mst@...hat.com
Cc: 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: [PATCH] vhost/scsi: Fix improper cleanup in
 vhost_scsi_set_endpoint()

On 1/12/25 11:35 AM, michael.christie@...cle.com wrote:
> So I think to fix the issue, we would want to:
> 
> 1. move the
> 
> memcpy(vs_tpg, vs->vs_tpg, len);
> 
> to the end of the function after we do the vhost_scsi_flush. This will
> be more complicated than the current memcpy though. We will want to
> merge the local vs_tpg and the vs->vs_tpg like:
> 
> for (i = 0; i < VHOST_SCSI_MAX_TARGET; i++) {
> 	if (vs_tpg[i])
> 		vs->vs_tpg[i] = vs_tpg[i])
> }

I think I wrote that in reverse. We would want:

vhost_scsi_flush(vs);

if (vs->vs_tpg) {
	for (i = 0; i < VHOST_SCSI_MAX_TARGET; i++) {
		if (vs->vs_tpg[i])
			vs_tpg[i] = vs->vs_tpg[i])
	}
}

kfree(vs->vs_tpg);
vs->vs_tpg = vs_tpg;

or we could just allocate the vs_tpg with the vhost_scsi like:

struct vhost_scsi {
	....

	struct vhost_scsi_tpg *vs_tpg[VHOST_SCSI_MAX_TARGET];

then when we loop in vhost_scsi_set/clear_endpoint set/clear the
every vs_tpg entry.