Re: [PATCH v4 1/2] s390/vfio-ap: fix memory leak in mdev remove callback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 5/25/21 9:03 AM, Halil Pasic wrote:
On Fri, 21 May 2021 15:36:47 -0400
Tony Krowiak <akrowiak@xxxxxxxxxxxxx> wrote:

The mdev remove callback for the vfio_ap device driver bails out with
-EBUSY if the mdev is in use by a KVM guest. The intended purpose was
to prevent the mdev from being removed while in use; however, returning a
non-zero rc does not prevent removal. This could result in a memory leak
of the resources allocated when the mdev was created. In addition, the
KVM guest will still have access to the AP devices assigned to the mdev
even though the mdev no longer exists.

To prevent this scenario, cleanup will be done - including unplugging the
AP adapters, domains and control domains - regardless of whether the mdev
is in use by a KVM guest or not.

Fixes: 258287c994de ("s390: vfio-ap: implement mediated device open callback")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>
AFAIU we all agree that, after patch there is a possibility for an use
after free error. I'm a little confused by the fact that we want this
one for stable, but the patch that fixes the use after free as no
Cc stable (it can't have a proper fixes tag, because this one is not yet
merged). Actually I'm not a big fan of splitting up patches to the
extent that when not all patches of the series are applied we get bugous
behavior (e.g. patch n breaks something that is live at patch n level,
but it is supposed to be OK, because patch n+m is going to fix it (where
n,m \in \Z^{+}).

After thinking about this some more, this patch does not really
fix a memory leak and should probably not be flagged as a fix
for 258287c994. Memory is not leaked
because the remove callback returns -EBUSY without freeing
mdev storage or resetting the queues.

Under normal circumstances, if the mdev is removed before
the mdev fd is closed (i.e., the guest is shut down), the process
will wait until the fd is closed, at which time the
release callback will get invoked. Since the release callback
clears the KVM pointer from the matrix_mdev, the remove
callback will not return -EBUSY and will in fact free the mdev
storage when it is subsequently invoked.

I am going to change the subject and remove the 'Fixes'
tag as well as the 'Cc' of stable. I'll change the subject to
something like:

"s390/vfio-ap: always free storage for mdev in remove callback"


Do we want to squash these? Is the use after free possible prior to this
patch?

Regards,
Halil




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux