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 am assuming here that you meant to say that after applying
patch 1/2, there is a possibility for a 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^{+}).
Do we want to squash these? Is the use after free possible prior to this
patch?
I am fine with squashing these if that is the consensus here. Prior
to this patch, the remove callback function returned -EBUSY
if a guest is still using the matrix_mdev (i.e., matrix_mdev->kvm
not NULL), so the matrix_mdev was not freed and hence the
memory leak for this this patch was designed to fix.
Regards,
Halil
