Commit c96f2c2a authored by Greg Kurz's avatar Greg Kurz Committed by Cornelia Huck
Browse files

vfio-ccw: introduce vfio_ccw_get_device() 



A recent patch fixed leaks of the dynamically allocated vcdev->vdev.name
field in vfio_ccw_realize(), but we now have three freeing sites for it.
This is unfortunate and seems to indicate something is wrong with its
life cycle.

The root issue is that vcdev->vdev.name is set before vfio_get_device()
is called, which theoretically prevents to call vfio_put_device() to
do the freeing. Well actually, we could call it anyway  because
vfio_put_base_device() is a nop if the device isn't attached, but this
would be confusing.

This patch hence moves all the logic of attaching the device, including
the "already attached" check, to a separate vfio_ccw_get_device() function,
counterpart of vfio_put_device(). While here, vfio_put_device() is renamed
to vfio_ccw_put_device() for consistency.

Signed-off-by: default avatarGreg Kurz <groug@kaod.org>
Message-Id: <152326891065.266543.9487977590811413472.stgit@bahia.lan>
Signed-off-by: default avatarCornelia Huck <cohuck@redhat.com>
parent 98e43b71

hw/vfio/ccw.c

+36 −20
Original line number Diff line number Diff line
@@ -292,12 +292,43 @@ static void vfio_ccw_put_region(VFIOCCWDevice *vcdev) 
    g_free(vcdev->io_region);

}



static void vfio_put_device(VFIOCCWDevice *vcdev)

static void vfio_ccw_put_device(VFIOCCWDevice *vcdev)

{

    g_free(vcdev->vdev.name);

    vfio_put_base_device(&vcdev->vdev);

}



static void vfio_ccw_get_device(VFIOGroup *group, VFIOCCWDevice *vcdev,

                                Error **errp)

{

    char *name = g_strdup_printf("%x.%x.%04x", vcdev->cdev.hostid.cssid,

                                 vcdev->cdev.hostid.ssid,

                                 vcdev->cdev.hostid.devid);

    VFIODevice *vbasedev;



    QLIST_FOREACH(vbasedev, &group->device_list, next) {

        if (strcmp(vbasedev->name, name) == 0) {

            error_setg(errp, "vfio: subchannel %s has already been attached",

                       name);

            goto out_err;

        }

    }



    if (vfio_get_device(group, vcdev->cdev.mdevid, &vcdev->vdev, errp)) {

        goto out_err;

    }



    vcdev->vdev.ops = &vfio_ccw_ops;

    vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW;

    vcdev->vdev.name = name;

    vcdev->vdev.dev = &vcdev->cdev.parent_obj.parent_obj;



    return;



out_err:

    g_free(name);

}



static VFIOGroup *vfio_ccw_get_group(S390CCWDevice *cdev, Error **errp)

{

    char *tmp, group_path[PATH_MAX];
@@ -327,7 +358,6 @@ static VFIOGroup *vfio_ccw_get_group(S390CCWDevice *cdev, Error **errp) 


static void vfio_ccw_realize(DeviceState *dev, Error **errp)

{

    VFIODevice *vbasedev;

    VFIOGroup *group;

    CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);

    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj, ccw_dev);
@@ -348,22 +378,8 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp) 
        goto out_group_err;

    }



    vcdev->vdev.ops = &vfio_ccw_ops;

    vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW;

    vcdev->vdev.name = g_strdup_printf("%x.%x.%04x", cdev->hostid.cssid,

                                       cdev->hostid.ssid, cdev->hostid.devid);

    vcdev->vdev.dev = dev;

    QLIST_FOREACH(vbasedev, &group->device_list, next) {

        if (strcmp(vbasedev->name, vcdev->vdev.name) == 0) {

            error_setg(&err, "vfio: subchannel %s has already been attached",

                       vcdev->vdev.name);

            g_free(vcdev->vdev.name);

            goto out_device_err;

        }

    }



    if (vfio_get_device(group, cdev->mdevid, &vcdev->vdev, &err)) {

        g_free(vcdev->vdev.name);

    vfio_ccw_get_device(group, vcdev, &err);

    if (err) {

        goto out_device_err;

    }
@@ -382,7 +398,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp) 
out_notifier_err:

    vfio_ccw_put_region(vcdev);

out_region_err:

    vfio_put_device(vcdev);

    vfio_ccw_put_device(vcdev);

out_device_err:

    vfio_put_group(group);

out_group_err:
@@ -403,7 +419,7 @@ static void vfio_ccw_unrealize(DeviceState *dev, Error **errp) 


    vfio_ccw_unregister_io_notifier(vcdev);

    vfio_ccw_put_region(vcdev);

    vfio_put_device(vcdev);

    vfio_ccw_put_device(vcdev);

    vfio_put_group(group);



    if (cdc->unrealize) {