Re: [PATCH v2 02/14] vfio/mbochs: Fix missing error unwind in mbochs_probe()
From: Jason Gunthorpe <jgg@nvidia.com>
Date: 2021-07-20 22:50:51
Also in:
dri-devel, intel-gfx, kvm, linux-s390
On Tue, Jul 20, 2021 at 04:01:27PM -0600, Alex Williamson wrote:
On Tue, 20 Jul 2021 14:42:48 -0300 Jason Gunthorpe [off-list ref] wrote:quoted
Compared to mbochs_remove() two cases are missing from the vfio_register_group_dev() unwind. Add them in. Fixes: 681c1615f891 ("vfio/mbochs: Convert to use vfio_register_group_dev()") Reported-by: Cornelia Huck <cohuck@redhat.com> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> samples/vfio-mdev/mbochs.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c index e81b875b4d87b4..501845b08c0974 100644 +++ b/samples/vfio-mdev/mbochs.c@@ -553,11 +553,14 @@ static int mbochs_probe(struct mdev_device *mdev) ret = vfio_register_group_dev(&mdev_state->vdev); if (ret) - goto err_mem; + goto err_bytes; dev_set_drvdata(&mdev->dev, mdev_state); return 0; +err_bytes: + mbochs_used_mbytes -= mdev_state->type->mbytes; err_mem: + kfree(mdev_state->pages); kfree(mdev_state->vconfig); kfree(mdev_state); return ret;@@ -567,8 +570,8 @@ static void mbochs_remove(struct mdev_device *mdev) { struct mdev_state *mdev_state = dev_get_drvdata(&mdev->dev); - mbochs_used_mbytes -= mdev_state->type->mbytes; vfio_unregister_group_dev(&mdev_state->vdev); + mbochs_used_mbytes -= mdev_state->type->mbytes; kfree(mdev_state->pages); kfree(mdev_state->vconfig); kfree(mdev_state);Hmm, doesn't this suggest we need another atomic conversion? (untested)
Sure why not, I can add this as another patch
quoted hunk ↗ jump to hunk
@@ -567,11 +573,11 @@ static void mbochs_remove(struct mdev_device *mdev) { struct mdev_state *mdev_state = dev_get_drvdata(&mdev->dev); - mbochs_used_mbytes -= mdev_state->type->mbytes; vfio_unregister_group_dev(&mdev_state->vdev); kfree(mdev_state->pages); kfree(mdev_state->vconfig); kfree(mdev_state); + atomic_add(mdev_state->type->mbytes, &mbochs_avail_mbytes);
This should be up after the vfio_unregister_group_dev(), it is a use after free? Jason