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: <CAPpAL=xC7+f_8V4D==JvZxs5X-CePa_VftOH=KDc8H1vPSNp9w@mail.gmail.com>
Date: Tue, 14 Jan 2025 10:17:50 +0800
From: Lei Yang <leiyang@...hat.com>
To: Haoran Zhang <wh1sper@....edu.cn>
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, 
	Mike Christie <michael.christie@...cle.com>
Subject: Re: [PATCH] vhost/scsi: Fix improper cleanup in vhost_scsi_set_endpoint()

I tested this patch with virtio-net regression tests, everything works fine.

Tested-by: Lei Yang <leiyang@...hat.com>


On Mon, Jan 13, 2025 at 5:20 AM Mike Christie
<michael.christie@...cle.com> wrote:
>
> 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.
>